diff mbox series

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

Message ID 20200715104850.19071-2-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Convert seed devices to proper list API | expand

Commit Message

Nikolay Borisov July 15, 2020, 10:48 a.m. UTC
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(-)

Comments

Josef Bacik Aug. 18, 2020, 3:02 p.m. UTC | #1
On 7/15/20 6:48 AM, Nikolay Borisov wrote:
> This is in preparation for moving fs_devices to proper lists.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
Anand Jain Aug. 29, 2020, 3:06 p.m. UTC | #2
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>
David Sterba Aug. 31, 2020, 12:24 p.m. UTC | #3
On Sat, Aug 29, 2020 at 11:06:32PM +0800, Anand Jain wrote:
> 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.

Right, I've updated the patch to drop the __ as there's no other
function without the underscores (unlike __reada_start_machine/reada_start_machine)
diff mbox series

Patch

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)
 {
-	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;