Re: [PATCH v2] btrfs: Factor out loop logic from btrfs_free_extra_devids

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

On 16/7/20 3:17 pm, Nikolay Borisov wrote:
> This prepares the code to switching seeds devices to a proper list.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> 
> V2:
>   * Added missing static modifier to the factored out function. Reported by
>   kernel test robot
> 
>   fs/btrfs/volumes.c | 34 +++++++++++++++++++++-------------
>   1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index ce01e44f8134..76a68edb3127 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1024,28 +1024,24 @@ static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig)
>   	return ERR_PTR(ret);
>   }
> 
> -/*
> - * After we have read the system tree and know devids belonging to
> - * this filesystem, remove the device which does not belong there.
> - */
> -void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step)
> +
> +
> +static void __btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices,
> +				      int step, struct btrfs_device **latest_dev)

David doesn't prefer __ prefix for helpers.

>   {
>   	struct btrfs_device *device, *next;
> -	struct btrfs_device *latest_dev = NULL;
> 
> -	mutex_lock(&uuid_mutex);
> -again:
>   	/* This is the initialized path, it is safe to release the devices. */
>   	list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) {
>   		if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
> -							&device->dev_state)) {
> +			     &device->dev_state)) {
>   			if (!test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
> -			     &device->dev_state) &&
> +				      &device->dev_state) &&
>   			    !test_bit(BTRFS_DEV_STATE_MISSING,
>   				      &device->dev_state) &&
> -			     (!latest_dev ||
> -			      device->generation > latest_dev->generation)) {
> -				latest_dev = device;
> +			    (!*latest_dev ||
> +			     device->generation > (*latest_dev)->generation)) {
> +				*latest_dev = device;
>   			}
>   			continue;
>   		}
> @@ -1083,6 +1079,18 @@ void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step)
>   		btrfs_free_device(device);
>   	}
> 
> +}
> +/*
> + * After we have read the system tree and know devids belonging to
> + * this filesystem, remove the device which does not belong there.
> + */
> +void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step)
> +{
> +	struct btrfs_device *latest_dev = NULL;
> +
> +	mutex_lock(&uuid_mutex);
> +again:
> +	__btrfs_free_extra_devids(fs_devices, step, &latest_dev);


>   	if (fs_devices->seed) {
>   		fs_devices = fs_devices->seed;
>   		goto again;
> --
> 2.17.1
> 


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