From 8d514bbf37eecf0a3e309284728637816a36764b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 16 Nov 2011 16:06:09 -0500 Subject: [PATCH 01/21] btrfs: fix double mntput() in mount_subvol() Signed-off-by: Al Viro --- fs/btrfs/super.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 8bd9d6d..969a774 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -861,7 +861,6 @@ static struct dentry *mount_subvol(const char *subvol_name, int flags, if (!is_subvolume_inode(path.dentry->d_inode)) { path_put(&path); - mntput(mnt); error = -EINVAL; printk(KERN_ERR "btrfs: '%s' is not a valid subvolume\n", subvol_name); -- 1.7.2.5 From c13344958780b4046305ee6235d686c846535529 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 16 Nov 2011 16:12:14 -0500 Subject: [PATCH 02/21] switch create_mnt_ns() to saner calling conventions, fix double mntput() in nfs Life is much saner if create_mnt_ns(mnt) drops mnt in case of error... Switch it to such calling conventions, switch callers, fix double mntput() in fs/nfs/super.c one. Signed-off-by: Al Viro --- fs/btrfs/super.c | 4 +--- fs/namespace.c | 2 ++ fs/nfs/super.c | 23 ++++++++--------------- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 969a774..cfbedd7 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -843,10 +843,8 @@ static struct dentry *mount_subvol(const char *subvol_name, int flags, return ERR_CAST(mnt); ns_private = create_mnt_ns(mnt); - if (IS_ERR(ns_private)) { - mntput(mnt); + if (IS_ERR(ns_private)) return ERR_CAST(ns_private); - } /* * This will trigger the automount of the subvol so we can just diff --git a/fs/namespace.c b/fs/namespace.c index e5e1c7d..aea4b76 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2483,6 +2483,8 @@ struct mnt_namespace *create_mnt_ns(struct vfsmount *mnt) __mnt_make_longterm(mnt); new_ns->root = mnt; list_add(&new_ns->list, &new_ns->root->mnt_list); + } else { + mntput(mnt); } return new_ns; } diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 480b3b6..46d69f38 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -2794,22 +2794,21 @@ static struct dentry *nfs_follow_remote_path(struct vfsmount *root_mnt, int ret; ns_private = create_mnt_ns(root_mnt); - ret = PTR_ERR(ns_private); if (IS_ERR(ns_private)) - goto out_mntput; + return ERR_CAST(ns_private); ret = nfs_referral_loop_protect(); - if (ret != 0) - goto out_put_mnt_ns; - - ret = vfs_path_lookup(root_mnt->mnt_root, root_mnt, - export_path, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &path); + if (ret == 0) { + ret = vfs_path_lookup(root_mnt->mnt_root, root_mnt, + export_path, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, + &path); + nfs_referral_loop_unprotect(); + } - nfs_referral_loop_unprotect(); put_mnt_ns(ns_private); if (ret != 0) - goto out_err; + return ERR_PTR(ret); s = path.mnt->mnt_sb; atomic_inc(&s->s_active); @@ -2818,12 +2817,6 @@ static struct dentry *nfs_follow_remote_path(struct vfsmount *root_mnt, path_put(&path); down_write(&s->s_umount); return dentry; -out_put_mnt_ns: - put_mnt_ns(ns_private); -out_mntput: - mntput(root_mnt); -out_err: - return ERR_PTR(ret); } static struct dentry *nfs4_try_mount(int flags, const char *dev_name, -- 1.7.2.5 From ea441d1104cf1efb471fa81bc91e9fd1e6ae29fd Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 16 Nov 2011 21:43:59 -0500 Subject: [PATCH 03/21] new helper: mount_subtree() takes vfsmount and relative path, does lookup within that vfsmount (possibly triggering automounts) and returns the result as root of subtree suitable for return by ->mount() (i.e. a reference to dentry and an active reference to its superblock grabbed, superblock locked exclusive). btrfs and nfs switched to it instead of open-coding the sucker. Signed-off-by: Al Viro --- fs/btrfs/super.c | 35 ++++++----------------------------- fs/namespace.c | 28 ++++++++++++++++++++++++++++ fs/nfs/super.c | 30 ++++++------------------------ include/linux/fs.h | 1 + 4 files changed, 41 insertions(+), 53 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index cfbedd7..17ee7fc 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -825,13 +825,9 @@ static char *setup_root_args(char *args) static struct dentry *mount_subvol(const char *subvol_name, int flags, const char *device_name, char *data) { - struct super_block *s; struct dentry *root; struct vfsmount *mnt; - struct mnt_namespace *ns_private; char *newargs; - struct path path; - int error; newargs = setup_root_args(data); if (!newargs) @@ -842,36 +838,17 @@ static struct dentry *mount_subvol(const char *subvol_name, int flags, if (IS_ERR(mnt)) return ERR_CAST(mnt); - ns_private = create_mnt_ns(mnt); - if (IS_ERR(ns_private)) - return ERR_CAST(ns_private); - - /* - * This will trigger the automount of the subvol so we can just - * drop the mnt we have here and return the dentry that we - * found. - */ - error = vfs_path_lookup(mnt->mnt_root, mnt, subvol_name, - LOOKUP_FOLLOW, &path); - put_mnt_ns(ns_private); - if (error) - return ERR_PTR(error); + root = mount_subtree(mnt, subvol_name); - if (!is_subvolume_inode(path.dentry->d_inode)) { - path_put(&path); - error = -EINVAL; + if (!IS_ERR(root) && !is_subvolume_inode(root->d_inode)) { + struct super_block *s = root->d_sb; + dput(root); + root = ERR_PTR(-EINVAL); + deactivate_locked_super(s); printk(KERN_ERR "btrfs: '%s' is not a valid subvolume\n", subvol_name); - return ERR_PTR(-EINVAL); } - /* Get a ref to the sb and the dentry we found and return it */ - s = path.mnt->mnt_sb; - atomic_inc(&s->s_active); - root = dget(path.dentry); - path_put(&path); - down_write(&s->s_umount); - return root; } diff --git a/fs/namespace.c b/fs/namespace.c index aea4b76..50ee303 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2490,6 +2490,34 @@ struct mnt_namespace *create_mnt_ns(struct vfsmount *mnt) } EXPORT_SYMBOL(create_mnt_ns); +struct dentry *mount_subtree(struct vfsmount *mnt, const char *name) +{ + struct mnt_namespace *ns; + struct path path; + int err; + + ns = create_mnt_ns(mnt); + if (IS_ERR(ns)) + return ERR_CAST(ns); + + err = vfs_path_lookup(mnt->mnt_root, mnt, + name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &path); + + put_mnt_ns(ns); + + if (err) + return ERR_PTR(err); + + /* trade a vfsmount reference for active sb one */ + atomic_inc(&path.mnt->mnt_sb->s_active); + mntput(path.mnt); + /* lock the sucker */ + down_write(&path.mnt->mnt_sb->s_umount); + /* ... and return the root of (sub)tree on it */ + return path.dentry; +} +EXPORT_SYMBOL(mount_subtree); + SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name, char __user *, type, unsigned long, flags, void __user *, data) { diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 46d69f38..1347774 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -2787,35 +2787,17 @@ static void nfs_referral_loop_unprotect(void) static struct dentry *nfs_follow_remote_path(struct vfsmount *root_mnt, const char *export_path) { - struct mnt_namespace *ns_private; - struct super_block *s; struct dentry *dentry; - struct path path; - int ret; + int ret = nfs_referral_loop_protect(); - ns_private = create_mnt_ns(root_mnt); - if (IS_ERR(ns_private)) - return ERR_CAST(ns_private); - - ret = nfs_referral_loop_protect(); - if (ret == 0) { - ret = vfs_path_lookup(root_mnt->mnt_root, root_mnt, - export_path, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, - &path); - nfs_referral_loop_unprotect(); - } - - put_mnt_ns(ns_private); - - if (ret != 0) + if (ret) { + mntput(root_mnt); return ERR_PTR(ret); + } - s = path.mnt->mnt_sb; - atomic_inc(&s->s_active); - dentry = dget(path.dentry); + dentry = mount_subtree(root_mnt, export_path); + nfs_referral_loop_unprotect(); - path_put(&path); - down_write(&s->s_umount); return dentry; } diff --git a/include/linux/fs.h b/include/linux/fs.h index 0c4df26..e313022 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1886,6 +1886,7 @@ extern struct dentry *mount_single(struct file_system_type *fs_type, extern struct dentry *mount_nodev(struct file_system_type *fs_type, int flags, void *data, int (*fill_super)(struct super_block *, void *, int)); +extern struct dentry *mount_subtree(struct vfsmount *mnt, const char *path); void generic_shutdown_super(struct super_block *sb); void kill_block_super(struct super_block *sb); void kill_anon_super(struct super_block *sb); -- 1.7.2.5 From 6d3ebb5c1abcc049282ed5f4cf3caac298009cb2 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 16 Nov 2011 21:52:06 -0500 Subject: [PATCH 04/21] make nfs_follow_remote_path() handle ERR_PTR() passed as root_mnt ... rather than duplicating that in callers Signed-off-by: Al Viro --- fs/nfs/super.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 1347774..0b39402 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -2788,11 +2788,15 @@ static struct dentry *nfs_follow_remote_path(struct vfsmount *root_mnt, const char *export_path) { struct dentry *dentry; - int ret = nfs_referral_loop_protect(); + int err; - if (ret) { + if (IS_ERR(root_mnt)) + return ERR_CAST(root_mnt); + + err = nfs_referral_loop_protect(); + if (err) { mntput(root_mnt); - return ERR_PTR(ret); + return ERR_PTR(err); } dentry = mount_subtree(root_mnt, export_path); @@ -2816,9 +2820,7 @@ static struct dentry *nfs4_try_mount(int flags, const char *dev_name, data->nfs_server.hostname); data->nfs_server.export_path = export_path; - res = ERR_CAST(root_mnt); - if (!IS_ERR(root_mnt)) - res = nfs_follow_remote_path(root_mnt, export_path); + res = nfs_follow_remote_path(root_mnt, export_path); dfprintk(MOUNT, "<-- nfs4_try_mount() = %ld%s\n", IS_ERR(res) ? PTR_ERR(res) : 0, @@ -3079,9 +3081,7 @@ static struct dentry *nfs4_referral_mount(struct file_system_type *fs_type, flags, data, data->hostname); data->mnt_path = export_path; - res = ERR_CAST(root_mnt); - if (!IS_ERR(root_mnt)) - res = nfs_follow_remote_path(root_mnt, export_path); + res = nfs_follow_remote_path(root_mnt, export_path); dprintk("<-- nfs4_referral_mount() = %ld%s\n", IS_ERR(res) ? PTR_ERR(res) : 0, IS_ERR(res) ? " [error]" : ""); -- 1.7.2.5 From 665112a406f86a8e6889e5a27aeee79caf68a635 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 16 Nov 2011 22:01:36 -0500 Subject: [PATCH 05/21] unexport put_mnt_ns(), make create_mnt_ns() static outright Signed-off-by: Al Viro --- fs/namespace.c | 4 +--- include/linux/mnt_namespace.h | 1 - 2 files changed, 1 insertions(+), 4 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 50ee303..b74f896 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2473,7 +2473,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns, * create_mnt_ns - creates a private namespace and adds a root filesystem * @mnt: pointer to the new root filesystem mountpoint */ -struct mnt_namespace *create_mnt_ns(struct vfsmount *mnt) +static struct mnt_namespace *create_mnt_ns(struct vfsmount *mnt) { struct mnt_namespace *new_ns; @@ -2488,7 +2488,6 @@ struct mnt_namespace *create_mnt_ns(struct vfsmount *mnt) } return new_ns; } -EXPORT_SYMBOL(create_mnt_ns); struct dentry *mount_subtree(struct vfsmount *mnt, const char *name) { @@ -2748,7 +2747,6 @@ void put_mnt_ns(struct mnt_namespace *ns) release_mounts(&umount_list); kfree(ns); } -EXPORT_SYMBOL(put_mnt_ns); struct vfsmount *kern_mount_data(struct file_system_type *type, void *data) { diff --git a/include/linux/mnt_namespace.h b/include/linux/mnt_namespace.h index 2930485..e87ec01 100644 --- a/include/linux/mnt_namespace.h +++ b/include/linux/mnt_namespace.h @@ -22,7 +22,6 @@ struct proc_mounts { struct fs_struct; -extern struct mnt_namespace *create_mnt_ns(struct vfsmount *mnt); extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace *, struct fs_struct *); extern void put_mnt_ns(struct mnt_namespace *ns); -- 1.7.2.5 From 1a9ae01366858d2b59c409cdc5bb9922649ff1cb Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 00:32:06 -0500 Subject: [PATCH 06/21] btrfs: sanitizing ->fs_info, part 1 take assignment of ->fs_info to callers of __setup_root() Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 62afe5c..4f69818 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1142,7 +1142,6 @@ static int __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize, root->orphan_item_inserted = 0; root->orphan_cleanup_state = 0; - root->fs_info = fs_info; root->objectid = objectid; root->last_trans = 0; root->highest_objectid = 0; @@ -1193,6 +1192,7 @@ static int find_and_setup_root(struct btrfs_root *tree_root, u32 blocksize; u64 generation; + root->fs_info = fs_info; __setup_root(tree_root->nodesize, tree_root->leafsize, tree_root->sectorsize, tree_root->stripesize, root, fs_info, objectid); @@ -1227,6 +1227,7 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans, if (!root) return ERR_PTR(-ENOMEM); + root->fs_info = fs_info; __setup_root(tree_root->nodesize, tree_root->leafsize, tree_root->sectorsize, tree_root->stripesize, root, fs_info, BTRFS_TREE_LOG_OBJECTID); @@ -1330,6 +1331,7 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root, goto out; } + root->fs_info = fs_info; __setup_root(tree_root->nodesize, tree_root->leafsize, tree_root->sectorsize, tree_root->stripesize, root, fs_info, location->objectid); @@ -2059,6 +2061,7 @@ struct btrfs_root *open_ctree(struct super_block *sb, init_waitqueue_head(&fs_info->transaction_blocked_wait); init_waitqueue_head(&fs_info->async_submit_wait); + tree_root->fs_info = fs_info; __setup_root(4096, 4096, 4096, 4096, tree_root, fs_info, BTRFS_ROOT_TREE_OBJECTID); @@ -2243,6 +2246,7 @@ struct btrfs_root *open_ctree(struct super_block *sb, btrfs_super_chunk_root_level(disk_super)); generation = btrfs_super_chunk_root_generation(disk_super); + chunk_root->fs_info = fs_info; __setup_root(nodesize, leafsize, sectorsize, stripesize, chunk_root, fs_info, BTRFS_CHUNK_TREE_OBJECTID); @@ -2369,6 +2373,7 @@ retry_root_backup: goto fail_trans_kthread; } + log_tree_root->fs_info = fs_info; __setup_root(nodesize, leafsize, sectorsize, stripesize, log_tree_root, fs_info, BTRFS_TREE_LOG_OBJECTID); -- 1.7.2.5 From dcddb1ee90cb095c442809fb17608efdcf5e772e Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 00:34:00 -0500 Subject: [PATCH 07/21] btrfs: sanitizing ->fs_info, part 2 lift assignment to callers of find_and_setup_root() Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 4f69818..565ed59 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1192,7 +1192,6 @@ static int find_and_setup_root(struct btrfs_root *tree_root, u32 blocksize; u64 generation; - root->fs_info = fs_info; __setup_root(tree_root->nodesize, tree_root->leafsize, tree_root->sectorsize, tree_root->stripesize, root, fs_info, objectid); @@ -1322,6 +1321,7 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root, if (!root) return ERR_PTR(-ENOMEM); if (location->offset == (u64)-1) { + root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, location->objectid, root); if (ret) { @@ -2296,18 +2296,21 @@ retry_root_backup: btrfs_set_root_node(&tree_root->root_item, tree_root->node); tree_root->commit_root = btrfs_root_node(tree_root); + extent_root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, BTRFS_EXTENT_TREE_OBJECTID, extent_root); if (ret) goto recovery_tree_root; extent_root->track_dirty = 1; + dev_root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, BTRFS_DEV_TREE_OBJECTID, dev_root); if (ret) goto recovery_tree_root; dev_root->track_dirty = 1; + csum_root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, BTRFS_CSUM_TREE_OBJECTID, csum_root); if (ret) -- 1.7.2.5 From 6b4a308b26c8509c6fe1e2a32ea8ec5202bccc75 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 00:38:54 -0500 Subject: [PATCH 08/21] btrfs: sanitizing ->fs_info, part 3 move assignments to ->fs_info in open_ctree() up, to the place just after the original allocations. Assignment for tree_root becomes a no-op - we'd obtained fs_info from tree_root->fs_info in the first place. Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 12 +++++------- 1 files changed, 5 insertions(+), 7 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 565ed59..2284789 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1320,8 +1320,8 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root, root = kzalloc(sizeof(*root), GFP_NOFS); if (!root) return ERR_PTR(-ENOMEM); + root->fs_info = fs_info; if (location->offset == (u64)-1) { - root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, location->objectid, root); if (ret) { @@ -1331,7 +1331,6 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root, goto out; } - root->fs_info = fs_info; __setup_root(tree_root->nodesize, tree_root->leafsize, tree_root->sectorsize, tree_root->stripesize, root, fs_info, location->objectid); @@ -1918,6 +1917,10 @@ struct btrfs_root *open_ctree(struct super_block *sb, err = -ENOMEM; goto fail; } + chunk_root->fs_info = fs_info; + extent_root->fs_info = fs_info; + dev_root->fs_info = fs_info; + csum_root->fs_info = fs_info; ret = init_srcu_struct(&fs_info->subvol_srcu); if (ret) { @@ -2061,7 +2064,6 @@ struct btrfs_root *open_ctree(struct super_block *sb, init_waitqueue_head(&fs_info->transaction_blocked_wait); init_waitqueue_head(&fs_info->async_submit_wait); - tree_root->fs_info = fs_info; __setup_root(4096, 4096, 4096, 4096, tree_root, fs_info, BTRFS_ROOT_TREE_OBJECTID); @@ -2246,7 +2248,6 @@ struct btrfs_root *open_ctree(struct super_block *sb, btrfs_super_chunk_root_level(disk_super)); generation = btrfs_super_chunk_root_generation(disk_super); - chunk_root->fs_info = fs_info; __setup_root(nodesize, leafsize, sectorsize, stripesize, chunk_root, fs_info, BTRFS_CHUNK_TREE_OBJECTID); @@ -2296,21 +2297,18 @@ retry_root_backup: btrfs_set_root_node(&tree_root->root_item, tree_root->node); tree_root->commit_root = btrfs_root_node(tree_root); - extent_root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, BTRFS_EXTENT_TREE_OBJECTID, extent_root); if (ret) goto recovery_tree_root; extent_root->track_dirty = 1; - dev_root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, BTRFS_DEV_TREE_OBJECTID, dev_root); if (ret) goto recovery_tree_root; dev_root->track_dirty = 1; - csum_root->fs_info = fs_info; ret = find_and_setup_root(tree_root, fs_info, BTRFS_CSUM_TREE_OBJECTID, csum_root); if (ret) -- 1.7.2.5 From a4f59b172ba8a501f057c1cf9d54a2928a4aaed3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 00:46:16 -0500 Subject: [PATCH 09/21] btrfs: sanitizing ->fs_info, part 4 A new helper: btrfs_alloc_root(fs_info); allocates btrfs_root and sets ->fs_info. All places allocating the suckers converted to it. At that point we *never* reassign ->fs_info of btrfs_root; it's set before anyone sees the address of newly allocated struct btrfs_root and never assigned anywhere else. Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 33 +++++++++++++++------------------ fs/btrfs/disk-io.h | 2 ++ fs/btrfs/super.c | 3 +-- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 2284789..9e98eec 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1215,6 +1215,14 @@ static int find_and_setup_root(struct btrfs_root *tree_root, return 0; } +struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info) +{ + struct btrfs_root *root = kzalloc(sizeof(*root), GFP_NOFS); + if (root) + root->fs_info = fs_info; + return root; +} + static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info) { @@ -1222,11 +1230,10 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans, struct btrfs_root *tree_root = fs_info->tree_root; struct extent_buffer *leaf; - root = kzalloc(sizeof(*root), GFP_NOFS); + root = btrfs_alloc_root(fs_info); if (!root) return ERR_PTR(-ENOMEM); - root->fs_info = fs_info; __setup_root(tree_root->nodesize, tree_root->leafsize, tree_root->sectorsize, tree_root->stripesize, root, fs_info, BTRFS_TREE_LOG_OBJECTID); @@ -1317,10 +1324,9 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root, u32 blocksize; int ret = 0; - root = kzalloc(sizeof(*root), GFP_NOFS); + root = btrfs_alloc_root(fs_info); if (!root) return ERR_PTR(-ENOMEM); - root->fs_info = fs_info; if (location->offset == (u64)-1) { ret = find_and_setup_root(tree_root, fs_info, location->objectid, root); @@ -1904,23 +1910,15 @@ struct btrfs_root *open_ctree(struct super_block *sb, int num_backups_tried = 0; int backup_index = 0; - extent_root = fs_info->extent_root = - kzalloc(sizeof(struct btrfs_root), GFP_NOFS); - csum_root = fs_info->csum_root = - kzalloc(sizeof(struct btrfs_root), GFP_NOFS); - chunk_root = fs_info->chunk_root = - kzalloc(sizeof(struct btrfs_root), GFP_NOFS); - dev_root = fs_info->dev_root = - kzalloc(sizeof(struct btrfs_root), GFP_NOFS); + extent_root = fs_info->extent_root = btrfs_alloc_root(fs_info); + csum_root = fs_info->csum_root = btrfs_alloc_root(fs_info); + chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info); + dev_root = fs_info->dev_root = btrfs_alloc_root(fs_info); if (!extent_root || !csum_root || !chunk_root || !dev_root) { err = -ENOMEM; goto fail; } - chunk_root->fs_info = fs_info; - extent_root->fs_info = fs_info; - dev_root->fs_info = fs_info; - csum_root->fs_info = fs_info; ret = init_srcu_struct(&fs_info->subvol_srcu); if (ret) { @@ -2368,13 +2366,12 @@ retry_root_backup: btrfs_level_size(tree_root, btrfs_super_log_root_level(disk_super)); - log_tree_root = kzalloc(sizeof(struct btrfs_root), GFP_NOFS); + log_tree_root = btrfs_alloc_root(fs_info); if (!log_tree_root) { err = -ENOMEM; goto fail_trans_kthread; } - log_tree_root->fs_info = fs_info; __setup_root(nodesize, leafsize, sectorsize, stripesize, log_tree_root, fs_info, BTRFS_TREE_LOG_OBJECTID); diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h index c99d0a8f..2bb5f59 100644 --- a/fs/btrfs/disk-io.h +++ b/fs/btrfs/disk-io.h @@ -86,6 +86,8 @@ int btrfs_init_log_root_tree(struct btrfs_trans_handle *trans, int btrfs_add_log_tree(struct btrfs_trans_handle *trans, struct btrfs_root *root); +struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info); + #ifdef CONFIG_DEBUG_LOCK_ALLOC void btrfs_init_lockdep(void); void btrfs_set_buffer_lockdep_class(u64 objectid, diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 17ee7fc..1c63467 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -903,12 +903,11 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, if (!fs_info) return ERR_PTR(-ENOMEM); - fs_info->tree_root = kzalloc(sizeof(struct btrfs_root), GFP_NOFS); + fs_info->tree_root = btrfs_alloc_root(fs_info); if (!fs_info->tree_root) { error = -ENOMEM; goto error_fs_info; } - fs_info->tree_root->fs_info = fs_info; fs_info->fs_devices = fs_devices; fs_info->super_copy = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_NOFS); -- 1.7.2.5 From d772a48dcc45447910fbcf47a8d75aea23ee4c53 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 00:56:18 -0500 Subject: [PATCH 10/21] btrfs: sanitizing ->fs_info, part 5 close_ctree() uses a weird mix of accesses to root->fs_info and its value at the beginning of function stored in local variable. Since ->fs_info *never* changes, let's just use the local variable to avoid confusion. Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 9e98eec..5b43477 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2866,7 +2866,7 @@ int close_ctree(struct btrfs_root *root) (atomic_read(&fs_info->defrag_running) == 0)); /* clear out the rbtree of defraggable inodes */ - btrfs_run_defrag_inodes(root->fs_info); + btrfs_run_defrag_inodes(fs_info); /* * Here come 2 situations when btrfs is broken to flip readonly: @@ -2895,8 +2895,8 @@ int close_ctree(struct btrfs_root *root) btrfs_put_block_group_cache(fs_info); - kthread_stop(root->fs_info->transaction_kthread); - kthread_stop(root->fs_info->cleaner_kthread); + kthread_stop(fs_info->transaction_kthread); + kthread_stop(fs_info->cleaner_kthread); fs_info->closing = 2; smp_mb(); @@ -2914,14 +2914,14 @@ int close_ctree(struct btrfs_root *root) free_extent_buffer(fs_info->extent_root->commit_root); free_extent_buffer(fs_info->tree_root->node); free_extent_buffer(fs_info->tree_root->commit_root); - free_extent_buffer(root->fs_info->chunk_root->node); - free_extent_buffer(root->fs_info->chunk_root->commit_root); - free_extent_buffer(root->fs_info->dev_root->node); - free_extent_buffer(root->fs_info->dev_root->commit_root); - free_extent_buffer(root->fs_info->csum_root->node); - free_extent_buffer(root->fs_info->csum_root->commit_root); - - btrfs_free_block_groups(root->fs_info); + free_extent_buffer(fs_info->chunk_root->node); + free_extent_buffer(fs_info->chunk_root->commit_root); + free_extent_buffer(fs_info->dev_root->node); + free_extent_buffer(fs_info->dev_root->commit_root); + free_extent_buffer(fs_info->csum_root->node); + free_extent_buffer(fs_info->csum_root->commit_root); + + btrfs_free_block_groups(fs_info); del_fs_roots(fs_info); -- 1.7.2.5 From 3a52336966dbe660b2a9f19f312a5c53441142cd Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:00:31 -0500 Subject: [PATCH 11/21] btrfs: sanitizing ->fs_info, part 6 lift free_fs_info() to callers of close_ctree() Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 3 +-- fs/btrfs/super.c | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 5b43477..f32e35e 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2423,6 +2423,7 @@ retry_root_backup: up_read(&fs_info->cleanup_work_sem); if (err) { close_ctree(tree_root); + free_fs_info(fs_info); return ERR_PTR(err); } } @@ -2947,8 +2948,6 @@ int close_ctree(struct btrfs_root *root) bdi_destroy(&fs_info->bdi); cleanup_srcu_struct(&fs_info->subvol_srcu); - free_fs_info(fs_info); - return 0; } diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 1c63467..8cc6a11 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -151,6 +151,7 @@ static void btrfs_put_super(struct super_block *sb) int ret; ret = close_ctree(root); + free_fs_info(root->fs_info); sb->s_fs_info = NULL; (void)ret; /* FIXME: need to fix VFS to return error? */ @@ -589,6 +590,7 @@ static int btrfs_fill_super(struct super_block *sb, struct inode *inode; struct dentry *root_dentry; struct btrfs_root *tree_root; + struct btrfs_fs_info *fs_info; struct btrfs_key key; int err; @@ -609,12 +611,13 @@ static int btrfs_fill_super(struct super_block *sb, printk("btrfs: open_ctree failed\n"); return PTR_ERR(tree_root); } + fs_info = tree_root->fs_info; sb->s_fs_info = tree_root; key.objectid = BTRFS_FIRST_FREE_OBJECTID; key.type = BTRFS_INODE_ITEM_KEY; key.offset = 0; - inode = btrfs_iget(sb, &key, tree_root->fs_info->fs_root, NULL); + inode = btrfs_iget(sb, &key, fs_info->fs_root, NULL); if (IS_ERR(inode)) { err = PTR_ERR(inode); goto fail_close; @@ -635,6 +638,7 @@ static int btrfs_fill_super(struct super_block *sb, fail_close: close_ctree(tree_root); + free_fs_info(fs_info); return err; } -- 1.7.2.5 From eea31b242641d6ec0da3af9d4acc2b032673792a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:10:02 -0500 Subject: [PATCH 12/21] btrfs: make open_ctree() return int It returns either ERR_PTR(-ve) or sb->s_fs_info. The latter can be found by caller just as well, TYVM, no need to return it. Just return -ve or 0... Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 12 ++++++------ fs/btrfs/disk-io.h | 6 +++--- fs/btrfs/super.c | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index f32e35e..8abeb5d 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1884,9 +1884,9 @@ static void free_root_pointers(struct btrfs_fs_info *info, int chunk_root) } -struct btrfs_root *open_ctree(struct super_block *sb, - struct btrfs_fs_devices *fs_devices, - char *options) +int open_ctree(struct super_block *sb, + struct btrfs_fs_devices *fs_devices, + char *options) { u32 sectorsize; u32 nodesize; @@ -2424,11 +2424,11 @@ retry_root_backup: if (err) { close_ctree(tree_root); free_fs_info(fs_info); - return ERR_PTR(err); + return err; } } - return tree_root; + return 0; fail_trans_kthread: kthread_stop(fs_info->transaction_kthread); @@ -2475,7 +2475,7 @@ fail_srcu: fail: btrfs_close_devices(fs_info->fs_devices); free_fs_info(fs_info); - return ERR_PTR(err); + return err; recovery_tree_root: if (!btrfs_test_opt(tree_root, RECOVERY)) diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h index 2bb5f59..6f5f487 100644 --- a/fs/btrfs/disk-io.h +++ b/fs/btrfs/disk-io.h @@ -46,9 +46,9 @@ struct extent_buffer *btrfs_find_create_tree_block(struct btrfs_root *root, u64 bytenr, u32 blocksize); int clean_tree_block(struct btrfs_trans_handle *trans, struct btrfs_root *root, struct extent_buffer *buf); -struct btrfs_root *open_ctree(struct super_block *sb, - struct btrfs_fs_devices *fs_devices, - char *options); +int open_ctree(struct super_block *sb, + struct btrfs_fs_devices *fs_devices, + char *options); int close_ctree(struct btrfs_root *root); int write_ctree_super(struct btrfs_trans_handle *trans, struct btrfs_root *root, int max_mirrors); diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 8cc6a11..d651908 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -605,12 +605,12 @@ static int btrfs_fill_super(struct super_block *sb, sb->s_flags |= MS_POSIXACL; #endif - tree_root = open_ctree(sb, fs_devices, (char *)data); - - if (IS_ERR(tree_root)) { + err = open_ctree(sb, fs_devices, (char *)data); + if (err) { printk("btrfs: open_ctree failed\n"); - return PTR_ERR(tree_root); + return err; } + tree_root = sb->s_fs_info; fs_info = tree_root->fs_info; sb->s_fs_info = tree_root; -- 1.7.2.5 From 4aeb5b0945694918433e96b05595d966a22d042a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:12:38 -0500 Subject: [PATCH 13/21] btrfs: kill pointless reassignment of ->s_fs_info in btrfs_fill_super() We do not (fortunately) modify ->s_fs_info of superblock on the fly in btrfs_fill_super(); apparent assignment is a no-op. Signed-off-by: Al Viro --- fs/btrfs/super.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index d651908..9e18cda 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -589,8 +589,8 @@ static int btrfs_fill_super(struct super_block *sb, { struct inode *inode; struct dentry *root_dentry; - struct btrfs_root *tree_root; - struct btrfs_fs_info *fs_info; + struct btrfs_root *tree_root = sb->s_fs_info; + struct btrfs_fs_info *fs_info = tree_root->fs_info; struct btrfs_key key; int err; @@ -610,9 +610,6 @@ static int btrfs_fill_super(struct super_block *sb, printk("btrfs: open_ctree failed\n"); return err; } - tree_root = sb->s_fs_info; - fs_info = tree_root->fs_info; - sb->s_fs_info = tree_root; key.objectid = BTRFS_FIRST_FREE_OBJECTID; key.type = BTRFS_INODE_ITEM_KEY; -- 1.7.2.5 From 47fec7f8a72759b8f83a7d880e89b39384ac5905 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:22:46 -0500 Subject: [PATCH 14/21] btrfs: get ->kill_sb() of its own ... and move free_fs_info() to that, out of ->put_super(). Do NOT set ->s_fs_info to NULL in the latter; we need it for sget() to be able to see and wait for fs in the middle of umount if we get a mount/umount race. Signed-off-by: Al Viro --- fs/btrfs/super.c | 27 ++++++++++++++++++--------- 1 files changed, 18 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 9e18cda..353d555 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -147,14 +147,13 @@ void __btrfs_std_error(struct btrfs_fs_info *fs_info, const char *function, static void btrfs_put_super(struct super_block *sb) { - struct btrfs_root *root = btrfs_sb(sb); - int ret; - - ret = close_ctree(root); - free_fs_info(root->fs_info); - sb->s_fs_info = NULL; - - (void)ret; /* FIXME: need to fix VFS to return error? */ + (void)close_ctree(btrfs_sb(sb)); + /* FIXME: need to fix VFS to return error? */ + /* AV: return it _where_? ->put_super() can be triggered by any number + * of async events, up to and including delivery of SIGKILL to the + * last process that kept it busy. Or segfault in the aforementioned + * process... Whom would you report that to? + */ } enum { @@ -1212,11 +1211,21 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf) return 0; } +static void btrfs_kill_super(struct super_block *sb) +{ + struct btrfs_fs_info *fs_info = NULL; + if (sb->s_root) + fs_info = btrfs_sb(sb)->fs_info; + kill_anon_super(sb); + if (fs_info) + free_fs_info(fs_info); +} + static struct file_system_type btrfs_fs_type = { .owner = THIS_MODULE, .name = "btrfs", .mount = btrfs_mount, - .kill_sb = kill_anon_super, + .kill_sb = btrfs_kill_super, .fs_flags = FS_REQUIRES_DEV, }; -- 1.7.2.5 From af38b4da9455d401ee820eb5e277c72d6d7d8387 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:29:09 -0500 Subject: [PATCH 15/21] btrfs: fix mount/umount race Do *NOT* skip doomed superblocks in btrfs_test_super(); we want sget() to wait for their shutdown to complete. Since we don't mutilate ->s_fs_info in ->put_super() anymore (or free what it used to point to until the superblock is past being findable by sget()), we can just DTRT there and report a match. Signed-off-by: Al Viro --- fs/btrfs/super.c | 13 ++++--------- 1 files changed, 4 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 353d555..c58708c 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -730,20 +730,15 @@ static int btrfs_test_super(struct super_block *s, void *data) struct btrfs_root *test_root = data; struct btrfs_root *root = btrfs_sb(s); - /* - * If this super block is going away, return false as it - * can't match as an existing super block. - */ - if (!atomic_read(&s->s_active)) - return 0; return root->fs_info->fs_devices == test_root->fs_info->fs_devices; } static int btrfs_set_super(struct super_block *s, void *data) { - s->s_fs_info = data; - - return set_anon_super(s, data); + int err = set_anon_super(s, data); + if (!err) + s->s_fs_info = data; + return err; } /* -- 1.7.2.5 From a2c7148fd8c4c6b29ed776e3515cbd3745a65e53 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:40:05 -0500 Subject: [PATCH 16/21] btrfs: merge free_fs_info() calls on fill_super failures ... all the way up into btrfs_mount(). Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 2 -- fs/btrfs/super.c | 2 +- 2 files changed, 1 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 8abeb5d..214309c 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2423,7 +2423,6 @@ retry_root_backup: up_read(&fs_info->cleanup_work_sem); if (err) { close_ctree(tree_root); - free_fs_info(fs_info); return err; } } @@ -2474,7 +2473,6 @@ fail_srcu: cleanup_srcu_struct(&fs_info->subvol_srcu); fail: btrfs_close_devices(fs_info->fs_devices); - free_fs_info(fs_info); return err; recovery_tree_root: diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index c58708c..3ca7703 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -634,7 +634,6 @@ static int btrfs_fill_super(struct super_block *sb, fail_close: close_ctree(tree_root); - free_fs_info(fs_info); return err; } @@ -947,6 +946,7 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, error = btrfs_fill_super(s, fs_devices, data, flags & MS_SILENT ? 1 : 0); if (error) { + free_fs_info(fs_info); deactivate_locked_super(s); return ERR_PTR(error); } -- 1.7.2.5 From a45a3b8e3db10730320b8eeaa77033a15d11b3c6 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:46:50 -0500 Subject: [PATCH 17/21] btrfs: make free_fs_info() call ->kill_sb() unconditional ... and don't bother with it after btrfs_fill_super() failure - ->kill_sb() (unlike ->put_super()) will be called even if we have not got non-NULL ->s_root. Signed-off-by: Al Viro --- fs/btrfs/super.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 3ca7703..dc48e60 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -946,7 +946,6 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, error = btrfs_fill_super(s, fs_devices, data, flags & MS_SILENT ? 1 : 0); if (error) { - free_fs_info(fs_info); deactivate_locked_super(s); return ERR_PTR(error); } @@ -1208,12 +1207,9 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf) static void btrfs_kill_super(struct super_block *sb) { - struct btrfs_fs_info *fs_info = NULL; - if (sb->s_root) - fs_info = btrfs_sb(sb)->fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(sb)->fs_info; kill_anon_super(sb); - if (fs_info) - free_fs_info(fs_info); + free_fs_info(fs_info); } static struct file_system_type btrfs_fs_type = { -- 1.7.2.5 From 40e959495a7e738cc5003adde28238bb8ebe3fa9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 01:56:28 -0500 Subject: [PATCH 18/21] btrfs: consolidate failure exits in btrfs_mount() a bit Signed-off-by: Al Viro --- fs/btrfs/super.c | 21 +++++---------------- 1 files changed, 5 insertions(+), 16 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index dc48e60..72fb547 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -630,6 +630,7 @@ static int btrfs_fill_super(struct super_block *sb, save_mount_options(sb, data); cleancache_init_fs(sb); + sb->s_flags |= MS_ACTIVE; return 0; fail_close: @@ -929,14 +930,10 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, } if (s->s_root) { - if ((flags ^ s->s_flags) & MS_RDONLY) { - deactivate_locked_super(s); - error = -EBUSY; - goto error_close_devices; - } - btrfs_close_devices(fs_devices); free_fs_info(fs_info); + if ((flags ^ s->s_flags) & MS_RDONLY) + error = -EBUSY; } else { char b[BDEVNAME_SIZE]; @@ -945,19 +942,11 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, btrfs_sb(s)->fs_info->bdev_holder = fs_type; error = btrfs_fill_super(s, fs_devices, data, flags & MS_SILENT ? 1 : 0); - if (error) { - deactivate_locked_super(s); - return ERR_PTR(error); - } - - s->s_flags |= MS_ACTIVE; } - root = get_default_root(s, subvol_objectid); - if (IS_ERR(root)) { + root = !error ? get_default_root(s, subvol_objectid) : ERR_PTR(error); + if (IS_ERR(root)) deactivate_locked_super(s); - return root; - } return root; -- 1.7.2.5 From 96ed1e6937f82b71ebde6bf64192fb40e4200893 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 15:05:22 -0500 Subject: [PATCH 19/21] btrfs: fix a deadlock in btrfs_scan_one_device() pathname resolution under a global mutex, taken on some paths in ->mount() is a Bad Idea(tm) - think what happens if said pathname resolution triggers automount of some btrfs instance and walks into attempt to grab the same mutex. Deadlock - we are waiting for daemon to finish walking the path, daemon is waiting for us to release the mutex... Signed-off-by: Al Viro --- fs/btrfs/volumes.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index c37433d..941fd46 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -700,8 +700,6 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, u64 devid; u64 transid; - mutex_lock(&uuid_mutex); - flags |= FMODE_EXCL; bdev = blkdev_get_by_path(path, flags, holder); @@ -710,6 +708,7 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, goto error; } + mutex_lock(&uuid_mutex); ret = set_blocksize(bdev, 4096); if (ret) goto error_close; @@ -731,9 +730,9 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, brelse(bh); error_close: + mutex_unlock(&uuid_mutex); blkdev_put(bdev, flags); error: - mutex_unlock(&uuid_mutex); return ret; } -- 1.7.2.5 From c5a02673fe89d13d0d07d7677f6d1e191432f6da Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 15:40:49 -0500 Subject: [PATCH 20/21] btrfs: let ->s_fs_info point to fs_info, not root... the latter can be obtained from the former (by looking as ->tree_root) just as cheaply as we currently are doing the other way round. Signed-off-by: Al Viro --- fs/btrfs/ctree.h | 2 +- fs/btrfs/disk-io.c | 4 +- fs/btrfs/export.c | 2 +- fs/btrfs/ioctl.c | 7 ++--- fs/btrfs/super.c | 75 ++++++++++++++++++++++++++------------------------- 5 files changed, 45 insertions(+), 45 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index b9ba59f..d14f993 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -2193,7 +2193,7 @@ static inline u32 btrfs_file_extent_inline_item_len(struct extent_buffer *eb, return btrfs_item_size(eb, e) - offset; } -static inline struct btrfs_root *btrfs_sb(struct super_block *sb) +static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb) { return sb->s_fs_info; } diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 214309c..a78c762 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1898,8 +1898,8 @@ int open_ctree(struct super_block *sb, struct btrfs_key location; struct buffer_head *bh; struct btrfs_super_block *disk_super; - struct btrfs_root *tree_root = btrfs_sb(sb); - struct btrfs_fs_info *fs_info = tree_root->fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(sb); + struct btrfs_root *tree_root = fs_info->tree_root; struct btrfs_root *extent_root; struct btrfs_root *csum_root; struct btrfs_root *chunk_root; diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c index 1b8dc33..5f77166 100644 --- a/fs/btrfs/export.c +++ b/fs/btrfs/export.c @@ -67,7 +67,7 @@ static struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid, u64 root_objectid, u32 generation, int check_generation) { - struct btrfs_fs_info *fs_info = btrfs_sb(sb)->fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(sb); struct btrfs_root *root; struct inode *inode; struct btrfs_key key; diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 4a34c47..dc40da5 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -276,14 +276,13 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg) static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) { - struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info; - struct btrfs_fs_info *fs_info = root->fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(fdentry(file)->d_sb); struct btrfs_device *device; struct request_queue *q; struct fstrim_range range; u64 minlen = ULLONG_MAX; u64 num_devices = 0; - u64 total_bytes = btrfs_super_total_bytes(root->fs_info->super_copy); + u64 total_bytes = btrfs_super_total_bytes(fs_info->super_copy); int ret; if (!capable(CAP_SYS_ADMIN)) @@ -312,7 +311,7 @@ static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) range.len = min(range.len, total_bytes - range.start); range.minlen = max(range.minlen, minlen); - ret = btrfs_trim_fs(root, &range); + ret = btrfs_trim_fs(fs_info->tree_root, &range); if (ret < 0) return ret; diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 72fb547..1f7b3e2 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -147,7 +147,7 @@ void __btrfs_std_error(struct btrfs_fs_info *fs_info, const char *function, static void btrfs_put_super(struct super_block *sb) { - (void)close_ctree(btrfs_sb(sb)); + (void)close_ctree(btrfs_sb(sb)->tree_root); /* FIXME: need to fix VFS to return error? */ /* AV: return it _where_? ->put_super() can be triggered by any number * of async events, up to and including delivery of SIGKILL to the @@ -500,7 +500,8 @@ out: static struct dentry *get_default_root(struct super_block *sb, u64 subvol_objectid) { - struct btrfs_root *root = sb->s_fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(sb); + struct btrfs_root *root = fs_info->tree_root; struct btrfs_root *new_root; struct btrfs_dir_item *di; struct btrfs_path *path; @@ -530,7 +531,7 @@ static struct dentry *get_default_root(struct super_block *sb, * will mount by default if we haven't been given a specific subvolume * to mount. */ - dir_id = btrfs_super_root_dir(root->fs_info->super_copy); + dir_id = btrfs_super_root_dir(fs_info->super_copy); di = btrfs_lookup_dir_item(NULL, root, path, dir_id, "default", 7, 0); if (IS_ERR(di)) { btrfs_free_path(path); @@ -544,7 +545,7 @@ static struct dentry *get_default_root(struct super_block *sb, */ btrfs_free_path(path); dir_id = BTRFS_FIRST_FREE_OBJECTID; - new_root = root->fs_info->fs_root; + new_root = fs_info->fs_root; goto setup_root; } @@ -552,7 +553,7 @@ static struct dentry *get_default_root(struct super_block *sb, btrfs_free_path(path); find_root: - new_root = btrfs_read_fs_root_no_name(root->fs_info, &location); + new_root = btrfs_read_fs_root_no_name(fs_info, &location); if (IS_ERR(new_root)) return ERR_CAST(new_root); @@ -588,8 +589,7 @@ static int btrfs_fill_super(struct super_block *sb, { struct inode *inode; struct dentry *root_dentry; - struct btrfs_root *tree_root = sb->s_fs_info; - struct btrfs_fs_info *fs_info = tree_root->fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(sb); struct btrfs_key key; int err; @@ -634,20 +634,21 @@ static int btrfs_fill_super(struct super_block *sb, return 0; fail_close: - close_ctree(tree_root); + close_ctree(fs_info->tree_root); return err; } int btrfs_sync_fs(struct super_block *sb, int wait) { struct btrfs_trans_handle *trans; - struct btrfs_root *root = btrfs_sb(sb); + struct btrfs_fs_info *fs_info = btrfs_sb(sb); + struct btrfs_root *root = fs_info->tree_root; int ret; trace_btrfs_sync_fs(wait); if (!wait) { - filemap_flush(root->fs_info->btree_inode->i_mapping); + filemap_flush(fs_info->btree_inode->i_mapping); return 0; } @@ -663,8 +664,8 @@ int btrfs_sync_fs(struct super_block *sb, int wait) static int btrfs_show_options(struct seq_file *seq, struct vfsmount *vfs) { - struct btrfs_root *root = btrfs_sb(vfs->mnt_sb); - struct btrfs_fs_info *info = root->fs_info; + struct btrfs_fs_info *info = btrfs_sb(vfs->mnt_sb); + struct btrfs_root *root = info->tree_root; char *compress_type; if (btrfs_test_opt(root, DEGRADED)) @@ -727,10 +728,10 @@ static int btrfs_show_options(struct seq_file *seq, struct vfsmount *vfs) static int btrfs_test_super(struct super_block *s, void *data) { - struct btrfs_root *test_root = data; - struct btrfs_root *root = btrfs_sb(s); + struct btrfs_fs_info *p = data; + struct btrfs_fs_info *fs_info = btrfs_sb(s); - return root->fs_info->fs_devices == test_root->fs_info->fs_devices; + return fs_info->fs_devices == p->fs_devices; } static int btrfs_set_super(struct super_block *s, void *data) @@ -922,8 +923,7 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, } bdev = fs_devices->latest_bdev; - s = sget(fs_type, btrfs_test_super, btrfs_set_super, - fs_info->tree_root); + s = sget(fs_type, btrfs_test_super, btrfs_set_super, fs_info); if (IS_ERR(s)) { error = PTR_ERR(s); goto error_close_devices; @@ -939,7 +939,7 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, s->s_flags = flags | MS_NOSEC; strlcpy(s->s_id, bdevname(bdev, b), sizeof(s->s_id)); - btrfs_sb(s)->fs_info->bdev_holder = fs_type; + btrfs_sb(s)->bdev_holder = fs_type; error = btrfs_fill_super(s, fs_devices, data, flags & MS_SILENT ? 1 : 0); } @@ -959,7 +959,8 @@ error_fs_info: static int btrfs_remount(struct super_block *sb, int *flags, char *data) { - struct btrfs_root *root = btrfs_sb(sb); + struct btrfs_fs_info *fs_info = btrfs_sb(sb); + struct btrfs_root *root = fs_info->tree_root; int ret; ret = btrfs_parse_options(root, data); @@ -975,13 +976,13 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) ret = btrfs_commit_super(root); WARN_ON(ret); } else { - if (root->fs_info->fs_devices->rw_devices == 0) + if (fs_info->fs_devices->rw_devices == 0) return -EACCES; - if (btrfs_super_log_root(root->fs_info->super_copy) != 0) + if (btrfs_super_log_root(fs_info->super_copy) != 0) return -EINVAL; - ret = btrfs_cleanup_fs_roots(root->fs_info); + ret = btrfs_cleanup_fs_roots(fs_info); WARN_ON(ret); /* recover relocation */ @@ -1143,18 +1144,18 @@ static int btrfs_calc_avail_data_space(struct btrfs_root *root, u64 *free_bytes) static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf) { - struct btrfs_root *root = btrfs_sb(dentry->d_sb); - struct btrfs_super_block *disk_super = root->fs_info->super_copy; - struct list_head *head = &root->fs_info->space_info; + struct btrfs_fs_info *fs_info = btrfs_sb(dentry->d_sb); + struct btrfs_super_block *disk_super = fs_info->super_copy; + struct list_head *head = &fs_info->space_info; struct btrfs_space_info *found; u64 total_used = 0; u64 total_free_data = 0; int bits = dentry->d_sb->s_blocksize_bits; - __be32 *fsid = (__be32 *)root->fs_info->fsid; + __be32 *fsid = (__be32 *)fs_info->fsid; int ret; /* holding chunk_muext to avoid allocating new chunks */ - mutex_lock(&root->fs_info->chunk_mutex); + mutex_lock(&fs_info->chunk_mutex); rcu_read_lock(); list_for_each_entry_rcu(found, head, list) { if (found->flags & BTRFS_BLOCK_GROUP_DATA) { @@ -1173,14 +1174,14 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf) buf->f_bsize = dentry->d_sb->s_blocksize; buf->f_type = BTRFS_SUPER_MAGIC; buf->f_bavail = total_free_data; - ret = btrfs_calc_avail_data_space(root, &total_free_data); + ret = btrfs_calc_avail_data_space(fs_info->tree_root, &total_free_data); if (ret) { - mutex_unlock(&root->fs_info->chunk_mutex); + mutex_unlock(&fs_info->chunk_mutex); return ret; } buf->f_bavail += total_free_data; buf->f_bavail = buf->f_bavail >> bits; - mutex_unlock(&root->fs_info->chunk_mutex); + mutex_unlock(&fs_info->chunk_mutex); /* We treat it as constant endianness (it doesn't matter _which_) because we want the fsid to come out the same whether mounted @@ -1196,7 +1197,7 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf) static void btrfs_kill_super(struct super_block *sb) { - struct btrfs_fs_info *fs_info = btrfs_sb(sb)->fs_info; + struct btrfs_fs_info *fs_info = btrfs_sb(sb); kill_anon_super(sb); free_fs_info(fs_info); } @@ -1239,17 +1240,17 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd, static int btrfs_freeze(struct super_block *sb) { - struct btrfs_root *root = btrfs_sb(sb); - mutex_lock(&root->fs_info->transaction_kthread_mutex); - mutex_lock(&root->fs_info->cleaner_mutex); + struct btrfs_fs_info *fs_info = btrfs_sb(sb); + mutex_lock(&fs_info->transaction_kthread_mutex); + mutex_lock(&fs_info->cleaner_mutex); return 0; } static int btrfs_unfreeze(struct super_block *sb) { - struct btrfs_root *root = btrfs_sb(sb); - mutex_unlock(&root->fs_info->cleaner_mutex); - mutex_unlock(&root->fs_info->transaction_kthread_mutex); + struct btrfs_fs_info *fs_info = btrfs_sb(sb); + mutex_unlock(&fs_info->cleaner_mutex); + mutex_unlock(&fs_info->transaction_kthread_mutex); return 0; } -- 1.7.2.5 From 011c04b1b5fb234952e45ab38068bedfb4d9c474 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 17 Nov 2011 15:57:57 -0500 Subject: [PATCH 21/21] btrfs: take allocation of ->tree_root into open_ctree() now that we don't need it for sget() anymore... Signed-off-by: Al Viro --- fs/btrfs/disk-io.c | 8 +++++--- fs/btrfs/disk-io.h | 2 -- fs/btrfs/super.c | 5 ----- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index a78c762..3540e9b 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1215,7 +1215,7 @@ static int find_and_setup_root(struct btrfs_root *tree_root, return 0; } -struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info) +static struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info) { struct btrfs_root *root = kzalloc(sizeof(*root), GFP_NOFS); if (root) @@ -1899,7 +1899,7 @@ int open_ctree(struct super_block *sb, struct buffer_head *bh; struct btrfs_super_block *disk_super; struct btrfs_fs_info *fs_info = btrfs_sb(sb); - struct btrfs_root *tree_root = fs_info->tree_root; + struct btrfs_root *tree_root; struct btrfs_root *extent_root; struct btrfs_root *csum_root; struct btrfs_root *chunk_root; @@ -1910,12 +1910,14 @@ int open_ctree(struct super_block *sb, int num_backups_tried = 0; int backup_index = 0; + tree_root = fs_info->tree_root = btrfs_alloc_root(fs_info); extent_root = fs_info->extent_root = btrfs_alloc_root(fs_info); csum_root = fs_info->csum_root = btrfs_alloc_root(fs_info); chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info); dev_root = fs_info->dev_root = btrfs_alloc_root(fs_info); - if (!extent_root || !csum_root || !chunk_root || !dev_root) { + if (!tree_root || !extent_root || !csum_root || + !chunk_root || !dev_root) { err = -ENOMEM; goto fail; } diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h index 6f5f487..e4bc474 100644 --- a/fs/btrfs/disk-io.h +++ b/fs/btrfs/disk-io.h @@ -86,8 +86,6 @@ int btrfs_init_log_root_tree(struct btrfs_trans_handle *trans, int btrfs_add_log_tree(struct btrfs_trans_handle *trans, struct btrfs_root *root); -struct btrfs_root *btrfs_alloc_root(struct btrfs_fs_info *fs_info); - #ifdef CONFIG_DEBUG_LOCK_ALLOC void btrfs_init_lockdep(void); void btrfs_set_buffer_lockdep_class(u64 objectid, diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 1f7b3e2..e0cf6e9 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -899,11 +899,6 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags, if (!fs_info) return ERR_PTR(-ENOMEM); - fs_info->tree_root = btrfs_alloc_root(fs_info); - if (!fs_info->tree_root) { - error = -ENOMEM; - goto error_fs_info; - } fs_info->fs_devices = fs_devices; fs_info->super_copy = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_NOFS); -- 1.7.2.5