From: David Sterba <dsterba@suse.cz>
To: Marcos Paulo de Souza <marcos@mpdesouza.com>
Cc: dsterba@suse.com, linux-btrfs@vger.kernel.org,
Marcos Paulo de Souza <mpdesouza@suse.com>
Subject: Re: [RFC PATCH 7/8] btrfs: Convert to fs_context
Date: Mon, 17 Aug 2020 15:26:14 +0200 [thread overview]
Message-ID: <20200817132614.GH2026@twin.jikos.cz> (raw)
In-Reply-To: <20200812163654.17080-8-marcos@mpdesouza.com>
On Wed, Aug 12, 2020 at 01:36:53PM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
>
> This commit makes btrfs full use of the fs_context API. It's heavily
> based in David Howells POC when fs_context was introduced[1]. As
> fs_context divides the mount procedure into different steps, the btrfs
> code needed to be rearranged in order to store the mount points and
> other properties without having a fs_info already allocated.
>
> The btrfs_fs_context struct is the responsible struct for storing the
> options used to mount the fs, and these options are set to
> fs_info->mount_opts in btrfs_apply_configuration.
>
> Some notable changes:
> * There is no need for a second file_system_type anymore. Now we use a
> special flag in btrfs_fs_context to identify the mount of the root
> subvol before mounting the specified subvol.
> * With the introduction of fs_context all mount options are parsed
> _before_ doing the mount procedure. In our case, btrfs_fc_parse_param
> is the function being called to parse all options, making
> btrfs_parse_device_options, btrfs_parse_subvol_options
> btrfs_parse_options obsolete.
> * Function btrfs_mount_root was renamed to btrfs_root_get_tree to
> reflect that fs_context is being used.
> * Function btrfs_remount was renamed to btrfs_reconfigure by the same
> reason from above.
> * Function btrfs_mount was renamed to btrfs_get_tree, and this function
> is called by vfs after all the mount options are parsed.
> * There is no need for btrfs_root_fs_type anymore, and so fs_info->bdev_holder
> can also be removed.
>
> Functions like open_ctree and btrfs_fill_super are now using a
> fs_context argument instead of other btrfs related structs, since
> the fs_context->fs_private contains a btrfs_fs_context that holds
> the necessary data.
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/commit/?h=Q46&id=554cb2019cda83e1aba10bd9eea485afd2ddb983
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> fs/btrfs/ctree.h | 1 +
> fs/btrfs/dev-replace.c | 2 +-
> fs/btrfs/disk-io.c | 10 +-
> fs/btrfs/disk-io.h | 4 +-
> fs/btrfs/super.c | 347 ++++++++++++++++++++---------------------
> fs/btrfs/volumes.c | 6 +-
> 6 files changed, 178 insertions(+), 192 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index d96bce2ea5bb..9060be6a6c6e 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3100,6 +3100,7 @@ int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
> struct btrfs_root *root);
>
> /* super.c */
> +extern struct file_system_type btrfs_fs_type;
This and the related changes shouldn't be in this patch.
> int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> unsigned long new_flags);
> int btrfs_apply_configuration(struct fs_context *fc, struct super_block *sb);
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index db93909b25e0..9a3275845b13 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -236,7 +236,7 @@ static int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> }
>
> bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
> - fs_info->bdev_holder);
> + &btrfs_fs_type);
So clean the bdev_holder in a separate patch.
> if (IS_ERR(bdev)) {
> btrfs_err(fs_info, "target device %s is invalid!", device_path);
> return PTR_ERR(bdev);
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c850d7f44fbe..f2e7543e9913 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2894,8 +2894,7 @@ static int btrfs_check_uuid_tree(struct btrfs_fs_info *fs_info)
> return 0;
> }
>
> -int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_devices,
> - char *options)
> +int __cold open_ctree(struct fs_context *fc, struct super_block *sb)
> {
> u32 sectorsize;
> u32 nodesize;
> @@ -2905,6 +2904,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> u16 csum_type;
> struct btrfs_super_block *disk_super;
> struct btrfs_fs_info *fs_info = btrfs_sb(sb);
> + struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> struct btrfs_root *tree_root;
> struct btrfs_root *chunk_root;
> int ret;
> @@ -3030,11 +3030,9 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> */
> fs_info->compress_type = BTRFS_COMPRESS_ZLIB;
>
> - ret = btrfs_parse_options(fs_info, options, sb->s_flags);
> - if (ret) {
> - err = ret;
> + err = btrfs_apply_configuration(fc, sb);
> + if (err)
> goto fail_alloc;
> - }
>
> features = btrfs_super_incompat_flags(disk_super) &
> ~BTRFS_FEATURE_INCOMPAT_SUPP;
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 00dc39d47ed3..712ea61ffdfe 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -50,9 +50,7 @@ struct extent_buffer *btrfs_find_create_tree_block(
> struct btrfs_fs_info *fs_info,
> u64 bytenr);
> void btrfs_clean_tree_block(struct extent_buffer *buf);
> -int __cold open_ctree(struct super_block *sb,
> - struct btrfs_fs_devices *fs_devices,
> - char *options);
> +int __cold open_ctree(struct fs_context *fc, struct super_block *sb);
> void __cold close_ctree(struct btrfs_fs_info *fs_info);
> int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors);
> struct btrfs_super_block *btrfs_read_dev_super(struct block_device *bdev);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 5bbf4b947125..d9a0faea8c88 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -63,10 +63,9 @@ static const struct super_operations btrfs_super_ops;
> *
> * The new btrfs_root_fs_type also servers as a tag for the bdev_holder.
> */
> -static struct file_system_type btrfs_fs_type;
> static struct file_system_type btrfs_root_fs_type;
>
> -static int btrfs_remount(struct super_block *sb, int *flags, char *data);
> +static int btrfs_reconfigure(struct fs_context *fc);
>
> /*
> * Generally the error codes correspond to their respective errors, but there
> @@ -1542,7 +1541,7 @@ static int btrfs_parse_device_options(const char *options, fmode_t flags,
> goto out;
> }
> device = btrfs_scan_one_device(device_name, flags,
> - holder);
> + &btrfs_fs_type);
> kfree(device_name);
> if (IS_ERR(device)) {
> error = PTR_ERR(device);
> @@ -1795,13 +1794,13 @@ static int get_default_subvol_objectid(struct btrfs_fs_info *fs_info, u64 *objec
> return 0;
> }
>
> -static int btrfs_fill_super(struct super_block *sb,
> - struct btrfs_fs_devices *fs_devices,
> - void *data)
> +static int btrfs_fill_super(struct fs_context *fc,
> + struct super_block *sb,
> + struct btrfs_fs_devices *fs_devices)
> {
> + int err;
> struct inode *inode;
> struct btrfs_fs_info *fs_info = btrfs_sb(sb);
> - int err;
>
> sb->s_maxbytes = MAX_LFS_FILESIZE;
> sb->s_magic = BTRFS_SUPER_MAGIC;
> @@ -1810,9 +1809,6 @@ static int btrfs_fill_super(struct super_block *sb,
> sb->s_export_op = &btrfs_export_ops;
> sb->s_xattr = btrfs_xattr_handlers;
> sb->s_time_gran = 1;
> -#ifdef CONFIG_BTRFS_FS_POSIX_ACL
> - sb->s_flags |= SB_POSIXACL;
> -#endif
> sb->s_flags |= SB_I_VERSION;
> sb->s_iflags |= SB_I_CGROUPWB;
>
> @@ -1822,7 +1818,7 @@ static int btrfs_fill_super(struct super_block *sb,
> return err;
> }
>
> - err = open_ctree(sb, fs_devices, (char *)data);
> + err = open_ctree(fc, sb);
> if (err) {
> btrfs_err(fs_info, "open_ctree failed");
> return err;
> @@ -1951,19 +1947,20 @@ int btrfs_apply_configuration(struct fs_context *fc,
> #endif
>
> #ifdef CONFIG_BTRFS_FS_REF_VERIFY
> - if (btrfs_test_exp_opt(ctx, REF_VERIFY))
> - btrfs_info(info, "doing ref verification");
> + if (btrfs_test_exp_opt(ctx, REF_VERIFY))
> + btrfs_info(info, "doing ref verification");
> #endif
>
> + info->pending_changes = ctx->pending_changes;
> if (btrfs_test_exp_opt(ctx, INODE_MAP_CACHE)) {
> - if (btrfs_opt_map[Opt_inode_cache].enabled) {
> - btrfs_clear_pending_and_info(info, INODE_MAP_CACHE,
> - "disabling inode map caching");
> + if (!btrfs_opt_map[Opt_inode_cache].enabled) {
> + btrfs_clear_pending_and_info(info, INODE_MAP_CACHE,
> + "disabling inode map caching");
> } else {
> btrfs_info(info,
> "the 'inode_cache' option is deprecated and will have no effect from 5.11");
> btrfs_set_pending_and_info(info, INODE_MAP_CACHE,
> - "enabling inode map caching");
> + "enabling inode map caching");
Whitespace-only change
> }
> }
>
> @@ -2015,10 +2012,9 @@ int btrfs_apply_configuration(struct fs_context *fc,
> if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE)) {
> bool opt_clear = btrfs_test_opt(ctx, CLEAR_CACHE);
> bool opt_cache_v1 = btrfs_test_opt(ctx, SPACE_CACHE);
> - bool no_space_cache = ctx->nospace_cache;
>
> - if ((no_space_cache && !opt_clear) || (opt_cache_v1 && !opt_clear)) {
> - btrfs_err(info, "cannot disable free space tree XX");
> + if ((ctx->nospace_cache && !opt_clear) || (opt_cache_v1 && !opt_clear)) {
> + btrfs_err(info, "cannot disable free space tree");
> return -EINVAL;
> }
> }
> @@ -2063,7 +2059,6 @@ int btrfs_apply_configuration(struct fs_context *fc,
>
> info->compress_type = ctx->compress_type;
> info->compress_level = ctx->compress_level;
> - info->pending_changes = ctx->pending_changes;
> info->mount_opt = ctx->mount_opt;
>
> return 0;
> @@ -2213,9 +2208,9 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
> return 0;
> }
>
> -static int btrfs_test_super(struct super_block *s, void *data)
> +static int btrfs_test_super(struct super_block *s, struct fs_context *fc)
> {
> - struct btrfs_fs_info *p = data;
> + struct btrfs_fs_info *p = fc->s_fs_info;
> struct btrfs_fs_info *fs_info = btrfs_sb(s);
>
> return fs_info->fs_devices == p->fs_devices;
> @@ -2239,34 +2234,34 @@ static inline int is_subvolume_inode(struct inode *inode)
> return 0;
> }
>
> -static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid,
> - struct vfsmount *mnt)
> +static int mount_subvol(struct fs_context *fc)
> {
> + struct btrfs_fs_context *ctx = fc->fs_private;
> + struct vfsmount *mnt = ctx->root_mnt;
> struct dentry *root;
> int ret;
>
> - if (!subvol_name) {
> - if (!subvol_objectid) {
> + if (!ctx->subvol_name) {
> + char *subvol_name;
> +
> + if (!ctx->subvolid) {
> ret = get_default_subvol_objectid(btrfs_sb(mnt->mnt_sb),
> - &subvol_objectid);
> - if (ret) {
> - root = ERR_PTR(ret);
> - goto out;
> - }
> + &ctx->subvolid);
> + if (ret)
> + return ret;
> }
> subvol_name = btrfs_get_subvol_name_from_objectid(
> - btrfs_sb(mnt->mnt_sb), subvol_objectid);
> - if (IS_ERR(subvol_name)) {
> - root = ERR_CAST(subvol_name);
> - subvol_name = NULL;
> - goto out;
> - }
> + btrfs_sb(mnt->mnt_sb),
> + ctx->subvolid);
> + if (IS_ERR(subvol_name))
> + return PTR_ERR(subvol_name);
>
> + ctx->subvol_name = subvol_name;
> }
>
> - root = mount_subtree(mnt, subvol_name);
> - /* mount_subtree() drops our reference on the vfsmount. */
> - mnt = NULL;
> + root = mount_subtree(mnt, ctx->subvol_name);
> + /* mount_subtree() dropped our reference on the vfsmount. */
> + ctx->root_mnt = NULL;
>
> if (!IS_ERR(root)) {
> struct super_block *s = root->d_sb;
> @@ -2277,10 +2272,10 @@ static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid,
> ret = 0;
> if (!is_subvolume_inode(root_inode)) {
> btrfs_err(fs_info, "'%s' is not a valid subvolume",
> - subvol_name);
> + ctx->subvol_name);
> ret = -EINVAL;
> }
> - if (subvol_objectid && root_objectid != subvol_objectid) {
> + if (ctx->subvolid && root_objectid != ctx->subvolid) {
> /*
> * This will also catch a race condition where a
> * subvolume which was passed by ID is renamed and
> @@ -2288,20 +2283,23 @@ static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid,
> */
> btrfs_err(fs_info,
> "subvol '%s' does not match subvolid %llu",
> - subvol_name, subvol_objectid);
> + ctx->subvol_name, ctx->subvolid);
> ret = -EINVAL;
> }
> if (ret) {
> dput(root);
> - root = ERR_PTR(ret);
> deactivate_locked_super(s);
> + goto out;
> }
> + } else {
> + return PTR_ERR(root);
> }
>
> + fc->root = root;
> + ret = 0;
> +
> out:
> - mntput(mnt);
> - kfree(subvol_name);
> - return root;
> + return ret;
> }
>
> /*
> @@ -2310,27 +2308,20 @@ static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid,
> * Note: This is based on mount_bdev from fs/super.c with a few additions
> * for multiple device setup. Make sure to keep it in sync.
> */
> -static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> - int flags, const char *device_name, void *data)
> +static int btrfs_root_get_tree(struct fs_context *fc)
The new naming is confusing as we use 'get' for reference counting in
connection with 'root' and 'tree'. You can keep btrfs_mount_root as it's
clear it's for mount.
> {
> - struct block_device *bdev = NULL;
> + struct block_device *bdev;
> struct super_block *s;
> + struct btrfs_fs_context *ctx = fc->fs_private;
> struct btrfs_device *device = NULL;
> struct btrfs_fs_devices *fs_devices = NULL;
> struct btrfs_fs_info *fs_info = NULL;
> - void *new_sec_opts = NULL;
> fmode_t mode = FMODE_READ;
> - int error = 0;
> + int error = 0, i;
>
> - if (!(flags & SB_RDONLY))
> + if (!(fc->sb_flags & SB_RDONLY))
> mode |= FMODE_WRITE;
>
> - if (data) {
> - error = security_sb_eat_lsm_opts(data, &new_sec_opts);
> - if (error)
> - return ERR_PTR(error);
> - }
> -
> /*
> * Setup a dummy root and fs_info for test/set super. This is because
> * we don't actually fill this stuff out until open_ctree, but we need
> @@ -2340,10 +2331,9 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> * superblock with our given fs_devices later on at sget() time.
> */
> fs_info = kvzalloc(sizeof(struct btrfs_fs_info), GFP_KERNEL);
> - if (!fs_info) {
> - error = -ENOMEM;
> - goto error_sec_opts;
> - }
> + if (!fs_info)
> + return -ENOMEM;
> +
> btrfs_init_fs_info(fs_info);
>
> fs_info->super_copy = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_KERNEL);
> @@ -2354,13 +2344,20 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> }
>
> mutex_lock(&uuid_mutex);
> - error = btrfs_parse_device_options(data, mode, fs_type);
> - if (error) {
> - mutex_unlock(&uuid_mutex);
> - goto error_fs_info;
> +
> + if (ctx->devices) {
> + for (i = 0; i < ctx->nr_devices; i++) {
> + device = btrfs_scan_one_device(ctx->devices[i], mode,
> + &btrfs_fs_type);
> + if (IS_ERR(device)) {
> + mutex_unlock(&uuid_mutex);
> + error = PTR_ERR(device);
> + goto error_fs_info;
> + }
> + }
> }
>
> - device = btrfs_scan_one_device(device_name, mode, fs_type);
> + device = btrfs_scan_one_device(fc->source, mode, &btrfs_fs_type);
> if (IS_ERR(device)) {
> mutex_unlock(&uuid_mutex);
> error = PTR_ERR(device);
> @@ -2370,53 +2367,49 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> fs_devices = device->fs_devices;
> fs_info->fs_devices = fs_devices;
>
> - error = btrfs_open_devices(fs_devices, mode, fs_type);
> + error = btrfs_open_devices(fs_devices, mode, &btrfs_fs_type);
> mutex_unlock(&uuid_mutex);
> if (error)
> goto error_fs_info;
>
> - if (!(flags & SB_RDONLY) && fs_devices->rw_devices == 0) {
> +
> + if (!(fc->sb_flags & SB_RDONLY) && fs_devices->rw_devices == 0) {
> error = -EACCES;
> goto error_close_devices;
> }
>
> bdev = fs_devices->latest_bdev;
> - s = sget(fs_type, btrfs_test_super, btrfs_set_super, flags | SB_NOSEC,
> - fs_info);
> +
> + fc->s_fs_info = fs_info;
> + s = sget_fc(fc, btrfs_test_super, set_anon_super_fc);
> if (IS_ERR(s)) {
> error = PTR_ERR(s);
> goto error_close_devices;
> }
>
> + /* sget_fc returned a previously allocated superblocl. */
Typo 'superblocl'.
> if (s->s_root) {
> - btrfs_close_devices(fs_devices);
> - btrfs_free_fs_info(fs_info);
> - if ((flags ^ s->s_flags) & SB_RDONLY)
> + btrfs_close_devices(fs_info->fs_devices);
> + if ((fc->sb_flags ^ s->s_flags) & SB_RDONLY)
> error = -EBUSY;
> } else {
> snprintf(s->s_id, sizeof(s->s_id), "%pg", bdev);
> - btrfs_sb(s)->bdev_holder = fs_type;
> - if (!strstr(crc32c_impl(), "generic"))
> - set_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags);
> - error = btrfs_fill_super(s, fs_devices, data);
> - }
> - if (!error)
> - error = security_sb_set_mnt_opts(s, new_sec_opts, 0, NULL);
> - security_free_mnt_opts(&new_sec_opts);
> - if (error) {
> - deactivate_locked_super(s);
> - return ERR_PTR(error);
> + error = btrfs_fill_super(fc, s, fs_devices);
> }
>
> - return dget(s->s_root);
> + if (error)
> + goto error_super;
> +
> + fc->root = dget(s->s_root);
> + return 0;
>
> +error_super:
> + deactivate_locked_super(s);
> error_close_devices:
> btrfs_close_devices(fs_devices);
> error_fs_info:
> - btrfs_free_fs_info(fs_info);
> -error_sec_opts:
> - security_free_mnt_opts(&new_sec_opts);
> - return ERR_PTR(error);
> + /*btrfs_free_fs_info(fs_info);*/
Stray comment
> + return error;
> }
>
> /*
> @@ -2424,7 +2417,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> * btrfs_get_tree will be called recursively, but then will check for the
> * ctx->root being set and call btrfs_root_get_tree.
> */
> -static int btrfs_mount_root_fc(struct fs_context *fc, unsigned int rdonly)
> +int btrfs_mount_root_fc(struct fs_context *fc, unsigned int rdonly)
> {
> struct btrfs_fs_context *ctx, *root_ctx;
> struct fs_context *root_fc;
> @@ -2460,81 +2453,76 @@ static int btrfs_mount_root_fc(struct fs_context *fc, unsigned int rdonly)
> return ret;
> }
>
> +static int btrfs_reconfigure_root_to_rw(struct fs_context *fc,
> + struct super_block *sb)
> +{
> + int error;
> + struct fs_context root_fc = {
> + .purpose = FS_CONTEXT_FOR_RECONFIGURE,
> + .fs_type = sb->s_type,
> + .root = sb->s_root,
> + .log = fc->log,
> + .sb_flags = 0,
> + .sb_flags_mask = SB_RDONLY,
> + };
> +
> + down_write(&sb->s_umount);
> + error = btrfs_reconfigure(&root_fc);
> + up_write(&sb->s_umount);
> + return error;
> +}
> +
> /*
> - * Mount function which is called by VFS layer.
> + * Mount function which is called by VFS layer after the argument parsing.
> *
> * In order to allow mounting a subvolume directly, btrfs uses mount_subtree()
> * which needs vfsmount* of device's root (/). This means device's root has to
> * be mounted internally in any case.
> *
> * Operation flow:
> - * 1. Parse subvol id related options for later use in mount_subvol().
> - *
> - * 2. Mount device's root (/) by calling vfs_kern_mount().
> + * 1. Mount device's root (/) by calling btrfs_mount_root_fc.
> *
> - * NOTE: vfs_kern_mount() is used by VFS to call btrfs_mount() in the
> - * first place. In order to avoid calling btrfs_mount() again, we use
> - * different file_system_type which is not registered to VFS by
> - * register_filesystem() (btrfs_root_fs_type). As a result,
> - * btrfs_mount_root() is called. The return value will be used by
> - * mount_subtree() in mount_subvol().
> + * 2. btrfs_get_tree is called recursively by btrfs_mount_fc -> fc_mount ->
> + * btrfs_get_tree, because we use the same file_system_type for both root and
> + * subvol mounts. We avoid the infinite recursive calls by checking the
> + * ctx->root member being set, which shows that we are trying to mount the
> + * root fs.
> *
> * 3. Call mount_subvol() to get the dentry of subvolume. Since there is
> * "btrfs subvolume set-default", mount_subvol() is called always.
> */
> -static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags,
> - const char *device_name, void *data)
> +static int btrfs_get_tree(struct fs_context *fc)
Please keep 'mount' somewhere in the function name.
> {
> - struct vfsmount *mnt_root;
> - struct dentry *root;
> - char *subvol_name = NULL;
> - u64 subvol_objectid = 0;
> - int error = 0;
> + struct btrfs_fs_context *ctx = fc->fs_private;
> + int error = btrfs_fc_validate(fc);
>
> - error = btrfs_parse_subvol_options(data, &subvol_name,
> - &subvol_objectid);
> - if (error) {
> - kfree(subvol_name);
> - return ERR_PTR(error);
> - }
> + if (error)
> + return error;
>
> - /* mount device's root (/) */
> - mnt_root = vfs_kern_mount(&btrfs_root_fs_type, flags, device_name, data);
> - if (PTR_ERR_OR_ZERO(mnt_root) == -EBUSY) {
> - if (flags & SB_RDONLY) {
> - mnt_root = vfs_kern_mount(&btrfs_root_fs_type,
> - flags & ~SB_RDONLY, device_name, data);
> - } else {
> - mnt_root = vfs_kern_mount(&btrfs_root_fs_type,
> - flags | SB_RDONLY, device_name, data);
> - if (IS_ERR(mnt_root)) {
> - root = ERR_CAST(mnt_root);
> - kfree(subvol_name);
> - goto out;
> - }
> + if (!fc->source)
> + return invalf(fc, "No source specified");
>
> - down_write(&mnt_root->mnt_sb->s_umount);
> - error = btrfs_remount(mnt_root->mnt_sb, &flags, NULL);
> - up_write(&mnt_root->mnt_sb->s_umount);
> - if (error < 0) {
> - root = ERR_PTR(error);
> - mntput(mnt_root);
> - kfree(subvol_name);
> - goto out;
> - }
> + if (ctx->root)
> + return btrfs_root_get_tree(fc);
> +
> + /* mount device's root (/) */
> + error = btrfs_mount_root_fc(fc, fc->sb_flags & SB_RDONLY);
> + if (error == -EBUSY) {
> + /*
> + * If returned EBUSY, try again the mount inverting the rdonly
> + * argument of btrfs_mount_root.
> + */
> + error = btrfs_mount_root_fc(fc, (fc->sb_flags & SB_RDONLY) ^ SB_RDONLY);
> + if (!error && !(fc->sb_flags & SB_RDONLY)) {
> + error = btrfs_reconfigure_root_to_rw(fc,
> + ctx->root_mnt->mnt_sb);
> }
> }
> - if (IS_ERR(mnt_root)) {
> - root = ERR_CAST(mnt_root);
> - kfree(subvol_name);
> - goto out;
> - }
>
> - /* mount_subvol() will free subvol_name and mnt_root */
> - root = mount_subvol(subvol_name, subvol_objectid, mnt_root);
> + if (error < 0)
> + return error;
>
> -out:
> - return root;
> + return mount_subvol(fc);
> }
>
> static void btrfs_resize_thread_pool(struct btrfs_fs_info *fs_info,
> @@ -2598,8 +2586,16 @@ static inline void btrfs_remount_cleanup(struct btrfs_fs_info *fs_info,
> btrfs_discard_cleanup(fs_info);
> }
>
> -static int btrfs_remount(struct super_block *sb, int *flags, char *data)
> +/*
> + * Change the configuration of an active superblock according to the supplied
> + * parameters. Note that the parameter pointer (fc->fs_private) may be NULL in
> + * the case of umount detach, emergency remount R/O and get_tree remounting as
> + * R/W.
> + */
> +int btrfs_reconfigure(struct fs_context *fc)
For some reason I don't like referring to mount-related things by terms
with overloaded meaning. Keeping btrfs_remount would be just fine.
> {
> + struct btrfs_fs_context *ctx = fc->fs_private;
> + struct super_block *sb = fc->root->d_sb;
> struct btrfs_fs_info *fs_info = btrfs_sb(sb);
> struct btrfs_root *root = fs_info->tree_root;
> unsigned old_flags = sb->s_flags;
> @@ -2608,34 +2604,28 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
> u64 old_max_inline = fs_info->max_inline;
> u32 old_thread_pool_size = fs_info->thread_pool_size;
> u32 old_metadata_ratio = fs_info->metadata_ratio;
> - int ret;
> + int ret = btrfs_fc_validate(fc);
> +
> + if (ret)
> + return ret;
>
> sync_filesystem(sb);
> set_bit(BTRFS_FS_STATE_REMOUNTING, &fs_info->fs_state);
> -
> - if (data) {
> - void *new_sec_opts = NULL;
> -
> - ret = security_sb_eat_lsm_opts(data, &new_sec_opts);
> - if (!ret)
> - ret = security_sb_remount(sb, new_sec_opts);
> - security_free_mnt_opts(&new_sec_opts);
> + if (ctx) {
> + ret = btrfs_apply_configuration(fc, sb);
> if (ret)
> goto restore;
> }
>
> - ret = btrfs_parse_options(fs_info, data, *flags);
> - if (ret)
> - goto restore;
> + btrfs_remount_begin(fs_info, old_opts, fc->sb_flags);
> + if (ctx && ctx->thread_pool_size)
> + btrfs_resize_thread_pool(fs_info, ctx->thread_pool_size,
> + old_thread_pool_size);
>
> - btrfs_remount_begin(fs_info, old_opts, *flags);
> - btrfs_resize_thread_pool(fs_info,
> - fs_info->thread_pool_size, old_thread_pool_size);
> -
> - if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
> + if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
> goto out;
>
> - if (*flags & SB_RDONLY) {
> + if (fc->sb_flags & SB_RDONLY) {
> /*
> * this also happens on 'umount -rf' or on shutdown, when
> * the filesystem is busy.
> @@ -2735,7 +2725,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
> * We need to set SB_I_VERSION here otherwise it'll get cleared by VFS,
> * since the absence of the flag means it can be toggled off by remount.
> */
> - *flags |= SB_I_VERSION;
> + fc->sb_flags |= SB_I_VERSION;
>
> wake_up_process(fs_info->transaction_kthread);
> btrfs_remount_cleanup(fs_info, old_opts);
> @@ -3058,9 +3048,11 @@ static void btrfs_fc_free(struct fs_context *fc)
> }
>
> static const struct fs_context_operations btrfs_context_ops = {
> - .dup = btrfs_dup_fc,
> - .free = btrfs_fc_free,
> - .parse_param = btrfs_fc_parse_param,
> + .dup = btrfs_dup_fc,
> + .free = btrfs_fc_free,
> + .get_tree = btrfs_get_tree,
> + .parse_param = btrfs_fc_parse_param,
> + .reconfigure = btrfs_reconfigure,
You should use the final indentation when the callbacks are added and
not at the end.
> };
>
> static int btrfs_init_fs_context(struct fs_context *fc)
> @@ -3072,7 +3064,6 @@ static int btrfs_init_fs_context(struct fs_context *fc)
> return -ENOMEM;
>
> /* currently default options */
> - btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
> #ifdef CONFIG_BTRFS_FS_POSIX_ACL
> fc->sb_flags |= SB_POSIXACL;
> #endif
> @@ -3083,19 +3074,18 @@ static int btrfs_init_fs_context(struct fs_context *fc)
> return 0;
> }
>
> -static struct file_system_type btrfs_fs_type = {
> +struct file_system_type btrfs_fs_type = {
> .owner = THIS_MODULE,
> .name = "btrfs",
> - .mount = btrfs_mount,
> .parameters = btrfs_fs_parameters,
> + .init_fs_context = btrfs_init_fs_context,
> .kill_sb = btrfs_kill_super,
> .fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
> -};
> + };
>
> static struct file_system_type btrfs_root_fs_type = {
> .owner = THIS_MODULE,
> .name = "btrfs",
> - .mount = btrfs_mount_root,
> .parameters = btrfs_fs_parameters,
> .kill_sb = btrfs_kill_super,
> .fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
> @@ -3136,7 +3126,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
> case BTRFS_IOC_SCAN_DEV:
> mutex_lock(&uuid_mutex);
> device = btrfs_scan_one_device(vol->name, FMODE_READ,
> - &btrfs_root_fs_type);
> + &btrfs_fs_type);
> ret = PTR_ERR_OR_ZERO(device);
> mutex_unlock(&uuid_mutex);
> break;
> @@ -3146,7 +3136,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
> case BTRFS_IOC_DEVICES_READY:
> mutex_lock(&uuid_mutex);
> device = btrfs_scan_one_device(vol->name, FMODE_READ,
> - &btrfs_root_fs_type);
> + &btrfs_fs_type);
> if (IS_ERR(device)) {
> mutex_unlock(&uuid_mutex);
> ret = PTR_ERR(device);
> @@ -3237,7 +3227,6 @@ static const struct super_operations btrfs_super_ops = {
> .destroy_inode = btrfs_destroy_inode,
> .free_inode = btrfs_free_inode,
> .statfs = btrfs_statfs,
> - .remount_fs = btrfs_remount,
> .freeze_fs = btrfs_freeze,
> .unfreeze_fs = btrfs_unfreeze,
> };
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index d7670e2a9f39..59ba27e82615 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2304,7 +2304,7 @@ static struct btrfs_device *btrfs_find_device_by_path(
> struct btrfs_device *device;
>
> ret = btrfs_get_bdev_and_sb(device_path, FMODE_READ,
> - fs_info->bdev_holder, 0, &bdev, &disk_super);
> + &btrfs_fs_type, 0, &bdev, &disk_super);
> if (ret)
> return ERR_PTR(ret);
>
> @@ -2516,7 +2516,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> return -EROFS;
>
> bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
> - fs_info->bdev_holder);
> + &btrfs_fs_type);
> if (IS_ERR(bdev))
> return PTR_ERR(bdev);
>
> @@ -6739,7 +6739,7 @@ static struct btrfs_fs_devices *open_seed_devices(struct btrfs_fs_info *fs_info,
> if (IS_ERR(fs_devices))
> return fs_devices;
>
> - ret = open_fs_devices(fs_devices, FMODE_READ, fs_info->bdev_holder);
> + ret = open_fs_devices(fs_devices, FMODE_READ, &btrfs_fs_type);
> if (ret) {
> free_fs_devices(fs_devices);
> fs_devices = ERR_PTR(ret);
> --
> 2.28.0
next prev parent reply other threads:[~2020-08-17 13:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-12 16:36 [RFC PATCH 0/8] btrfs: convert to fscontext Marcos Paulo de Souza
2020-08-12 16:36 ` [RFC PATCH 1/8] btrfs: fs_context: Add initial fscontext parameters Marcos Paulo de Souza
2020-08-12 16:36 ` [RFC PATCH 2/8] btrfs: super: Introduce fs_context ops, init and free functions Marcos Paulo de Souza
2020-08-17 12:50 ` David Sterba
2020-08-12 16:36 ` [RFC PATCH 3/8] btrfs: super: Introduce btrfs_fc_parse_param and btrfs_apply_configuration Marcos Paulo de Souza
2020-08-17 13:09 ` David Sterba
2020-08-12 16:36 ` [RFC PATCH 4/8] btrfs: super: Introduce btrfs_fc_validate Marcos Paulo de Souza
2020-08-12 16:36 ` [RFC PATCH 5/8] btrfs: super: Introduce btrfs_dup_fc Marcos Paulo de Souza
2020-08-12 16:36 ` [RFC PATCH 6/8] btrfs: super: Introduce btrfs_mount_root_fc Marcos Paulo de Souza
2020-08-17 13:14 ` David Sterba
2020-08-12 16:36 ` [RFC PATCH 7/8] btrfs: Convert to fs_context Marcos Paulo de Souza
2020-08-17 13:26 ` David Sterba [this message]
2020-08-12 16:36 ` [RFC PATCH 8/8] btrfs: Remove leftover code from fscontext conversion Marcos Paulo de Souza
2020-08-17 12:44 ` [RFC PATCH 0/8] btrfs: convert to fscontext David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200817132614.GH2026@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=dsterba@suse.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=marcos@mpdesouza.com \
--cc=mpdesouza@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).