2017-07-04 04:29:51

by Markus Trippelsdorf

[permalink] [raw]
Subject: Commit edf064e7c (btrfs: nowait aio support) breaks shells

commit edf064e7c6fec3646b06c944a8e35d1a3de5c2c3 (HEAD, refs/bisect/bad)
Author: Goldwyn Rodrigues <[email protected]>
Date: Tue Jun 20 07:05:49 2017 -0500

btrfs: nowait aio support

apparently breaks several shell related features on my system.
In zsh history stopped working, because no new entries are added
anymore.
I fist noticed the issue when I tried to build mplayer. It uses a shell
script to generate a help_mp.h file:

% help/help_create.sh help/help_mp-en.h UTF-8

This file gets corrupted:

--- help_mp_good.h 2017-07-04 05:38:33.161640826 +0200
+++ help_mp_bad.h 2017-07-04 05:51:00.650730726 +0200
@@ -1,14 +1,8 @@
-/* WARNING! This is a generated file, do NOT edit.
- * See the help/ subdirectory for the editable files. */

-#ifndef MPLAYER_HELP_MP_H
-#define MPLAYER_HELP_MP_H
-
-#include <inttypes.h>
-#include "config.h"
+#endif /* MPLAYER_HELP_MP_H */
+he English master file */

-// $Revision: 37846 $
-// MASTER FILE. Use this file as base for translations.
+ for translations.
...
(I have attached the testcase.)

/dev/sdc3 on / type btrfs (rw,noatime,lazytime,compress=lzo,ssd,noacl,space_cache=v2,subvolid=5,subvol=/)
# cat /sys/block/sdc/queue/scheduler
[none] mq-deadline

--
Markus


Attachments:
(No filename) (1.22 kB)
test.tar.bz2 (31.05 kB)
Download all attachments

2017-07-04 07:45:15

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

On 2017.07.04 at 06:23 +0200, Markus Trippelsdorf wrote:
> commit edf064e7c6fec3646b06c944a8e35d1a3de5c2c3 (HEAD, refs/bisect/bad)
> Author: Goldwyn Rodrigues <[email protected]>
> Date: Tue Jun 20 07:05:49 2017 -0500
>
> btrfs: nowait aio support
>
> apparently breaks several shell related features on my system.

Here is a simple testcase:

% echo "foo" >> test
% echo "foo" >> test
% cat test
foo
%

--
Markus

2017-07-04 15:31:49

by Goldwyn Rodrigues

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells



On 07/04/2017 02:45 AM, Markus Trippelsdorf wrote:
> On 2017.07.04 at 06:23 +0200, Markus Trippelsdorf wrote:
>> commit edf064e7c6fec3646b06c944a8e35d1a3de5c2c3 (HEAD, refs/bisect/bad)
>> Author: Goldwyn Rodrigues <[email protected]>
>> Date: Tue Jun 20 07:05:49 2017 -0500
>>
>> btrfs: nowait aio support
>>
>> apparently breaks several shell related features on my system.
>
> Here is a simple testcase:
>
> % echo "foo" >> test
> % echo "foo" >> test
> % cat test
> foo
> %
>

Thanks for testing.
Yes, pos must be set with iocb->ki_pos for appends. I should not have
removed the initialization. Could you try this patch?

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 59e2dccdf75b..7947781229e5 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1931,6 +1931,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb
*iocb,
*/
update_time_for_write(inode);

