Re: [PATCH] btrfs: zoned: use alloc_list instead of rcu locked device_list

Johannes Thumshirn
2022-03-16T08:39:35Z

On 16/03/2022 09:37, Anand Jain wrote:
> On 16/03/2022 04:03, David Sterba wrote:
>> On Mon, Mar 14, 2022 at 07:16:47AM -0700, Johannes Thumshirn wrote:
>>> Anand pointed out, that instead of using the rcu locked version of
>>> fs_devices->device_list we cab use fs_devices->alloc_list, prrotected by
>>> the chunk_mutex to traverse the list of active deviices in
>>> btrfs_can_activate_zone().
>>>
>>> Suggested-by: Anand Jain <anand.jain@oracle.com>
>>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>
>> With updated changelog added to misc-next, thanks.
>
>
> Misc-next hit an NPD for new chunk alloc.

Thanks for the change, but how did you trigger it? A.k.a why didn't it
trigger for me?

[...]

> We should use fs_devices->alloc_list with dev_alloc_list as below.
> Also, missing devices aren't part of dev_alloc_list, so we don't have
> to check for if (!device->bdev).
>
> --------------
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 7da630ff5708..c29e67c621be 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1984,12 +1984,9 @@ bool btrfs_can_activate_zone(struct
> btrfs_fs_devices *fs_devices, u64 flags)
>
> /* Check if there is a device with active zones left */
> mutex_lock(&fs_info->chunk_mutex);
> - list_for_each_entry(device, &fs_devices->alloc_list, dev_list) {
> + list_for_each_entry(device, &fs_devices->alloc_list,
> dev_alloc_list) {
> struct btrfs_zoned_device_info *zinfo = device->zone_info;
>
> - if (!device->bdev)
> - continue;
> -
> if (!zinfo->max_active_zones ||
> atomic_read(&zinfo->active_zones_left)) {
> ret = true;
> -------------
>
> With this fixed you may add
>
> Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks I'll update the patch