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

Anand Jain
2022-03-16T08:44:27Z

On 16/03/2022 16:39, Johannes Thumshirn wrote:
> 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?
>

Well, not much. Write enough so to new chunks.

-------
nullb 4096 64 4 8
Created /dev/nullb0
mkfs.btrfs -f -q -dsingle -msingle /dev/nullb0
mount /dev/nullb0 /btrfs
dd if=/dev/zero of=/btrfs/tf1
dd: writing to '/btrfs/tf1': No space left on device
-------

Thanks, Anand

> [...]
>
>> 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