On 12/11/2020 08:00, Anand Jain wrote:
>
>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 8840a4fa81eb..ed55014fd1bd 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -2462,6 +2462,11 @@ static void __init btrfs_print_mod_info(void)
>> #endif
>> #ifdef CONFIG_BTRFS_FS_REF_VERIFY
>> ", ref-verify=on"
>> +#endif
>> +#ifdef CONFIG_BLK_DEV_ZONED
>> + ", zoned=yes"
>> +#else
>> + ", zoned=no"
>> #endif
>
> IMO, we don't need this, as most of the generic kernel will be compiled
> with the CONFIG_BLK_DEV_ZONED defined.
> For review purpose we may want to know if the mounted device
> is a zoned device. So log of zone device and its type may be useful
> when we have verified the zoned devices in the open_ctree().
>
>> @@ -374,6 +375,7 @@ void btrfs_free_device(struct btrfs_device *device)
>> rcu_string_free(device->name);
>> extent_io_tree_release(&device->alloc_state);
>> bio_put(device->flush_bio);
>
>> + btrfs_destroy_dev_zone_info(device);
>
> Free of btrfs_device::zone_info is already happening in the path..
>
> btrfs_close_one_device()
> btrfs_destroy_dev_zone_info()
>
> We don't need this..
>
> btrfs_free_device()
> btrfs_destroy_dev_zone_info()
>
>
>> @@ -2543,6 +2551,14 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>> }
>> rcu_assign_pointer(device->name, name);
>>
>> + device->fs_info = fs_info;
>> + device->bdev = bdev;
>> +
>> + /* Get zone type information of zoned block devices */
>> + ret = btrfs_get_dev_zone_info(device);
>> + if (ret)
>> + goto error_free_device;
>> +
>> trans = btrfs_start_transaction(root, 0);
>> if (IS_ERR(trans)) {
>> ret = PTR_ERR(trans);
>
> It should be something like goto error_free_zone from here.
>
>
>> @@ -2707,6 +2721,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>> sb->s_flags |= SB_RDONLY;
>> if (trans)
>> btrfs_end_transaction(trans);
>
>
> error_free_zone:
>> + btrfs_destroy_dev_zone_info(device);
>> error_free_device:
>> btrfs_free_device(device);
>> error:
>
> As mentioned we don't need btrfs_destroy_dev_zone_info()
> again in btrfs_free_device(). Otherwise we end up calling
> btrfs_destroy_dev_zone_info twice here.
Which doesn't do any harm as:
void btrfs_destroy_dev_zone_info(struct btrfs_device *device)
{
struct btrfs_zoned_device_info *zone_info = device->zone_info;
if (!zone_info)
return;
/* ... */
device->zone_info = NULL;
}
Not sure what would be the preferred style here
