Re: [PATCH 1/5] btrfs: Factor out reada loop in __reada_start_machine

[Anand Jain <anand.jain@oracle.com>]

On 15/7/20 6:48 pm, Nikolay Borisov wrote:
> This is in preparation for moving fs_devices to proper lists.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>   fs/btrfs/reada.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
> index 243a2e44526e..aa9d24ed56d7 100644
> --- a/fs/btrfs/reada.c
> +++ b/fs/btrfs/reada.c
> @@ -767,15 +767,14 @@ static void reada_start_machine_worker(struct btrfs_work *work)
>   	kfree(rmw);
>   }
>   
> -static void __reada_start_machine(struct btrfs_fs_info *fs_info)
> +
> +/* Try to start up to 10k READA requests for a group of devices. */
> +static int __reada_start_for_fsdevs(struct btrfs_fs_devices *fs_devices)

  In my experience David doesn't prefer __ prefix for helper functions.

>   {
> -	struct btrfs_device *device;
> -	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>   	u64 enqueued;
>   	u64 total = 0;
> -	int i;
> +	struct btrfs_device *device;
>   
> -again:
>   	do {
>   		enqueued = 0;
>   		mutex_lock(&fs_devices->device_list_mutex);
> @@ -787,6 +786,18 @@ static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>   		mutex_unlock(&fs_devices->device_list_mutex);
>   		total += enqueued;
>   	} while (enqueued && total < 10000);
> +
> +	return total;
> +}
> +
> +static void __reada_start_machine(struct btrfs_fs_info *fs_info)
> +{
> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> +	int i;
> +	u64 enqueued = 0;
> +
> +again:
> +	enqueued += __reada_start_for_fsdevs(fs_devices);
>   	if (fs_devices->seed) {
>   		fs_devices = fs_devices->seed;
>   		goto again;
> 

There wasn't any need to create another helper function here IMO.

Anyway changes looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>