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;