+ pos = iocb->ki_pos;
start_pos = round_down(pos, fs_info->sectorsize);
oldsize = i_size_read(inode);
if (start_pos > oldsize) {



--
Goldwyn

2017-07-04 15:37:56

by Markus Trippelsdorf

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

On 2017.07.04 at 10:31 -0500, Goldwyn Rodrigues wrote:
>
>
> On 07/04/2017 02:45 AM, Markus Trippelsdorf wrote:
> > On 2017.07.04 at 06:23 +0200, Markus Trippelsdorf wrote:
> >> commit edf064e7c6fec3646b06c944a8e35d1a3de5c2c3 (HEAD, refs/bisect/bad)
> >> Author: Goldwyn Rodrigues <[email protected]>
> >> Date: Tue Jun 20 07:05:49 2017 -0500
> >>
> >> btrfs: nowait aio support
> >>
> >> apparently breaks several shell related features on my system.
> >
> > Here is a simple testcase:
> >
> > % echo "foo" >> test
> > % echo "foo" >> test
> > % cat test
> > foo
> > %
> >
>
> Thanks for testing.
> Yes, pos must be set with iocb->ki_pos for appends. I should not have
> removed the initialization. Could you try this patch?

It fixes the issue. Thank you.

--
Markus

2017-07-04 22:16:57

by Jens Axboe

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

On 07/04/2017 09:31 AM, Goldwyn Rodrigues wrote:
>
>
> On 07/04/2017 02:45 AM, Markus Trippelsdorf wrote:
>> On 2017.07.04 at 06:23 +0200, Markus Trippelsdorf wrote:
>>> commit edf064e7c6fec3646b06c944a8e35d1a3de5c2c3 (HEAD, refs/bisect/bad)
>>> Author: Goldwyn Rodrigues <[email protected]>
>>> Date: Tue Jun 20 07:05:49 2017 -0500
>>>
>>> btrfs: nowait aio support
>>>
>>> apparently breaks several shell related features on my system.
>>
>> Here is a simple testcase:
>>
>> % echo "foo" >> test
>> % echo "foo" >> test
>> % cat test
>> foo
>> %
>>
>
> Thanks for testing.
> Yes, pos must be set with iocb->ki_pos for appends. I should not have
> removed the initialization. Could you try this patch?
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 59e2dccdf75b..7947781229e5 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1931,6 +1931,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb
> *iocb,
> */
> update_time_for_write(inode);
>
> + pos = iocb->ki_pos;
> start_pos = round_down(pos, fs_info->sectorsize);
> oldsize = i_size_read(inode);
> if (start_pos > oldsize) {

Please expedite getting this upstream, asap.

--
Jens Axboe

2017-07-08 01:51:22

by Goldwyn Rodrigues

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells



On 07/04/2017 05:16 PM, Jens Axboe wrote:
>
> Please expedite getting this upstream, asap.
>

Jens,

I have posted an updated patch [1] and it is acked by David. Would you
pick it up or should it go through the btrfs tree (or some other tree)?

[1] https://patchwork.kernel.org/patch/9825813/

--
Goldwyn

2017-07-08 02:09:35

by Jens Axboe

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

On 07/07/2017 07:51 PM, Goldwyn Rodrigues wrote:
>
>
> On 07/04/2017 05:16 PM, Jens Axboe wrote:
>>
>> Please expedite getting this upstream, asap.
>>
>
> Jens,
>
> I have posted an updated patch [1] and it is acked by David. Would you
> pick it up or should it go through the btrfs tree (or some other tree)?
>
> [1] https://patchwork.kernel.org/patch/9825813/

I'm fine with either, I just want it to go in asap. I'm sending off
a pull Monday. David, up to you.

--
Jens Axboe

2017-07-10 12:34:27

by David Sterba

[permalink] [raw]
Subject: Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

On Fri, Jul 07, 2017 at 08:09:28PM -0600, Jens Axboe wrote:
> On 07/07/2017 07:51 PM, Goldwyn Rodrigues wrote:
> > On 07/04/2017 05:16 PM, Jens Axboe wrote:
> >>
> >> Please expedite getting this upstream, asap.
> >
> > I have posted an updated patch [1] and it is acked by David. Would you
> > pick it up or should it go through the btrfs tree (or some other tree)?
> >
> > [1] https://patchwork.kernel.org/patch/9825813/
>
> I'm fine with either, I just want it to go in asap. I'm sending off
> a pull Monday. David, up to you.

I'm reading this just now, so I'll send the patch in my pull today.