Re: [PATCH v4] f2fs: fix zoned block device information initialization

[Chao Yu <chao@kernel.org>]

On 2024/2/4 10:18, Wenjie Qi wrote:
> Hi Chao,
> 
> It seems to me that when mounting multiple zoned devices,
> if their max_open_zones are all 0, then sbi->max_open_zones is 0.
> This suggests that all of the mounted devices can open an unlimited
> number of zones,
> and that we don't need to compare sbi->max_open_zones with
> F2FS_OPTION( sbi).active_logs.

Yes, but I'm curious about how this case (sbi->max_open_zones is zero)
works w/ following patch, do we need to initialized sbi->max_open_zones
w/ UINT_MAX to indicate the unlimited open zone status of device if
all zoned devices' max_open_zones is zero?

> 
> Thanks,
> 
> Chao Yu <chao@kernel.org> 于2024年2月4日周日 09:47写道：
>>
>> On 2024/2/3 23:24, Wenjie Qi wrote:
>>> If the max open zones of zoned devices are less than
>>> the active logs of F2FS, the device may error due to
>>> insufficient zone resources when multiple active logs are
>>> being written at the same time. If this value is 0,
>>> there is no limit.
>>>
>>> Signed-off-by: Wenjie Qi <qwjhust@gmail.com>
>>> ---
>>>    fs/f2fs/f2fs.h  |  1 +
>>>    fs/f2fs/super.c | 21 +++++++++++++++++++++
>>>    2 files changed, 22 insertions(+)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 543898482f8b..161107f2d3bd 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1558,6 +1558,7 @@ struct f2fs_sb_info {
>>>
>>>    #ifdef CONFIG_BLK_DEV_ZONED
>>>        unsigned int blocks_per_blkz;           /* F2FS blocks per zone */
>>> +     unsigned int max_open_zones;            /* max open zone resources of the zoned device */
>>>    #endif
>>>
>>>        /* for node-related operations */
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 1b718bebfaa1..45e82d6016fc 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -2388,6 +2388,16 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>>>        if (err)
>>>                goto restore_opts;
>>>
>>> +#ifdef CONFIG_BLK_DEV_ZONED
>>> +     if (sbi->max_open_zones && sbi->max_open_zones < F2FS_OPTION(sbi).active_logs) {
>>> +             f2fs_err(sbi,
>>> +                     "zoned: max open zones %u is too small, need at least %u open zones",
>>> +                              sbi->max_open_zones, F2FS_OPTION(sbi).active_logs);
>>> +             err = -EINVAL;
>>> +             goto restore_opts;
>>> +     }
>>> +#endif
>>> +
>>>        /* flush outstanding errors before changing fs state */
>>>        flush_work(&sbi->s_error_work);
>>>
>>> @@ -3930,11 +3940,22 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int devi)
>>>        sector_t nr_sectors = bdev_nr_sectors(bdev);
>>>        struct f2fs_report_zones_args rep_zone_arg;
>>>        u64 zone_sectors;
>>> +     unsigned int max_open_zones;
>>>        int ret;
>>>
>>>        if (!f2fs_sb_has_blkzoned(sbi))
>>>                return 0;
>>>
>>> +     max_open_zones = bdev_max_open_zones(bdev);
>>
>> Wenjie,
>>
>> max_open_zones can always be zero? then sbi->max_open_zones will be zero,
>> is this a valid case?
>>
>> Thanks,
>>
>>> +     if (max_open_zones && (max_open_zones < sbi->max_open_zones || !sbi->max_open_zones))
>>> +             sbi->max_open_zones = max_open_zones;
>>> +     if (sbi->max_open_zones && sbi->max_open_zones < F2FS_OPTION(sbi).active_logs) {
>>> +             f2fs_err(sbi,
>>> +                     "zoned: max open zones %u is too small, need at least %u open zones",
>>> +                              sbi->max_open_zones, F2FS_OPTION(sbi).active_logs);
>>> +             return -EINVAL;
>>> +     }
>>> +
>>>        zone_sectors = bdev_zone_sectors(bdev);
>>>        if (!is_power_of_2(zone_sectors)) {
>>>                f2fs_err(sbi, "F2FS does not support non power of 2 zone sizes\n");