btrfs: kill the subvol_srcu

Now that we have proper root ref counting everywhere we can kill the
subvol_srcu.

* removal of fs_info::subvol_srcu reduces size of fs_info by 1176 bytes

* the refcount_t used for the references checks for accidental 0->1
  in cases where the root lifetime would not be properly protected

* there's a leak detector for roots to catch unfreed roots at umount
  time

* SRCU served us well over the years but is was not a proper
  synchronization mechanism for some cases

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
[ update changelog ]
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index f2728fb3ee8f4b3979c4de56f71e0aaeff509394..cd2d39b60be06ac17835c53f061229bca8118273 100644 (file)
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -542,24 +542,19 @@ static int resolve_indirect_ref(struct btrfs_fs_info *fs_info,
        int ret = 0;
        int root_level;
        int level = ref->level;
-       int index;
        struct btrfs_key search_key = ref->key_for_search;
 
        root_key.objectid = ref->root_id;
        root_key.type = BTRFS_ROOT_ITEM_KEY;
        root_key.offset = (u64)-1;
 
-       index = srcu_read_lock(&fs_info->subvol_srcu);
-
        root = btrfs_get_fs_root(fs_info, &root_key, false);
        if (IS_ERR(root)) {
-               srcu_read_unlock(&fs_info->subvol_srcu, index);
                ret = PTR_ERR(root);
                goto out_free;
        }
 
        if (btrfs_is_testing(fs_info)) {
-               srcu_read_unlock(&fs_info->subvol_srcu, index);
                ret = -ENOENT;
                goto out;
        }
@@ -571,10 +566,8 @@ static int resolve_indirect_ref(struct btrfs_fs_info *fs_info,
        else
                root_level = btrfs_old_root_level(root, time_seq);
 
-       if (root_level + 1 == level) {
-               srcu_read_unlock(&fs_info->subvol_srcu, index);
+       if (root_level + 1 == level)
                goto out;
-       }
 
        /*
         * We can often find data backrefs with an offset that is too large
@@ -604,9 +597,6 @@ static int resolve_indirect_ref(struct btrfs_fs_info *fs_info,
        else
                ret = btrfs_search_old_slot(root, &search_key, path, time_seq);
 
-       /* root node has been locked, we can release @subvol_srcu safely here */
-       srcu_read_unlock(&fs_info->subvol_srcu, index);
-
        btrfs_debug(fs_info,
                "search slot in root %llu (level %d, ref count %d) returned %d for key (%llu %u %llu)",
                 ref->root_id, level, ref->count, ret,

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index f1eef81aaa49632334475b2285bea65dc2e5b25b..8aa7b9dac40557339514b7a32e90b04218c83e30 100644 (file)
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -697,7 +697,6 @@ struct btrfs_fs_info {
        struct rw_semaphore cleanup_work_sem;
 
        struct rw_semaphore subvol_sem;
-       struct srcu_struct subvol_srcu;
 
        spinlock_t trans_lock;
        /*

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4d3eba909664fc3f4390d2a548eb8a262cfb204f..a6cb5cbbdb9f56a849cacceba0b96b3ee825cb97 100644 (file)
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2757,46 +2757,33 @@ static int init_mount_fs_info(struct btrfs_fs_info *fs_info, struct super_block
        sb->s_blocksize = BTRFS_BDEV_BLOCKSIZE;
        sb->s_blocksize_bits = blksize_bits(BTRFS_BDEV_BLOCKSIZE);
 
-       ret = init_srcu_struct(&fs_info->subvol_srcu);
-       if (ret)
-               return ret;
-
        ret = percpu_counter_init(&fs_info->dio_bytes, 0, GFP_KERNEL);
        if (ret)
-               goto fail;
+               return ret;
 
        ret = percpu_counter_init(&fs_info->dirty_metadata_bytes, 0, GFP_KERNEL);
        if (ret)
-               goto fail;
+               return ret;
 
        fs_info->dirty_metadata_batch = PAGE_SIZE *
                                        (1 + ilog2(nr_cpu_ids));
 
        ret = percpu_counter_init(&fs_info->delalloc_bytes, 0, GFP_KERNEL);
        if (ret)
-               goto fail;
+               return ret;
 
        ret = percpu_counter_init(&fs_info->dev_replace.bio_counter, 0,
                        GFP_KERNEL);
        if (ret)
-               goto fail;
+               return ret;
 
        fs_info->delayed_root = kmalloc(sizeof(struct btrfs_delayed_root),
                                        GFP_KERNEL);
-       if (!fs_info->delayed_root) {
-               ret = -ENOMEM;
-               goto fail;
-       }
+       if (!fs_info->delayed_root)
+               return -ENOMEM;
        btrfs_init_delayed_root(fs_info->delayed_root);
 
-       ret = btrfs_alloc_stripe_hash_table(fs_info);
-       if (ret)
-               goto fail;
-
-       return 0;
-fail:
-       cleanup_srcu_struct(&fs_info->subvol_srcu);
-       return ret;
+       return btrfs_alloc_stripe_hash_table(fs_info);
 }
 
 static int btrfs_uuid_rescan_kthread(void *data)
@@ -2870,13 +2857,13 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
        fs_info->chunk_root = chunk_root;
        if (!tree_root || !chunk_root) {
                err = -ENOMEM;
-               goto fail_srcu;
+               goto fail;
        }
 
        fs_info->btree_inode = new_inode(sb);
        if (!fs_info->btree_inode) {
                err = -ENOMEM;
-               goto fail_srcu;
+               goto fail;
        }
        mapping_set_gfp_mask(fs_info->btree_inode->i_mapping, GFP_NOFS);
        btrfs_init_btree_inode(fs_info);
@@ -3398,8 +3385,6 @@ fail_alloc:
        btrfs_mapping_tree_free(&fs_info->mapping_tree);
 
        iput(fs_info->btree_inode);
-fail_srcu:
-       cleanup_srcu_struct(&fs_info->subvol_srcu);
 fail:
        btrfs_close_devices(fs_info->fs_devices);
        return err;
@@ -3902,9 +3887,6 @@ void btrfs_drop_and_free_fs_root(struct btrfs_fs_info *fs_info,
                drop_ref = true;
        spin_unlock(&fs_info->fs_roots_radix_lock);
 
-       if (btrfs_root_refs(&root->root_item) == 0)
-               synchronize_srcu(&fs_info->subvol_srcu);
-
        if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state)) {
                btrfs_free_log(NULL, root);
                if (root->reloc_root) {
@@ -4116,7 +4098,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
 
        btrfs_mapping_tree_free(&fs_info->mapping_tree);
        btrfs_close_devices(fs_info->fs_devices);
-       cleanup_srcu_struct(&fs_info->subvol_srcu);
 }
 
 int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,

diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
index 25bd4317bf5a74bddc483f6f5aca633e72994964..2bb25d2dc44bdfb26dae60e7ac4d27b7001b0297 100644 (file)
--- a/fs/btrfs/export.c
+++ b/fs/btrfs/export.c
@@ -65,8 +65,6 @@ struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
        struct btrfs_root *root;
        struct inode *inode;
        struct btrfs_key key;
-       int index;
-       int err = 0;
 
        if (objectid < BTRFS_FIRST_FREE_OBJECTID)
                return ERR_PTR(-ESTALE);
@@ -75,13 +73,9 @@ struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
        key.type = BTRFS_ROOT_ITEM_KEY;
        key.offset = (u64)-1;
 
-       index = srcu_read_lock(&fs_info->subvol_srcu);
-
        root = btrfs_get_fs_root(fs_info, &key, true);
-       if (IS_ERR(root)) {
-               err = PTR_ERR(root);
-               goto fail;
-       }
+       if (IS_ERR(root))
+               return ERR_CAST(root);
 
        key.objectid = objectid;
        key.type = BTRFS_INODE_ITEM_KEY;
@@ -89,12 +83,8 @@ struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
 
        inode = btrfs_iget(sb, &key, root);
        btrfs_put_root(root);
-       if (IS_ERR(inode)) {
-               err = PTR_ERR(inode);
-               goto fail;
-       }
-
-       srcu_read_unlock(&fs_info->subvol_srcu, index);
+       if (IS_ERR(inode))
+               return ERR_CAST(inode);
 
        if (check_generation && generation != inode->i_generation) {
                iput(inode);
@@ -102,9 +92,6 @@ struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
        }
 
        return d_obtain_alias(inode);
-fail:
-       srcu_read_unlock(&fs_info->subvol_srcu, index);
-       return ERR_PTR(err);
 }
 
 static struct dentry *btrfs_fh_to_parent(struct super_block *sb, struct fid *fh,

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 18c88f514a0d999e8c5a15beaeb705f76f9dcd1c..20107f42a7669195eb6f053348258a507a5ea685 100644 (file)
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -278,7 +278,6 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
        struct btrfs_key key;
        struct btrfs_ioctl_defrag_range_args range;
        int num_defrag;
-       int index;
        int ret;
 
        /* get the inode */
@@ -286,8 +285,6 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
        key.type = BTRFS_ROOT_ITEM_KEY;
        key.offset = (u64)-1;
 
-       index = srcu_read_lock(&fs_info->subvol_srcu);
-
        inode_root = btrfs_get_fs_root(fs_info, &key, true);
        if (IS_ERR(inode_root)) {
                ret = PTR_ERR(inode_root);
@@ -303,7 +300,6 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
                ret = PTR_ERR(inode);
                goto cleanup;
        }
-       srcu_read_unlock(&fs_info->subvol_srcu, index);
 
        /* do a chunk of defrag */
        clear_bit(BTRFS_INODE_IN_DEFRAG, &BTRFS_I(inode)->runtime_flags);
@@ -339,7 +335,6 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
        iput(inode);
        return 0;
 cleanup:
-       srcu_read_unlock(&fs_info->subvol_srcu, index);
        kmem_cache_free(btrfs_inode_defrag_cachep, defrag);
        return ret;
 }

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 683bb20c1d0f1b8f4b87f71c875b3016d5d3db11..320d1062068d36efa08869fd6f557255cf38b4cc 100644 (file)
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5364,7 +5364,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
        struct btrfs_root *sub_root = root;
        struct btrfs_key location;
        u8 di_type = 0;
-       int index;
        int ret = 0;
 
        if (dentry->d_name.len > BTRFS_NAME_LEN)
@@ -5391,7 +5390,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
                return inode;
        }
 
-       index = srcu_read_lock(&fs_info->subvol_srcu);
        ret = fixup_tree_root_location(fs_info, dir, dentry,
                                       &location, &sub_root);
        if (ret < 0) {
@@ -5404,7 +5402,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
        }
        if (root != sub_root)
                btrfs_put_root(sub_root);
-       srcu_read_unlock(&fs_info->subvol_srcu, index);
 
        if (!IS_ERR(inode) && root != sub_root) {
                down_read(&fs_info->cleanup_work_sem);

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index e47f768cec3d8f7d520d6b3e004d9bdbf07a8b10..c5f41bd86765f4452408e2e6b634dd02a2de3dd1 100644 (file)
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -7028,7 +7028,6 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
        int clone_sources_to_rollback = 0;
        unsigned alloc_size;
        int sort_clone_roots = 0;
-       int index;
 
        if (!capable(CAP_SYS_ADMIN))
                return -EPERM;
@@ -7155,11 +7154,8 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
                        key.type = BTRFS_ROOT_ITEM_KEY;
                        key.offset = (u64)-1;
 
-                       index = srcu_read_lock(&fs_info->subvol_srcu);
-
                        clone_root = btrfs_get_fs_root(fs_info, &key, true);
                        if (IS_ERR(clone_root)) {
-                               srcu_read_unlock(&fs_info->subvol_srcu, index);
                                ret = PTR_ERR(clone_root);
                                goto out;
                        }
@@ -7168,7 +7164,6 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
                            btrfs_root_dead(clone_root)) {
                                spin_unlock(&clone_root->root_item_lock);
                                btrfs_put_root(clone_root);
-                               srcu_read_unlock(&fs_info->subvol_srcu, index);
                                ret = -EPERM;
                                goto out;
                        }
@@ -7176,13 +7171,11 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
                                dedupe_in_progress_warn(clone_root);
                                spin_unlock(&clone_root->root_item_lock);
                                btrfs_put_root(clone_root);
-                               srcu_read_unlock(&fs_info->subvol_srcu, index);
                                ret = -EAGAIN;
                                goto out;
                        }
                        clone_root->send_in_progress++;
                        spin_unlock(&clone_root->root_item_lock);
-                       srcu_read_unlock(&fs_info->subvol_srcu, index);
 
                        sctx->clone_roots[i].root = clone_root;
                        clone_sources_to_rollback = i + 1;
@@ -7196,11 +7189,8 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
                key.type = BTRFS_ROOT_ITEM_KEY;
                key.offset = (u64)-1;
 
-               index = srcu_read_lock(&fs_info->subvol_srcu);
-
                sctx->parent_root = btrfs_get_fs_root(fs_info, &key, true);
                if (IS_ERR(sctx->parent_root)) {
-                       srcu_read_unlock(&fs_info->subvol_srcu, index);
                        ret = PTR_ERR(sctx->parent_root);
                        goto out;
                }
@@ -7210,20 +7200,16 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
                if (!btrfs_root_readonly(sctx->parent_root) ||
                                btrfs_root_dead(sctx->parent_root)) {
                        spin_unlock(&sctx->parent_root->root_item_lock);
-                       srcu_read_unlock(&fs_info->subvol_srcu, index);
                        ret = -EPERM;
                        goto out;
                }
                if (sctx->parent_root->dedupe_in_progress) {
                        dedupe_in_progress_warn(sctx->parent_root);
                        spin_unlock(&sctx->parent_root->root_item_lock);
-                       srcu_read_unlock(&fs_info->subvol_srcu, index);
                        ret = -EAGAIN;
                        goto out;
                }
                spin_unlock(&sctx->parent_root->root_item_lock);
-
-               srcu_read_unlock(&fs_info->subvol_srcu, index);
        }
 
        /*

diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index 42e62fd2809c14e9fdf156cb29f9823fb0ef82bd..999c14e5d0bdd939c672076e963df4c87a1614f8 100644 (file)
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -134,14 +134,6 @@ struct btrfs_fs_info *btrfs_alloc_dummy_fs_info(u32 nodesize, u32 sectorsize)
 
        fs_info->nodesize = nodesize;
        fs_info->sectorsize = sectorsize;
-
-       if (init_srcu_struct(&fs_info->subvol_srcu)) {
-               kfree(fs_info->fs_devices);
-               kfree(fs_info->super_copy);
-               kfree(fs_info);
-               return NULL;
-       }
-
        set_bit(BTRFS_FS_STATE_DUMMY_FS_INFO, &fs_info->fs_state);
 
        test_mnt->mnt_sb->s_fs_info = fs_info;
@@ -191,7 +183,6 @@ void btrfs_free_dummy_fs_info(struct btrfs_fs_info *fs_info)
        }
        btrfs_free_qgroup_config(fs_info);
        btrfs_free_fs_roots(fs_info);
-       cleanup_srcu_struct(&fs_info->subvol_srcu);
        kfree(fs_info->super_copy);
        btrfs_check_leaked_roots(fs_info);
        btrfs_extent_buffer_leak_debug_check(fs_info);