diff mbox series

btrfs: start readahead also in seed devices

Message ID 20190606075444.15481-1-naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: start readahead also in seed devices | expand

Commit Message

Naohiro Aota June 6, 2019, 7:54 a.m. UTC
Currently, btrfs does not consult seed devices to start readahead. As a
result, if readahead zone is added to the seed devices, btrfs_reada_wait()
indefinitely wait for the reada_ctl to finish.

You can reproduce the hung by modifying btrfs/163 to have larger initial
file size (e.g. xfs_io pwrite 4M instead of current 256K).

Fixes: 7414a03fbf9e ("btrfs: initial readahead code and prototypes")
Cc: stable@vger.kernel.org # 3.2+: ce7791ffee1e: Btrfs: fix race between readahead and device replace/removal
Cc: stable@vger.kernel.org # 3.2+
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/reada.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Filipe Manana June 6, 2019, 11:14 a.m. UTC | #1
On Thu, Jun 6, 2019 at 8:56 AM Naohiro Aota <naohiro.aota@wdc.com> wrote:
>
> Currently, btrfs does not consult seed devices to start readahead. As a
> result, if readahead zone is added to the seed devices, btrfs_reada_wait()
> indefinitely wait for the reada_ctl to finish.
>
> You can reproduce the hung by modifying btrfs/163 to have larger initial
> file size (e.g. xfs_io pwrite 4M instead of current 256K).

Are you planning on submitting a patch for the test case as well, so
that it writes at least 4Mb?
Would be useful to have.

>
> Fixes: 7414a03fbf9e ("btrfs: initial readahead code and prototypes")
> Cc: stable@vger.kernel.org # 3.2+: ce7791ffee1e: Btrfs: fix race between readahead and device replace/removal
> Cc: stable@vger.kernel.org # 3.2+
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, thanks.

> ---
>  fs/btrfs/reada.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
> index 10d9589001a9..bb5bd49573b4 100644
> --- a/fs/btrfs/reada.c
> +++ b/fs/btrfs/reada.c
> @@ -747,6 +747,7 @@ static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>         u64 total = 0;
>         int i;
>
> +again:
>         do {
>                 enqueued = 0;
>                 mutex_lock(&fs_devices->device_list_mutex);
> @@ -758,6 +759,10 @@ static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>                 mutex_unlock(&fs_devices->device_list_mutex);
>                 total += enqueued;
>         } while (enqueued && total < 10000);
> +       if (fs_devices->seed) {
> +               fs_devices = fs_devices->seed;
> +               goto again;
> +       }
>
>         if (enqueued == 0)
>                 return;
> --
> 2.21.0
>
Naohiro Aota June 7, 2019, 2:29 a.m. UTC | #2
On 2019/06/06 20:14, Filipe Manana wrote:
> On Thu, Jun 6, 2019 at 8:56 AM Naohiro Aota <naohiro.aota@wdc.com> wrote:
>>
>> Currently, btrfs does not consult seed devices to start readahead. As a
>> result, if readahead zone is added to the seed devices, btrfs_reada_wait()
>> indefinitely wait for the reada_ctl to finish.
>>
>> You can reproduce the hung by modifying btrfs/163 to have larger initial
>> file size (e.g. xfs_io pwrite 4M instead of current 256K).
> 
> Are you planning on submitting a patch for the test case as well, so
> that it writes at least 4Mb?
> Would be useful to have.

I will send that patch soon.

Thanks,

> 
>>
>> Fixes: 7414a03fbf9e ("btrfs: initial readahead code and prototypes")
>> Cc: stable@vger.kernel.org # 3.2+: ce7791ffee1e: Btrfs: fix race between readahead and device replace/removal
>> Cc: stable@vger.kernel.org # 3.2+
>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> 
> Looks good, thanks.
> 
>> ---
>>   fs/btrfs/reada.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
>> index 10d9589001a9..bb5bd49573b4 100644
>> --- a/fs/btrfs/reada.c
>> +++ b/fs/btrfs/reada.c
>> @@ -747,6 +747,7 @@ static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>>          u64 total = 0;
>>          int i;
>>
>> +again:
>>          do {
>>                  enqueued = 0;
>>                  mutex_lock(&fs_devices->device_list_mutex);
>> @@ -758,6 +759,10 @@ static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>>                  mutex_unlock(&fs_devices->device_list_mutex);
>>                  total += enqueued;
>>          } while (enqueued && total < 10000);
>> +       if (fs_devices->seed) {
>> +               fs_devices = fs_devices->seed;
>> +               goto again;
>> +       }
>>
>>          if (enqueued == 0)
>>                  return;
>> --
>> 2.21.0
>>
> 
>
David Sterba June 7, 2019, 1:39 p.m. UTC | #3
On Thu, Jun 06, 2019 at 04:54:44PM +0900, Naohiro Aota wrote:
> Currently, btrfs does not consult seed devices to start readahead. As a
> result, if readahead zone is added to the seed devices, btrfs_reada_wait()
> indefinitely wait for the reada_ctl to finish.
> 
> You can reproduce the hung by modifying btrfs/163 to have larger initial
> file size (e.g. xfs_io pwrite 4M instead of current 256K).
> 
> Fixes: 7414a03fbf9e ("btrfs: initial readahead code and prototypes")
> Cc: stable@vger.kernel.org # 3.2+: ce7791ffee1e: Btrfs: fix race between readahead and device replace/removal
> Cc: stable@vger.kernel.org # 3.2+
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>

Thanks, added to misc-next.
diff mbox series

Patch

diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
index 10d9589001a9..bb5bd49573b4 100644
--- a/fs/btrfs/reada.c
+++ b/fs/btrfs/reada.c
@@ -747,6 +747,7 @@  static void __reada_start_machine(struct btrfs_fs_info *fs_info)
 	u64 total = 0;
 	int i;
 
+again:
 	do {
 		enqueued = 0;
 		mutex_lock(&fs_devices->device_list_mutex);
@@ -758,6 +759,10 @@  static void __reada_start_machine(struct btrfs_fs_info *fs_info)
 		mutex_unlock(&fs_devices->device_list_mutex);
 		total += enqueued;
 	} while (enqueued && total < 10000);
+	if (fs_devices->seed) {
+		fs_devices = fs_devices->seed;
+		goto again;
+	}
 
 	if (enqueued == 0)
 		return;