Skip to contentSkip to conversation

Commit edf064e7c (btrfs: nowait aio support) breaks shells

PATCH Markus Trippelsdorf 4 participants 8 messages Jul 4, 2017 – Jul 10, 2017

Subsystems (2)
Block layer Btrfs file system
lore
Share & tools
lore

Markus Trippelsdorf

author
#0 lore raw

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_bad.h 2017-07-04 05:51:00.650730726 +0200 renamed from help_mp_good.h 2017-07-04 05:38:33.161640826 +0200 +0 -2
@@ -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
Signature · 1 line
Markus
author
#1 lore raw
Quoted 7 lines · Markus Trippelsdorf

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
%

Signature · 1 line
Markus
#2 lore raw
Quoted 9 lines · Markus Trippelsdorf
Quoted 7 lines · Markus Trippelsdorf

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.

Quoted 8 lines

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?

fs/btrfs/file.c +0 -0
index 59e2dccdf75b..7947781229e5 100644
@@ -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) {

Signature · 1 line
Goldwyn
author
#3 lore raw
Quoted 21 lines · Goldwyn Rodrigues
Quoted 9 lines · Markus Trippelsdorf
Quoted 7 lines · Markus Trippelsdorf

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.

Quoted 8 lines

Here is a simple testcase:

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

Quoted 3 lines

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.

Signature · 1 line
Markus
maintainer
#4 lore raw
In reply to Goldwyn Rodrigues
Quoted 37 lines · Goldwyn Rodrigues
Quoted 9 lines · Markus Trippelsdorf
Quoted 7 lines · Markus Trippelsdorf

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.

Quoted 8 lines

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?

fs/btrfs/file.c +1 -0
index 59e2dccdf75b..7947781229e5 100644
@@ -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);
Quoted 1 line

if (start_pos > oldsize) {

Please expedite getting this upstream, asap.

Signature · 1 line
Jens Axboe
#5 lore raw
Quoted 3 lines · Jens Axboe

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/

Signature · 1 line
Goldwyn
maintainer
#6 lore raw
Quoted 7 lines · Goldwyn Rodrigues
Quoted 3 lines · Jens Axboe

Please expedite getting this upstream, asap.

Quoted 6 lines

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.

Signature · 1 line
Jens Axboe
#7 lore raw
Quoted 5 lines · Jens Axboe
Quoted 4 lines · Goldwyn Rodrigues
Quoted 2 lines · Jens Axboe

Please expedite getting this upstream, asap.

Quoted 7 lines
Quoted 4 lines

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.