| Message ID | 20240220164729.179594-1-bodonnel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] efs: convert efs to use the new mount api | expand |
On 2/20/24 8:45 AM, Bill O'Donnell wrote: > Convert the efs filesystem to use the new mount API. > > Signed-off-by: Bill O'Donnell <bodonnel@redhat.com> > --- > > Changelog: > v2: Remove efs_param_spec and efs_parse_param, since no mount options. A few more items below > --- > fs/efs/super.c | 91 +++++++++++++++++++++++++++++++++----------------- > 1 file changed, 61 insertions(+), 30 deletions(-) > > diff --git a/fs/efs/super.c b/fs/efs/super.c > index f17fdac76b2e..d86c84e9e497 100644 > --- a/fs/efs/super.c > +++ b/fs/efs/super.c > @@ -14,19 +14,13 @@ > #include <linux/buffer_head.h> > #include <linux/vfs.h> > #include <linux/blkdev.h> > - > +#include <linux/fs_context.h> > #include "efs.h" > #include <linux/efs_vh.h> > #include <linux/efs_fs_sb.h> > > static int efs_statfs(struct dentry *dentry, struct kstatfs *buf); > -static int efs_fill_super(struct super_block *s, void *d, int silent); > - > -static struct dentry *efs_mount(struct file_system_type *fs_type, > - int flags, const char *dev_name, void *data) > -{ > - return mount_bdev(fs_type, flags, dev_name, data, efs_fill_super); > -} > +static int efs_init_fs_context(struct fs_context *fc); > > static void efs_kill_sb(struct super_block *s) > { > @@ -35,15 +29,6 @@ static void efs_kill_sb(struct super_block *s) > kfree(sbi); > } > > -static struct file_system_type efs_fs_type = { > - .owner = THIS_MODULE, > - .name = "efs", > - .mount = efs_mount, > - .kill_sb = efs_kill_sb, > - .fs_flags = FS_REQUIRES_DEV, > -}; > -MODULE_ALIAS_FS("efs"); > - > static struct pt_types sgi_pt_types[] = { > {0x00, "SGI vh"}, > {0x01, "SGI trkrepl"}, > @@ -63,6 +48,17 @@ static struct pt_types sgi_pt_types[] = { > {0, NULL} > }; > > +/* > + * File system definition and registration. > + */ > +static struct file_system_type efs_fs_type = { > + .owner = THIS_MODULE, > + .name = "efs", > + .kill_sb = efs_kill_sb, > + .fs_flags = FS_REQUIRES_DEV, > + .init_fs_context = efs_init_fs_context, > +}; > +MODULE_ALIAS_FS("efs"); > > static struct kmem_cache * efs_inode_cachep; > > @@ -108,18 +104,10 @@ static void destroy_inodecache(void) > kmem_cache_destroy(efs_inode_cachep); > } > > -static int efs_remount(struct super_block *sb, int *flags, char *data) > -{ > - sync_filesystem(sb); > - *flags |= SB_RDONLY; > - return 0; > -} > - > static const struct super_operations efs_superblock_operations = { > .alloc_inode = efs_alloc_inode, > .free_inode = efs_free_inode, > .statfs = efs_statfs, > - .remount_fs = efs_remount, > }; > > static const struct export_operations efs_export_ops = { > @@ -249,26 +237,26 @@ static int efs_validate_super(struct efs_sb_info *sb, struct efs_super *super) { > return 0; > } > > -static int efs_fill_super(struct super_block *s, void *d, int silent) > +static int efs_fill_super(struct super_block *s, struct fs_context *fc) > { > struct efs_sb_info *sb; > struct buffer_head *bh; > struct inode *root; > > - sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL); > + sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL); Ok, I guess this and elsewhere is fixing up whitespace oddities, not adding them. :) > if (!sb) > return -ENOMEM; > s->s_fs_info = sb; > s->s_time_min = 0; > s->s_time_max = U32_MAX; > - > + > s->s_magic = EFS_SUPER_MAGIC; > if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) { > pr_err("device does not support %d byte blocks\n", > EFS_BLOCKSIZE); > return -EINVAL; I think this can (should?) be converted to: return invalf(fc, "device does not support %d byte blocks", EFS_BLOCKSIZE); and similarly for other error printing failures along the fill_super path, with appropriate variants of invalf()/errorf()/warnf()/etc (dhowells - am I right about this?) > } > - > + > /* read the vh (volume header) block */ > bh = sb_bread(s, 0); > > @@ -294,7 +282,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent) > pr_err("cannot read superblock\n"); > return -EIO; > } > - > + > if (efs_validate_super(sb, (struct efs_super *) bh->b_data)) { > #ifdef DEBUG > pr_warn("invalid superblock at block %u\n", > @@ -328,6 +316,49 @@ static int efs_fill_super(struct super_block *s, void *d, int silent) > return 0; > } > > +static void efs_free_fc(struct fs_context *fc) > +{ > + kfree(fc->fs_private); > +} unneeded; see below > +static int efs_get_tree(struct fs_context *fc) > +{ > + return get_tree_bdev(fc, efs_fill_super); > +} > + > +static int efs_reconfigure(struct fs_context *fc) > +{ > + sync_filesystem(fc->root->d_sb); I think you need: fc->sb_flags |= SB_RDONLY; here to preserve the original behavior in efs_remount() > + > + return 0; > +} > + > +struct efs_context { > + unsigned long s_mount_opts; > +}; This looks unused, and probably also copied from zonefs, which used it to store mount options - something efs doesn't have. > + > +static const struct fs_context_operations efs_context_opts = { > + .get_tree = efs_get_tree, > + .reconfigure = efs_reconfigure, > + .free = efs_free_fc, > +}; > + > +/* > + * Set up the filesystem mount context. > + */ > +static int efs_init_fs_context(struct fs_context *fc) > +{ > + struct efs_context *ctx; > + > + ctx = kzalloc(sizeof(struct efs_context), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + fc->fs_private = ctx; so there's no reason to allocate and assign it here. which means efs_free_fc() doesn't need to exist either. > + fc->ops = &efs_context_opts; > + > + return 0; > +} > + > static int efs_statfs(struct dentry *dentry, struct kstatfs *buf) { > struct super_block *sb = dentry->d_sb; > struct efs_sb_info *sbi = SUPER_INFO(sb);
On Tue, Feb 20, 2024 at 03:05:38PM -0600, Eric Sandeen wrote: > On 2/20/24 8:45 AM, Bill O'Donnell wrote: > > Convert the efs filesystem to use the new mount API. > > > > Signed-off-by: Bill O'Donnell <bodonnel@redhat.com> > > --- > > > > Changelog: > > v2: Remove efs_param_spec and efs_parse_param, since no mount options. > > A few more items below > > > --- > > fs/efs/super.c | 91 +++++++++++++++++++++++++++++++++----------------- > > 1 file changed, 61 insertions(+), 30 deletions(-) > > > > diff --git a/fs/efs/super.c b/fs/efs/super.c > > index f17fdac76b2e..d86c84e9e497 100644 > > --- a/fs/efs/super.c > > +++ b/fs/efs/super.c > > @@ -14,19 +14,13 @@ > > #include <linux/buffer_head.h> > > #include <linux/vfs.h> > > #include <linux/blkdev.h> > > - > > +#include <linux/fs_context.h> > > #include "efs.h" > > #include <linux/efs_vh.h> > > #include <linux/efs_fs_sb.h> > > > > static int efs_statfs(struct dentry *dentry, struct kstatfs *buf); > > -static int efs_fill_super(struct super_block *s, void *d, int silent); > > - > > -static struct dentry *efs_mount(struct file_system_type *fs_type, > > - int flags, const char *dev_name, void *data) > > -{ > > - return mount_bdev(fs_type, flags, dev_name, data, efs_fill_super); > > -} > > +static int efs_init_fs_context(struct fs_context *fc); > > > > static void efs_kill_sb(struct super_block *s) > > { > > @@ -35,15 +29,6 @@ static void efs_kill_sb(struct super_block *s) > > kfree(sbi); > > } > > > > -static struct file_system_type efs_fs_type = { > > - .owner = THIS_MODULE, > > - .name = "efs", > > - .mount = efs_mount, > > - .kill_sb = efs_kill_sb, > > - .fs_flags = FS_REQUIRES_DEV, > > -}; > > -MODULE_ALIAS_FS("efs"); > > - > > static struct pt_types sgi_pt_types[] = { > > {0x00, "SGI vh"}, > > {0x01, "SGI trkrepl"}, > > @@ -63,6 +48,17 @@ static struct pt_types sgi_pt_types[] = { > > {0, NULL} > > }; > > > > +/* > > + * File system definition and registration. > > + */ > > +static struct file_system_type efs_fs_type = { > > + .owner = THIS_MODULE, > > + .name = "efs", > > + .kill_sb = efs_kill_sb, > > + .fs_flags = FS_REQUIRES_DEV, > > + .init_fs_context = efs_init_fs_context, > > +}; > > +MODULE_ALIAS_FS("efs"); > > > > static struct kmem_cache * efs_inode_cachep; > > > > @@ -108,18 +104,10 @@ static void destroy_inodecache(void) > > kmem_cache_destroy(efs_inode_cachep); > > } > > > > -static int efs_remount(struct super_block *sb, int *flags, char *data) > > -{ > > - sync_filesystem(sb); > > - *flags |= SB_RDONLY; > > - return 0; > > -} > > - > > static const struct super_operations efs_superblock_operations = { > > .alloc_inode = efs_alloc_inode, > > .free_inode = efs_free_inode, > > .statfs = efs_statfs, > > - .remount_fs = efs_remount, > > }; > > > > static const struct export_operations efs_export_ops = { > > @@ -249,26 +237,26 @@ static int efs_validate_super(struct efs_sb_info *sb, struct efs_super *super) { > > return 0; > > } > > > > -static int efs_fill_super(struct super_block *s, void *d, int silent) > > +static int efs_fill_super(struct super_block *s, struct fs_context *fc) > > { > > struct efs_sb_info *sb; > > struct buffer_head *bh; > > struct inode *root; > > > > - sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL); > > + sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL); > > Ok, I guess this and elsewhere is fixing up whitespace oddities, > not adding them. :) Yeah, I fixed some tabs to spaces whitespace, when I was in that area of code. > > > if (!sb) > > return -ENOMEM; > > s->s_fs_info = sb; > > s->s_time_min = 0; > > s->s_time_max = U32_MAX; > > - > > + > > s->s_magic = EFS_SUPER_MAGIC; > > if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) { > > pr_err("device does not support %d byte blocks\n", > > EFS_BLOCKSIZE); > > return -EINVAL; > > I think this can (should?) be converted to: > > return invalf(fc, > "device does not support %d byte blocks", > EFS_BLOCKSIZE); > > and similarly for other error printing failures along the fill_super path, > with appropriate variants of invalf()/errorf()/warnf()/etc > > (dhowells - am I right about this?) I'm still looking at this. > > > } > > - > > + > > /* read the vh (volume header) block */ > > bh = sb_bread(s, 0); > > > > @@ -294,7 +282,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent) > > pr_err("cannot read superblock\n"); > > return -EIO; > > } > > - > > + > > if (efs_validate_super(sb, (struct efs_super *) bh->b_data)) { > > #ifdef DEBUG > > pr_warn("invalid superblock at block %u\n", > > @@ -328,6 +316,49 @@ static int efs_fill_super(struct super_block *s, void *d, int silent) > > return 0; > > } > > > > +static void efs_free_fc(struct fs_context *fc) > > +{ > > + kfree(fc->fs_private); > > +} > > unneeded; see below Agreed. > > > +static int efs_get_tree(struct fs_context *fc) > > +{ > > + return get_tree_bdev(fc, efs_fill_super); > > +} > > + > > +static int efs_reconfigure(struct fs_context *fc) > > +{ > > + sync_filesystem(fc->root->d_sb); > > I think you need: > > fc->sb_flags |= SB_RDONLY; > > here to preserve the original behavior in efs_remount() Good catch. I had noticed that /proc/mounts changed the permission to rw, but not behaving as rw. > > > + > > + return 0; > > +} > > + > > +struct efs_context { > > + unsigned long s_mount_opts; > > +}; > > This looks unused, and probably also copied from zonefs, which used it > to store mount options - something efs doesn't have. Agreed. > > > + > > +static const struct fs_context_operations efs_context_opts = { > > + .get_tree = efs_get_tree, > > + .reconfigure = efs_reconfigure, > > + .free = efs_free_fc, > > +}; > > + > > +/* > > + * Set up the filesystem mount context. > > + */ > > +static int efs_init_fs_context(struct fs_context *fc) > > +{ > > + struct efs_context *ctx; > > + > > + ctx = kzalloc(sizeof(struct efs_context), GFP_KERNEL); > > + if (!ctx) > > + return -ENOMEM; > > + fc->fs_private = ctx; > > so there's no reason to allocate and assign it here. > which means efs_free_fc() doesn't need to exist either. Agreed. > > > + fc->ops = &efs_context_opts; > > + > > + return 0; > > +} > > + > > static int efs_statfs(struct dentry *dentry, struct kstatfs *buf) { > > struct super_block *sb = dentry->d_sb; > > struct efs_sb_info *sbi = SUPER_INFO(sb); >
diff --git a/fs/efs/super.c b/fs/efs/super.c index f17fdac76b2e..d86c84e9e497 100644 --- a/fs/efs/super.c +++ b/fs/efs/super.c @@ -14,19 +14,13 @@ #include <linux/buffer_head.h> #include <linux/vfs.h> #include <linux/blkdev.h> - +#include <linux/fs_context.h> #include "efs.h" #include <linux/efs_vh.h> #include <linux/efs_fs_sb.h> static int efs_statfs(struct dentry *dentry, struct kstatfs *buf); -static int efs_fill_super(struct super_block *s, void *d, int silent); - -static struct dentry *efs_mount(struct file_system_type *fs_type, - int flags, const char *dev_name, void *data) -{ - return mount_bdev(fs_type, flags, dev_name, data, efs_fill_super); -} +static int efs_init_fs_context(struct fs_context *fc); static void efs_kill_sb(struct super_block *s) { @@ -35,15 +29,6 @@ static void efs_kill_sb(struct super_block *s) kfree(sbi); } -static struct file_system_type efs_fs_type = { - .owner = THIS_MODULE, - .name = "efs", - .mount = efs_mount, - .kill_sb = efs_kill_sb, - .fs_flags = FS_REQUIRES_DEV, -}; -MODULE_ALIAS_FS("efs"); - static struct pt_types sgi_pt_types[] = { {0x00, "SGI vh"}, {0x01, "SGI trkrepl"}, @@ -63,6 +48,17 @@ static struct pt_types sgi_pt_types[] = { {0, NULL} }; +/* + * File system definition and registration. + */ +static struct file_system_type efs_fs_type = { + .owner = THIS_MODULE, + .name = "efs", + .kill_sb = efs_kill_sb, + .fs_flags = FS_REQUIRES_DEV, + .init_fs_context = efs_init_fs_context, +}; +MODULE_ALIAS_FS("efs"); static struct kmem_cache * efs_inode_cachep; @@ -108,18 +104,10 @@ static void destroy_inodecache(void) kmem_cache_destroy(efs_inode_cachep); } -static int efs_remount(struct super_block *sb, int *flags, char *data) -{ - sync_filesystem(sb); - *flags |= SB_RDONLY; - return 0; -} - static const struct super_operations efs_superblock_operations = { .alloc_inode = efs_alloc_inode, .free_inode = efs_free_inode, .statfs = efs_statfs, - .remount_fs = efs_remount, }; static const struct export_operations efs_export_ops = { @@ -249,26 +237,26 @@ static int efs_validate_super(struct efs_sb_info *sb, struct efs_super *super) { return 0; } -static int efs_fill_super(struct super_block *s, void *d, int silent) +static int efs_fill_super(struct super_block *s, struct fs_context *fc) { struct efs_sb_info *sb; struct buffer_head *bh; struct inode *root; - sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL); + sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL); if (!sb) return -ENOMEM; s->s_fs_info = sb; s->s_time_min = 0; s->s_time_max = U32_MAX; - + s->s_magic = EFS_SUPER_MAGIC; if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) { pr_err("device does not support %d byte blocks\n", EFS_BLOCKSIZE); return -EINVAL; } - + /* read the vh (volume header) block */ bh = sb_bread(s, 0); @@ -294,7 +282,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent) pr_err("cannot read superblock\n"); return -EIO; } - + if (efs_validate_super(sb, (struct efs_super *) bh->b_data)) { #ifdef DEBUG pr_warn("invalid superblock at block %u\n", @@ -328,6 +316,49 @@ static int efs_fill_super(struct super_block *s, void *d, int silent) return 0; } +static void efs_free_fc(struct fs_context *fc) +{ + kfree(fc->fs_private); +} + +static int efs_get_tree(struct fs_context *fc) +{ + return get_tree_bdev(fc, efs_fill_super); +} + +static int efs_reconfigure(struct fs_context *fc) +{ + sync_filesystem(fc->root->d_sb); + + return 0; +} + +struct efs_context { + unsigned long s_mount_opts; +}; + +static const struct fs_context_operations efs_context_opts = { + .get_tree = efs_get_tree, + .reconfigure = efs_reconfigure, + .free = efs_free_fc, +}; + +/* + * Set up the filesystem mount context. + */ +static int efs_init_fs_context(struct fs_context *fc) +{ + struct efs_context *ctx; + + ctx = kzalloc(sizeof(struct efs_context), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + fc->fs_private = ctx; + fc->ops = &efs_context_opts; + + return 0; +} + static int efs_statfs(struct dentry *dentry, struct kstatfs *buf) { struct super_block *sb = dentry->d_sb; struct efs_sb_info *sbi = SUPER_INFO(sb);
Convert the efs filesystem to use the new mount API. Signed-off-by: Bill O'Donnell <bodonnel@redhat.com> --- Changelog: v2: Remove efs_param_spec and efs_parse_param, since no mount options. --- fs/efs/super.c | 91 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 30 deletions(-)