| Message ID | 20230913111013.77623-10-hch@lst.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [01/19] fs: reflow deactivate_locked_super | expand |
On 9/13/23 20:10, Christoph Hellwig wrote: > When ->fill_super fails, ->kill_sb is called which already cleans up > the inodes and zgroups. > > Drop the extra cleanup code in zonefs_fill_super. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good to me. Acked-by: Damien Le Moal <dlemoal@kernel.org> > --- > fs/zonefs/super.c | 21 +++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-) > > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 9d1a9808fbbba6..35b2554ce2ac2e 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -1309,13 +1309,12 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) > /* Initialize the zone groups */ > ret = zonefs_init_zgroups(sb); > if (ret) > - goto cleanup; > + return ret; > > /* Create the root directory inode */ > - ret = -ENOMEM; > inode = new_inode(sb); > if (!inode) > - goto cleanup; > + return -ENOMEM; > > inode->i_ino = bdev_nr_zones(sb->s_bdev); > inode->i_mode = S_IFDIR | 0555; > @@ -1333,7 +1332,7 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) > > sb->s_root = d_make_root(inode); > if (!sb->s_root) > - goto cleanup; > + return -ENOMEM; > > /* > * Take a reference on the zone groups directory inodes > @@ -1341,19 +1340,9 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) > */ > ret = zonefs_get_zgroup_inodes(sb); > if (ret) > - goto cleanup; > - > - ret = zonefs_sysfs_register(sb); > - if (ret) > - goto cleanup; > - > - return 0; > - > -cleanup: > - zonefs_release_zgroup_inodes(sb); > - zonefs_free_zgroups(sb); > + return ret; > > - return ret; > + return zonefs_sysfs_register(sb); > } > > static struct dentry *zonefs_mount(struct file_system_type *fs_type,
On Wed, Sep 13, 2023 at 08:10:03AM -0300, Christoph Hellwig wrote: > When ->fill_super fails, ->kill_sb is called which already cleans up > the inodes and zgroups. Ugh... The use of "->" strongly suggests that you are talking about a method; 'fill_super' here actually refers to callback passed to mount_bdev(). Have a pity for those who'll be trying to parse it - that might be yourself a couple of years down the road... Something like "If zonefs_fill_super() returns an error, its caller (mount_bdev()) will make sure to call zonefs_kill_super(), which already cleans up the inodes and zgroups.", perhaps? > > Drop the extra cleanup code in zonefs_fill_super. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/zonefs/super.c | 21 +++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-) > > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 9d1a9808fbbba6..35b2554ce2ac2e 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -1309,13 +1309,12 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) > /* Initialize the zone groups */ > ret = zonefs_init_zgroups(sb); > if (ret) > - goto cleanup; > + return ret; > > /* Create the root directory inode */ > - ret = -ENOMEM; > inode = new_inode(sb); > if (!inode) > - goto cleanup; > + return -ENOMEM; > > inode->i_ino = bdev_nr_zones(sb->s_bdev); > inode->i_mode = S_IFDIR | 0555; > @@ -1333,7 +1332,7 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) > > sb->s_root = d_make_root(inode); > if (!sb->s_root) > - goto cleanup; > + return -ENOMEM; > > /* > * Take a reference on the zone groups directory inodes > @@ -1341,19 +1340,9 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) > */ > ret = zonefs_get_zgroup_inodes(sb); > if (ret) > - goto cleanup; > - > - ret = zonefs_sysfs_register(sb); > - if (ret) > - goto cleanup; > - > - return 0; > - > -cleanup: > - zonefs_release_zgroup_inodes(sb); > - zonefs_free_zgroups(sb); > + return ret; > > - return ret; > + return zonefs_sysfs_register(sb); > } > > static struct dentry *zonefs_mount(struct file_system_type *fs_type, > -- > 2.39.2 >
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c index 9d1a9808fbbba6..35b2554ce2ac2e 100644 --- a/fs/zonefs/super.c +++ b/fs/zonefs/super.c @@ -1309,13 +1309,12 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) /* Initialize the zone groups */ ret = zonefs_init_zgroups(sb); if (ret) - goto cleanup; + return ret; /* Create the root directory inode */ - ret = -ENOMEM; inode = new_inode(sb); if (!inode) - goto cleanup; + return -ENOMEM; inode->i_ino = bdev_nr_zones(sb->s_bdev); inode->i_mode = S_IFDIR | 0555; @@ -1333,7 +1332,7 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) sb->s_root = d_make_root(inode); if (!sb->s_root) - goto cleanup; + return -ENOMEM; /* * Take a reference on the zone groups directory inodes @@ -1341,19 +1340,9 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent) */ ret = zonefs_get_zgroup_inodes(sb); if (ret) - goto cleanup; - - ret = zonefs_sysfs_register(sb); - if (ret) - goto cleanup; - - return 0; - -cleanup: - zonefs_release_zgroup_inodes(sb); - zonefs_free_zgroups(sb); + return ret; - return ret; + return zonefs_sysfs_register(sb); } static struct dentry *zonefs_mount(struct file_system_type *fs_type,
When ->fill_super fails, ->kill_sb is called which already cleans up the inodes and zgroups. Drop the extra cleanup code in zonefs_fill_super. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/zonefs/super.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-)