* [PATCH v2] efs: convert efs to use the new mount api
@ 2024-02-20 16:45 Bill O'Donnell
2024-02-20 21:05 ` Eric Sandeen
0 siblings, 1 reply; 3+ messages in thread
From: Bill O'Donnell @ 2024-02-20 16:45 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, Bill O'Donnell
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(-)
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);
--
2.43.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] efs: convert efs to use the new mount api
2024-02-20 16:45 [PATCH v2] efs: convert efs to use the new mount api Bill O'Donnell
@ 2024-02-20 21:05 ` Eric Sandeen
2024-02-20 23:01 ` Bill O'Donnell
0 siblings, 1 reply; 3+ messages in thread
From: Eric Sandeen @ 2024-02-20 21:05 UTC (permalink / raw)
To: Bill O'Donnell, linux-fsdevel; +Cc: brauner, David Howells
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);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] efs: convert efs to use the new mount api
2024-02-20 21:05 ` Eric Sandeen
@ 2024-02-20 23:01 ` Bill O'Donnell
0 siblings, 0 replies; 3+ messages in thread
From: Bill O'Donnell @ 2024-02-20 23:01 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-fsdevel, brauner, David Howells
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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-20 23:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-20 16:45 [PATCH v2] efs: convert efs to use the new mount api Bill O'Donnell
2024-02-20 21:05 ` Eric Sandeen
2024-02-20 23:01 ` Bill O'Donnell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).