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

Anand Jain
2020-08-29T15:13:49Z

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>