Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
Andrew Price wrote: > Would this fly? Can you update the comment to document what it's for? David
Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
On 19/03/2019 17:53, Andrew Price wrote: On 19/03/2019 17:05, David Howells wrote: Can you use vfs_get_block_super()? It might be possible if we can rearrange things so that this can be done outside of the function: if (args->ar_meta) fc->root = dget(sdp->sd_master_dir); else fc->root = dget(sdp->sd_root_dir); but we can't do that in our fill_super() because it needs to be selected whether we have an existing mount or not. Would this fly? diff --git a/fs/super.c b/fs/super.c index 6d8dbf309241..0cdeaf28126f 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1232,11 +1232,12 @@ static int test_bdev_super_fc(struct super_block *s, struct fs_context *fc) * @fill_super: Helper to initialise a new superblock */ int vfs_get_block_super(struct fs_context *fc, - int (*fill_super)(struct super_block *, - struct fs_context *)) + int (*fill_super)(struct super_block *, struct fs_context *), + struct dentry* (*select_root)(struct fs_context *, struct super_block *)) { struct block_device *bdev; struct super_block *s; + struct dentry *root; int error = 0; fc->bdev_mode = FMODE_READ | FMODE_EXCL; @@ -1302,7 +1303,11 @@ int vfs_get_block_super(struct fs_context *fc, } BUG_ON(fc->root); - fc->root = dget(s->s_root); + if (select_root) + root = select_root(fc, s); + else + root = s->s_root; + fc->root = dget(root); return 0; error_sb: diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h index 8233c873af73..c2e9dc5826ce 100644 --- a/include/linux/fs_context.h +++ b/include/linux/fs_context.h @@ -145,8 +145,8 @@ extern int vfs_get_super(struct fs_context *fc, struct fs_context *fc)); extern int vfs_get_block_super(struct fs_context *fc, - int (*fill_super)(struct super_block *sb, -struct fs_context *fc)); + int (*fill_super)(struct super_block *, struct fs_context *), + struct dentry* (*select_root)(struct fs_context *, struct super_block *)); extern const struct file_operations fscontext_fops;
Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
On 19/03/2019 17:05, David Howells wrote: Andrew Price wrote: + pr_warn("-o debug and -o errors=panic are mutually exclusive\n"); + return -EINVAL; return invalf(fc, "gfs2: -o debug and -o errors=panic are mutually exclusive"); Thanks. (Note: no "\n") + if (result.int_32 > 0) + args->ar_quota = opt_quota_values[result.int_32]; + else if (result.negated) + args->ar_quota = GFS2_QUOTA_OFF; + else + args->ar_quota = GFS2_QUOTA_ON; I recommend checking result.negated first. OK + /* Not allowed to change locking details */ + if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) || + strcmp(newargs->ar_locktable, oldargs->ar_locktable) || + strcmp(newargs->ar_hostdata, oldargs->ar_hostdata)) + return -EINVAL; Use errorf(). (Not invalf - the parameter isn't exactly invalid, it's just that you're not allowed to do this operation). Yes, that makes more sense. + error = gfs2_make_fs_ro(sdp); + else + error = gfs2_make_fs_rw(sdp); + if (error) + return error; Might want to call errorf() here too. - s = sget(_fs_type, test_gfs2_super, set_meta_super, flags, + s = sget(_fs_type, test_meta_super, set_meta_super, flags, Try and use sget_fc() please. That happens in patch 2/2 for gfs2meta. I might just roll both patches together in v3 so there's no intermediate churn. If you look at the fuse patchset I cc'd you on, there's a commit there that adds a ->bdev and ->bdev_mode to fs_context that may be of use to you. Yes, that looks useful - thanks. Can you use vfs_get_block_super()? It might be possible if we can rearrange things so that this can be done outside of the function: if (args->ar_meta) fc->root = dget(sdp->sd_master_dir); else fc->root = dget(sdp->sd_root_dir); but we can't do that in our fill_super() because it needs to be selected whether we have an existing mount or not. Would it be of use to export test_bdev_super_fc() and set_bdev_super_fc()? There's only a little maintenance value in it, I think, but we could certainly use them in gfs2 if we can't solve the fc->root selection issue. Andy
Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
Andrew Price wrote: > + pr_warn("-o debug and -o errors=panic are mutually > exclusive\n"); > + return -EINVAL; return invalf(fc, "gfs2: -o debug and -o errors=panic are mutually exclusive"); (Note: no "\n") > + if (result.int_32 > 0) > + args->ar_quota = opt_quota_values[result.int_32]; > + else if (result.negated) > + args->ar_quota = GFS2_QUOTA_OFF; > + else > + args->ar_quota = GFS2_QUOTA_ON; I recommend checking result.negated first. > + /* Not allowed to change locking details */ > + if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) || > + strcmp(newargs->ar_locktable, oldargs->ar_locktable) || > + strcmp(newargs->ar_hostdata, oldargs->ar_hostdata)) > + return -EINVAL; Use errorf(). (Not invalf - the parameter isn't exactly invalid, it's just that you're not allowed to do this operation). > + error = gfs2_make_fs_ro(sdp); > + else > + error = gfs2_make_fs_rw(sdp); > + if (error) > + return error; Might want to call errorf() here too. > - s = sget(_fs_type, test_gfs2_super, set_meta_super, flags, > + s = sget(_fs_type, test_meta_super, set_meta_super, flags, Try and use sget_fc() please. If you look at the fuse patchset I cc'd you on, there's a commit there that adds a ->bdev and ->bdev_mode to fs_context that may be of use to you. Can you use vfs_get_block_super()? Would it be of use to export test_bdev_super_fc() and set_bdev_super_fc()? David
[Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
Switch to using fs_context ops for mount/remount. Signed-off-by: Andrew Price --- fs/gfs2/incore.h | 11 +- fs/gfs2/ops_fstype.c | 405 ++- fs/gfs2/super.c | 335 +-- fs/gfs2/super.h | 3 +- 4 files changed, 370 insertions(+), 384 deletions(-) diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index cdf07b408f54..ee474201f3b5 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -587,10 +587,13 @@ struct gfs2_args { unsigned int ar_rgrplvb:1; /* use lvbs for rgrp info */ unsigned int ar_loccookie:1;/* use location based readdir cookies */ - int ar_commit; /* Commit interval */ - int ar_statfs_quantum; /* The fast statfs interval */ - int ar_quota_quantum; /* The quota interval */ - int ar_statfs_percent; /* The % change to force sync */ + s32 ar_commit; /* Commit interval */ + s32 ar_statfs_quantum; /* The fast statfs interval */ + s32 ar_quota_quantum; /* The quota interval */ + s32 ar_statfs_percent; /* The % change to force sync */ + + /* This is just for propagating the bdev through sget_fc() */ + struct block_device *ar_bdev; }; struct gfs2_tune { diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index b041cb8ae383..ad2c98a37bd4 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "gfs2.h" #include "incore.h" @@ -1200,50 +1201,49 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent return error; } -static int set_gfs2_super(struct super_block *s, void *data) +static int set_gfs2_super(struct super_block *s, struct fs_context *fc) { - s->s_bdev = data; + struct gfs2_args *args = fc->fs_private; + + s->s_bdev = args->ar_bdev; s->s_dev = s->s_bdev->bd_dev; s->s_bdi = bdi_get(s->s_bdev->bd_bdi); return 0; } -static int test_gfs2_super(struct super_block *s, void *ptr) +static int test_gfs2_super(struct super_block *s, struct fs_context *fc) { - struct block_device *bdev = ptr; - return (bdev == s->s_bdev); + struct gfs2_args *args = fc->fs_private; + + return (args->ar_bdev == s->s_bdev); } /** - * gfs2_mount - Get the GFS2 superblock - * @fs_type: The GFS2 filesystem type - * @flags: Mount flags - * @dev_name: The name of the device - * @data: The mount arguments + * gfs2_get_tree - Get the GFS2 superblock and root directory + * @fc: The filesystem context * * Q. Why not use get_sb_bdev() ? * A. We need to select one of two root directories to mount, independent *of whether this is the initial, or subsequent, mount of this sb * - * Returns: 0 or -ve on error + * Returns: 0 or -errno on error */ -static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags, - const char *dev_name, void *data) +static int gfs2_get_tree(struct fs_context *fc) { + struct gfs2_args *args = fc->fs_private; + fmode_t mode = FMODE_READ | FMODE_EXCL; struct block_device *bdev; struct super_block *s; - fmode_t mode = FMODE_READ | FMODE_EXCL; - int error; - struct gfs2_args args; struct gfs2_sbd *sdp; + int error; - if (!(flags & SB_RDONLY)) + if (!(fc->sb_flags & SB_RDONLY)) mode |= FMODE_WRITE; - bdev = blkdev_get_by_path(dev_name, mode, fs_type); + bdev = blkdev_get_by_path(fc->source, mode, fc->fs_type); if (IS_ERR(bdev)) - return ERR_CAST(bdev); + return PTR_ERR(bdev); /* * once the super is inserted into the list by sget, s_umount @@ -1256,7 +1256,8 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags, error = -EBUSY; goto error_bdev; } - s = sget(fs_type, test_gfs2_super, set_gfs2_super, flags, bdev); + args->ar_bdev = bdev; + s = sget_fc(fc, test_gfs2_super, set_gfs2_super); mutex_unlock(>bd_fsfreeze_mutex); error = PTR_ERR(s); if (IS_ERR(s)) @@ -1278,28 +1279,14 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags, s->s_mode = mode; } - memset(, 0, sizeof(args)); - args.ar_quota = GFS2_QUOTA_DEFAULT; - args.ar_data = GFS2_DATA_DEFAULT; - args.ar_commit = 30; - args.ar_statfs_quantum = 30; - args.ar_quota_quantum = 60; - args.ar_errors = GFS2_ERRORS_DEFAULT; - - error = gfs2_mount_args(, data); - if (error) { - pr_warn("can't parse mount arguments\n"); - goto error_super;