On 11/4/22 18:47, Johannes Thumshirn wrote:
> On 04.11.22 01:26, Damien Le Moal wrote:
>> On 11/3/22 19:32, Johannes Thumshirn wrote:
>>> When initializing a file inode, check if the zone's size if bigger than
>>> the number of device zone sectors. This can only be the case if we mount
>>> the filesystem with the -oaggr_cnv mount option.
>>>
>>> Emit an error in case this case happens and fail the mount.
>>>
>>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
>>> ---
>>> fs/zonefs/super.c | 27 +++++++++++++++++++++------
>>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
>>> index 860f0b1032c6..605364638720 100644
>>> --- a/fs/zonefs/super.c
>>> +++ b/fs/zonefs/super.c
>>> @@ -1407,6 +1407,14 @@ static int zonefs_init_file_inode(struct inode *inode, struct blk_zone *zone,
>>> zi->i_ztype = type;
>>> zi->i_zsector = zone->start;
>>> zi->i_zone_size = zone->len << SECTOR_SHIFT;
>>> + if (zi->i_zone_size > bdev_zone_sectors(sb->s_bdev) << SECTOR_SHIFT &&
>>> + !sbi->s_features & ZONEFS_F_AGGRCNV) {
>>> + zonefs_err(sb,
>>> + "zone size %llu doesn't match device's zone sectors %llu\n",
>>> + zi->i_zone_size,
>>> + bdev_zone_sectors(sb->s_bdev) << SECTOR_SHIFT);
>>> + return -EINVAL;
>>> + }
>>>
>>> zi->i_max_size = min_t(loff_t, MAX_LFS_FILESIZE,
>>> zone->capacity << SECTOR_SHIFT);
>>> @@ -1485,7 +1493,7 @@ static struct dentry *zonefs_create_inode(struct dentry *parent,
>>> dput:
>>> dput(dentry);
>>>
>>> - return NULL;
>>> + return ERR_PTR(ret);
>>> }
>>>
>>> struct zonefs_zone_data {
>>> @@ -1505,7 +1513,7 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd,
>>> struct blk_zone *zone, *next, *end;
>>> const char *zgroup_name;
>>> char *file_name;
>>> - struct dentry *dir;
>>> + struct dentry *dir, *ret2;
>>> unsigned int n = 0;
>>> int ret;
>>>
>>> @@ -1523,8 +1531,11 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd,
>>> zgroup_name = "seq";
>>>
>>> dir = zonefs_create_inode(sb->s_root, zgroup_name, NULL, type);
>>> - if (!dir) {
>>> - ret = -ENOMEM;
>>> + if (IS_ERR_OR_NULL(dir)) {
>>> + if (!dir)
>>> + ret = -ENOMEM;
>>
>> It would be cleaner to return ERR_PTR(-ENOMEM) instead of NULL in
>> zonefs_create_inode(). This way, this can simply be:
>> if (IS_ERR(dir)) {
>> ret = PTR_ERR(dir);
>> goto free;
>> }
>>
>> And the hunk below would be similar too.
>>
>
> Agreed, I'll update the patch. Or do you want to do it when squashing it
> into your "zonefs: fix zone report size in __zonefs_io_error()" patch?
If you can do the squashing, that would be nice too :)
--
Damien Le Moal
Western Digital Research
