On Mon 16-09-24 13:26:22, Eric Sandeen wrote: > Convert the hfsplus filesystem to use the new mount API. > Tested by comparing random mount & remount options before and after > the change. > > Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/hfsplus/hfsplus_fs.h | 4 +- > fs/hfsplus/options.c | 263 ++++++++++++++-------------------------- > fs/hfsplus/super.c | 84 ++++++++----- > 3 files changed, 149 insertions(+), 202 deletions(-) > > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h > index 9e78f181c24f..53cb3c7dfc73 100644 > --- a/fs/hfsplus/hfsplus_fs.h > +++ b/fs/hfsplus/hfsplus_fs.h > @@ -21,6 +21,7 @@ > #include <linux/mutex.h> > #include <linux/buffer_head.h> > #include <linux/blkdev.h> > +#include <linux/fs_context.h> > #include "hfsplus_raw.h" > > #define DBG_BNODE_REFS 0x00000001 > @@ -496,8 +497,7 @@ long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); > > /* options.c */ > void hfsplus_fill_defaults(struct hfsplus_sb_info *opts); > -int hfsplus_parse_options_remount(char *input, int *force); > -int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi); > +int hfsplus_parse_param(struct fs_context *fc, struct fs_parameter *param); > int hfsplus_show_options(struct seq_file *seq, struct dentry *root); > > /* part_tbl.c */ > diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c > index c94a58762ad6..a66a09a56bf7 100644 > --- a/fs/hfsplus/options.c > +++ b/fs/hfsplus/options.c > @@ -12,7 +12,8 @@ > #include <linux/string.h> > #include <linux/kernel.h> > #include <linux/sched.h> > -#include <linux/parser.h> > +#include <linux/fs_context.h> > +#include <linux/fs_parser.h> > #include <linux/nls.h> > #include <linux/mount.h> > #include <linux/seq_file.h> > @@ -23,26 +24,23 @@ enum { > opt_creator, opt_type, > opt_umask, opt_uid, opt_gid, > opt_part, opt_session, opt_nls, > - opt_nodecompose, opt_decompose, > - opt_barrier, opt_nobarrier, > - opt_force, opt_err > + opt_decompose, opt_barrier, > + opt_force, > }; > > -static const match_table_t tokens = { > - { opt_creator, "creator=%s" }, > - { opt_type, "type=%s" }, > - { opt_umask, "umask=%o" }, > - { opt_uid, "uid=%u" }, > - { opt_gid, "gid=%u" }, > - { opt_part, "part=%u" }, > - { opt_session, "session=%u" }, > - { opt_nls, "nls=%s" }, > - { opt_decompose, "decompose" }, > - { opt_nodecompose, "nodecompose" }, > - { opt_barrier, "barrier" }, > - { opt_nobarrier, "nobarrier" }, > - { opt_force, "force" }, > - { opt_err, NULL } > +static const struct fs_parameter_spec hfs_param_spec[] = { > + fsparam_string ("creator", opt_creator), > + fsparam_string ("type", opt_type), > + fsparam_u32oct ("umask", opt_umask), > + fsparam_u32 ("uid", opt_uid), > + fsparam_u32 ("gid", opt_gid), > + fsparam_u32 ("part", opt_part), > + fsparam_u32 ("session", opt_session), > + fsparam_string ("nls", opt_nls), > + fsparam_flag_no ("decompose", opt_decompose), > + fsparam_flag_no ("barrier", opt_barrier), > + fsparam_flag ("force", opt_force), > + {} > }; > > /* Initialize an options object to reasonable defaults */ > @@ -60,162 +58,89 @@ void hfsplus_fill_defaults(struct hfsplus_sb_info *opts) > opts->session = -1; > } > > -/* convert a "four byte character" to a 32 bit int with error checks */ > -static inline int match_fourchar(substring_t *arg, u32 *result) > +/* Parse options from mount. Returns nonzero errno on failure */ > +int hfsplus_parse_param(struct fs_context *fc, struct fs_parameter *param) > { > - if (arg->to - arg->from != 4) > - return -EINVAL; > - memcpy(result, arg->from, 4); > - return 0; > -} > - > -int hfsplus_parse_options_remount(char *input, int *force) > -{ > - char *p; > - substring_t args[MAX_OPT_ARGS]; > - int token; > - > - if (!input) > - return 1; > - > - while ((p = strsep(&input, ",")) != NULL) { > - if (!*p) > - continue; > - > - token = match_token(p, tokens, args); > - switch (token) { > - case opt_force: > - *force = 1; > - break; > - default: > - break; > + struct hfsplus_sb_info *sbi = fc->s_fs_info; > + struct fs_parse_result result; > + int opt; > + > + /* > + * Only the force option is examined during remount, all others > + * are ignored. > + */ > + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && > + strncmp(param->key, "force", 5)) > + return 0; > + > + opt = fs_parse(fc, hfs_param_spec, param, &result); > + if (opt < 0) > + return opt; > + > + switch (opt) { > + case opt_creator: > + if (strlen(param->string) != 4) { > + pr_err("creator requires a 4 character value\n"); > + return -EINVAL; > } > - } > - > - return 1; > -} > - > -/* Parse options from mount. Returns 0 on failure */ > -/* input is the options passed to mount() as a string */ > -int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi) > -{ > - char *p; > - substring_t args[MAX_OPT_ARGS]; > - int tmp, token; > - > - if (!input) > - goto done; > - > - while ((p = strsep(&input, ",")) != NULL) { > - if (!*p) > - continue; > - > - token = match_token(p, tokens, args); > - switch (token) { > - case opt_creator: > - if (match_fourchar(&args[0], &sbi->creator)) { > - pr_err("creator requires a 4 character value\n"); > - return 0; > - } > - break; > - case opt_type: > - if (match_fourchar(&args[0], &sbi->type)) { > - pr_err("type requires a 4 character value\n"); > - return 0; > - } > - break; > - case opt_umask: > - if (match_octal(&args[0], &tmp)) { > - pr_err("umask requires a value\n"); > - return 0; > - } > - sbi->umask = (umode_t)tmp; > - break; > - case opt_uid: > - if (match_int(&args[0], &tmp)) { > - pr_err("uid requires an argument\n"); > - return 0; > - } > - sbi->uid = make_kuid(current_user_ns(), (uid_t)tmp); > - if (!uid_valid(sbi->uid)) { > - pr_err("invalid uid specified\n"); > - return 0; > - } else { > - set_bit(HFSPLUS_SB_UID, &sbi->flags); > - } > - break; > - case opt_gid: > - if (match_int(&args[0], &tmp)) { > - pr_err("gid requires an argument\n"); > - return 0; > - } > - sbi->gid = make_kgid(current_user_ns(), (gid_t)tmp); > - if (!gid_valid(sbi->gid)) { > - pr_err("invalid gid specified\n"); > - return 0; > - } else { > - set_bit(HFSPLUS_SB_GID, &sbi->flags); > - } > - break; > - case opt_part: > - if (match_int(&args[0], &sbi->part)) { > - pr_err("part requires an argument\n"); > - return 0; > - } > - break; > - case opt_session: > - if (match_int(&args[0], &sbi->session)) { > - pr_err("session requires an argument\n"); > - return 0; > - } > - break; > - case opt_nls: > - if (sbi->nls) { > - pr_err("unable to change nls mapping\n"); > - return 0; > - } > - p = match_strdup(&args[0]); > - if (p) > - sbi->nls = load_nls(p); > - if (!sbi->nls) { > - pr_err("unable to load nls mapping \"%s\"\n", > - p); > - kfree(p); > - return 0; > - } > - kfree(p); > - break; > - case opt_decompose: > - clear_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags); > - break; > - case opt_nodecompose: > + memcpy(&sbi->creator, param->string, 4); > + break; > + case opt_type: > + if (strlen(param->string) != 4) { > + pr_err("type requires a 4 character value\n"); > + return -EINVAL; > + } > + memcpy(&sbi->type, param->string, 4); > + break; > + case opt_umask: > + sbi->umask = (umode_t)result.uint_32; > + break; > + case opt_uid: > + sbi->uid = result.uid; > + set_bit(HFSPLUS_SB_UID, &sbi->flags); > + break; > + case opt_gid: > + sbi->gid = result.gid; > + set_bit(HFSPLUS_SB_GID, &sbi->flags); > + break; > + case opt_part: > + sbi->part = result.uint_32; > + break; > + case opt_session: > + sbi->session = result.uint_32; > + break; > + case opt_nls: > + if (sbi->nls) { > + pr_err("unable to change nls mapping\n"); > + return -EINVAL; > + } > + sbi->nls = load_nls(param->string); > + if (!sbi->nls) { > + pr_err("unable to load nls mapping \"%s\"\n", > + param->string); > + return -EINVAL; > + } > + break; > + case opt_decompose: > + if (result.negated) > set_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags); > - break; > - case opt_barrier: > - clear_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags); > - break; > - case opt_nobarrier: > + else > + clear_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags); > + break; > + case opt_barrier: > + if (result.negated) > set_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags); > - break; > - case opt_force: > - set_bit(HFSPLUS_SB_FORCE, &sbi->flags); > - break; > - default: > - return 0; > - } > - } > - > -done: > - if (!sbi->nls) { > - /* try utf8 first, as this is the old default behaviour */ > - sbi->nls = load_nls("utf8"); > - if (!sbi->nls) > - sbi->nls = load_nls_default(); > - if (!sbi->nls) > - return 0; > + else > + clear_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags); > + break; > + case opt_force: > + set_bit(HFSPLUS_SB_FORCE, &sbi->flags); > + break; > + default: > + return -EINVAL; > } > > - return 1; > + return 0; > } > > int hfsplus_show_options(struct seq_file *seq, struct dentry *root) > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c > index 8f986f30a1ce..261af016efd3 100644 > --- a/fs/hfsplus/super.c > +++ b/fs/hfsplus/super.c > @@ -14,6 +14,7 @@ > #include <linux/blkdev.h> > #include <linux/backing-dev.h> > #include <linux/fs.h> > +#include <linux/fs_context.h> > #include <linux/slab.h> > #include <linux/vfs.h> > #include <linux/nls.h> > @@ -332,34 +333,33 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf) > return 0; > } > > -static int hfsplus_remount(struct super_block *sb, int *flags, char *data) > +static int hfsplus_reconfigure(struct fs_context *fc) > { > + struct super_block *sb = fc->root->d_sb; > + > sync_filesystem(sb); > - if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb)) > + if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb)) > return 0; > - if (!(*flags & SB_RDONLY)) { > - struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr; > - int force = 0; > - > - if (!hfsplus_parse_options_remount(data, &force)) > - return -EINVAL; > + if (!(fc->sb_flags & SB_RDONLY)) { > + struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb); > + struct hfsplus_vh *vhdr = sbi->s_vhdr; > > if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) { > pr_warn("filesystem was not cleanly unmounted, running fsck.hfsplus is recommended. leaving read-only.\n"); > sb->s_flags |= SB_RDONLY; > - *flags |= SB_RDONLY; > - } else if (force) { > + fc->sb_flags |= SB_RDONLY; > + } else if (test_bit(HFSPLUS_SB_FORCE, &sbi->flags)) { > /* nothing */ > } else if (vhdr->attributes & > cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) { > pr_warn("filesystem is marked locked, leaving read-only.\n"); > sb->s_flags |= SB_RDONLY; > - *flags |= SB_RDONLY; > + fc->sb_flags |= SB_RDONLY; > } else if (vhdr->attributes & > cpu_to_be32(HFSPLUS_VOL_JOURNALED)) { > pr_warn("filesystem is marked journaled, leaving read-only.\n"); > sb->s_flags |= SB_RDONLY; > - *flags |= SB_RDONLY; > + fc->sb_flags |= SB_RDONLY; > } > } > return 0; > @@ -373,38 +373,33 @@ static const struct super_operations hfsplus_sops = { > .put_super = hfsplus_put_super, > .sync_fs = hfsplus_sync_fs, > .statfs = hfsplus_statfs, > - .remount_fs = hfsplus_remount, > .show_options = hfsplus_show_options, > }; > > -static int hfsplus_fill_super(struct super_block *sb, void *data, int silent) > +static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) > { > struct hfsplus_vh *vhdr; > - struct hfsplus_sb_info *sbi; > + struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb); > hfsplus_cat_entry entry; > struct hfs_find_data fd; > struct inode *root, *inode; > struct qstr str; > - struct nls_table *nls = NULL; > + struct nls_table *nls; > u64 last_fs_block, last_fs_page; > + int silent = fc->sb_flags & SB_SILENT; > int err; > > - err = -ENOMEM; > - sbi = kzalloc(sizeof(*sbi), GFP_KERNEL); > - if (!sbi) > - goto out; > - > - sb->s_fs_info = sbi; > mutex_init(&sbi->alloc_mutex); > mutex_init(&sbi->vh_mutex); > spin_lock_init(&sbi->work_lock); > INIT_DELAYED_WORK(&sbi->sync_work, delayed_sync_fs); > - hfsplus_fill_defaults(sbi); > > err = -EINVAL; > - if (!hfsplus_parse_options(data, sbi)) { > - pr_err("unable to parse mount options\n"); > - goto out_unload_nls; > + if (!sbi->nls) { > + /* try utf8 first, as this is the old default behaviour */ > + sbi->nls = load_nls("utf8"); > + if (!sbi->nls) > + sbi->nls = load_nls_default(); > } > > /* temporarily use utf8 to correctly find the hidden dir below */ > @@ -616,7 +611,6 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent) > unload_nls(sbi->nls); > unload_nls(nls); > kfree(sbi); > -out: > return err; > } > > @@ -641,18 +635,46 @@ static void hfsplus_free_inode(struct inode *inode) > > #define HFSPLUS_INODE_SIZE sizeof(struct hfsplus_inode_info) > > -static struct dentry *hfsplus_mount(struct file_system_type *fs_type, > - int flags, const char *dev_name, void *data) > +static int hfsplus_get_tree(struct fs_context *fc) > +{ > + return get_tree_bdev(fc, hfsplus_fill_super); > +} > + > +static void hfsplus_free_fc(struct fs_context *fc) > { > - return mount_bdev(fs_type, flags, dev_name, data, hfsplus_fill_super); > + kfree(fc->s_fs_info); > +} > + > +static const struct fs_context_operations hfsplus_context_ops = { > + .parse_param = hfsplus_parse_param, > + .get_tree = hfsplus_get_tree, > + .reconfigure = hfsplus_reconfigure, > + .free = hfsplus_free_fc, > +}; > + > +static int hfsplus_init_fs_context(struct fs_context *fc) > +{ > + struct hfsplus_sb_info *sbi; > + > + sbi = kzalloc(sizeof(struct hfsplus_sb_info), GFP_KERNEL); > + if (!sbi) > + return -ENOMEM; > + > + if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE) > + hfsplus_fill_defaults(sbi); > + > + fc->s_fs_info = sbi; > + fc->ops = &hfsplus_context_ops; > + > + return 0; > } > > static struct file_system_type hfsplus_fs_type = { > .owner = THIS_MODULE, > .name = "hfsplus", > - .mount = hfsplus_mount, > .kill_sb = kill_block_super, > .fs_flags = FS_REQUIRES_DEV, > + .init_fs_context = hfsplus_init_fs_context, > }; > MODULE_ALIAS_FS("hfsplus"); > > -- > 2.46.0 > > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR