Skip to content

Commit c75e839

Browse files
josefbacikkdave
authored andcommitted
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 <[email protected]> Reviewed-by: David Sterba <[email protected]> [ update changelog ] Signed-off-by: David Sterba <[email protected]>
1 parent efc3453 commit c75e839

File tree

8 files changed

+14
-88
lines changed

8 files changed

+14
-88
lines changed

fs/btrfs/backref.c

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -542,24 +542,19 @@ static int resolve_indirect_ref(struct btrfs_fs_info *fs_info,
542542
int ret = 0;
543543
int root_level;
544544
int level = ref->level;
545-
int index;
546545
struct btrfs_key search_key = ref->key_for_search;
547546

548547
root_key.objectid = ref->root_id;
549548
root_key.type = BTRFS_ROOT_ITEM_KEY;
550549
root_key.offset = (u64)-1;
551550

552-
index = srcu_read_lock(&fs_info->subvol_srcu);
553-
554551
root = btrfs_get_fs_root(fs_info, &root_key, false);
555552
if (IS_ERR(root)) {
556-
srcu_read_unlock(&fs_info->subvol_srcu, index);
557553
ret = PTR_ERR(root);
558554
goto out_free;
559555
}
560556

561557
if (btrfs_is_testing(fs_info)) {
562-
srcu_read_unlock(&fs_info->subvol_srcu, index);
563558
ret = -ENOENT;
564559
goto out;
565560
}
@@ -571,10 +566,8 @@ static int resolve_indirect_ref(struct btrfs_fs_info *fs_info,
571566
else
572567
root_level = btrfs_old_root_level(root, time_seq);
573568

574-
if (root_level + 1 == level) {
575-
srcu_read_unlock(&fs_info->subvol_srcu, index);
569+
if (root_level + 1 == level)
576570
goto out;
577-
}
578571

579572
/*
580573
* 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,
604597
else
605598
ret = btrfs_search_old_slot(root, &search_key, path, time_seq);
606599

607-
/* root node has been locked, we can release @subvol_srcu safely here */
608-
srcu_read_unlock(&fs_info->subvol_srcu, index);
609-
610600
btrfs_debug(fs_info,
611601
"search slot in root %llu (level %d, ref count %d) returned %d for key (%llu %u %llu)",
612602
ref->root_id, level, ref->count, ret,

fs/btrfs/ctree.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,6 @@ struct btrfs_fs_info {
697697
struct rw_semaphore cleanup_work_sem;
698698

699699
struct rw_semaphore subvol_sem;
700-
struct srcu_struct subvol_srcu;
701700

702701
spinlock_t trans_lock;
703702
/*

fs/btrfs/disk-io.c

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2757,46 +2757,33 @@ static int init_mount_fs_info(struct btrfs_fs_info *fs_info, struct super_block
27572757
sb->s_blocksize = BTRFS_BDEV_BLOCKSIZE;
27582758
sb->s_blocksize_bits = blksize_bits(BTRFS_BDEV_BLOCKSIZE);
27592759

2760-
ret = init_srcu_struct(&fs_info->subvol_srcu);
2761-
if (ret)
2762-
return ret;
2763-
27642760
ret = percpu_counter_init(&fs_info->dio_bytes, 0, GFP_KERNEL);
27652761
if (ret)
2766-
goto fail;
2762+
return ret;
27672763

27682764
ret = percpu_counter_init(&fs_info->dirty_metadata_bytes, 0, GFP_KERNEL);
27692765
if (ret)
2770-
goto fail;
2766+
return ret;
27712767

27722768
fs_info->dirty_metadata_batch = PAGE_SIZE *
27732769
(1 + ilog2(nr_cpu_ids));
27742770

27752771
ret = percpu_counter_init(&fs_info->delalloc_bytes, 0, GFP_KERNEL);
27762772
if (ret)
2777-
goto fail;
2773+
return ret;
27782774

27792775
ret = percpu_counter_init(&fs_info->dev_replace.bio_counter, 0,
27802776
GFP_KERNEL);
27812777
if (ret)
2782-
goto fail;
2778+
return ret;
27832779

27842780
fs_info->delayed_root = kmalloc(sizeof(struct btrfs_delayed_root),
27852781
GFP_KERNEL);
2786-
if (!fs_info->delayed_root) {
2787-
ret = -ENOMEM;
2788-
goto fail;
2789-
}
2782+
if (!fs_info->delayed_root)
2783+
return -ENOMEM;
27902784
btrfs_init_delayed_root(fs_info->delayed_root);
27912785

2792-
ret = btrfs_alloc_stripe_hash_table(fs_info);
2793-
if (ret)
2794-
goto fail;
2795-
2796-
return 0;
2797-
fail:
2798-
cleanup_srcu_struct(&fs_info->subvol_srcu);
2799-
return ret;
2786+
return btrfs_alloc_stripe_hash_table(fs_info);
28002787
}
28012788

28022789
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
28702857
fs_info->chunk_root = chunk_root;
28712858
if (!tree_root || !chunk_root) {
28722859
err = -ENOMEM;
2873-
goto fail_srcu;
2860+
goto fail;
28742861
}
28752862

28762863
fs_info->btree_inode = new_inode(sb);
28772864
if (!fs_info->btree_inode) {
28782865
err = -ENOMEM;
2879-
goto fail_srcu;
2866+
goto fail;
28802867
}
28812868
mapping_set_gfp_mask(fs_info->btree_inode->i_mapping, GFP_NOFS);
28822869
btrfs_init_btree_inode(fs_info);
@@ -3398,8 +3385,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
33983385
btrfs_mapping_tree_free(&fs_info->mapping_tree);
33993386

34003387
iput(fs_info->btree_inode);
3401-
fail_srcu:
3402-
cleanup_srcu_struct(&fs_info->subvol_srcu);
34033388
fail:
34043389
btrfs_close_devices(fs_info->fs_devices);
34053390
return err;
@@ -3902,9 +3887,6 @@ void btrfs_drop_and_free_fs_root(struct btrfs_fs_info *fs_info,
39023887
drop_ref = true;
39033888
spin_unlock(&fs_info->fs_roots_radix_lock);
39043889

3905-
if (btrfs_root_refs(&root->root_item) == 0)
3906-
synchronize_srcu(&fs_info->subvol_srcu);
3907-
39083890
if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state)) {
39093891
btrfs_free_log(NULL, root);
39103892
if (root->reloc_root) {
@@ -4116,7 +4098,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
41164098

41174099
btrfs_mapping_tree_free(&fs_info->mapping_tree);
41184100
btrfs_close_devices(fs_info->fs_devices);
4119-
cleanup_srcu_struct(&fs_info->subvol_srcu);
41204101
}
41214102

41224103
int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,

fs/btrfs/export.c

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
6565
struct btrfs_root *root;
6666
struct inode *inode;
6767
struct btrfs_key key;
68-
int index;
69-
int err = 0;
7068

7169
if (objectid < BTRFS_FIRST_FREE_OBJECTID)
7270
return ERR_PTR(-ESTALE);
@@ -75,36 +73,25 @@ struct dentry *btrfs_get_dentry(struct super_block *sb, u64 objectid,
7573
key.type = BTRFS_ROOT_ITEM_KEY;
7674
key.offset = (u64)-1;
7775

78-
index = srcu_read_lock(&fs_info->subvol_srcu);
79-
8076
root = btrfs_get_fs_root(fs_info, &key, true);
81-
if (IS_ERR(root)) {
82-
err = PTR_ERR(root);
83-
goto fail;
84-
}
77+
if (IS_ERR(root))
78+
return ERR_CAST(root);
8579

8680
key.objectid = objectid;
8781
key.type = BTRFS_INODE_ITEM_KEY;
8882
key.offset = 0;
8983

9084
inode = btrfs_iget(sb, &key, root);
9185
btrfs_put_root(root);
92-
if (IS_ERR(inode)) {
93-
err = PTR_ERR(inode);
94-
goto fail;
95-
}
96-
97-
srcu_read_unlock(&fs_info->subvol_srcu, index);
86+
if (IS_ERR(inode))
87+
return ERR_CAST(inode);
9888

9989
if (check_generation && generation != inode->i_generation) {
10090
iput(inode);
10191
return ERR_PTR(-ESTALE);
10292
}
10393

10494
return d_obtain_alias(inode);
105-
fail:
106-
srcu_read_unlock(&fs_info->subvol_srcu, index);
107-
return ERR_PTR(err);
10895
}
10996

11097
static struct dentry *btrfs_fh_to_parent(struct super_block *sb, struct fid *fh,

fs/btrfs/file.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -278,16 +278,13 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
278278
struct btrfs_key key;
279279
struct btrfs_ioctl_defrag_range_args range;
280280
int num_defrag;
281-
int index;
282281
int ret;
283282

284283
/* get the inode */
285284
key.objectid = defrag->root;
286285
key.type = BTRFS_ROOT_ITEM_KEY;
287286
key.offset = (u64)-1;
288287

289-
index = srcu_read_lock(&fs_info->subvol_srcu);
290-
291288
inode_root = btrfs_get_fs_root(fs_info, &key, true);
292289
if (IS_ERR(inode_root)) {
293290
ret = PTR_ERR(inode_root);
@@ -303,7 +300,6 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
303300
ret = PTR_ERR(inode);
304301
goto cleanup;
305302
}
306-
srcu_read_unlock(&fs_info->subvol_srcu, index);
307303

308304
/* do a chunk of defrag */
309305
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,
339335
iput(inode);
340336
return 0;
341337
cleanup:
342-
srcu_read_unlock(&fs_info->subvol_srcu, index);
343338
kmem_cache_free(btrfs_inode_defrag_cachep, defrag);
344339
return ret;
345340
}

fs/btrfs/inode.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5364,7 +5364,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
53645364
struct btrfs_root *sub_root = root;
53655365
struct btrfs_key location;
53665366
u8 di_type = 0;
5367-
int index;
53685367
int ret = 0;
53695368

53705369
if (dentry->d_name.len > BTRFS_NAME_LEN)
@@ -5391,7 +5390,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
53915390
return inode;
53925391
}
53935392

5394-
index = srcu_read_lock(&fs_info->subvol_srcu);
53955393
ret = fixup_tree_root_location(fs_info, dir, dentry,
53965394
&location, &sub_root);
53975395
if (ret < 0) {
@@ -5404,7 +5402,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
54045402
}
54055403
if (root != sub_root)
54065404
btrfs_put_root(sub_root);
5407-
srcu_read_unlock(&fs_info->subvol_srcu, index);
54085405

54095406
if (!IS_ERR(inode) && root != sub_root) {
54105407
down_read(&fs_info->cleanup_work_sem);

fs/btrfs/send.c

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7028,7 +7028,6 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
70287028
int clone_sources_to_rollback = 0;
70297029
unsigned alloc_size;
70307030
int sort_clone_roots = 0;
7031-
int index;
70327031

70337032
if (!capable(CAP_SYS_ADMIN))
70347033
return -EPERM;
@@ -7155,11 +7154,8 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
71557154
key.type = BTRFS_ROOT_ITEM_KEY;
71567155
key.offset = (u64)-1;
71577156

7158-
index = srcu_read_lock(&fs_info->subvol_srcu);
7159-
71607157
clone_root = btrfs_get_fs_root(fs_info, &key, true);
71617158
if (IS_ERR(clone_root)) {
7162-
srcu_read_unlock(&fs_info->subvol_srcu, index);
71637159
ret = PTR_ERR(clone_root);
71647160
goto out;
71657161
}
@@ -7168,21 +7164,18 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
71687164
btrfs_root_dead(clone_root)) {
71697165
spin_unlock(&clone_root->root_item_lock);
71707166
btrfs_put_root(clone_root);
7171-
srcu_read_unlock(&fs_info->subvol_srcu, index);
71727167
ret = -EPERM;
71737168
goto out;
71747169
}
71757170
if (clone_root->dedupe_in_progress) {
71767171
dedupe_in_progress_warn(clone_root);
71777172
spin_unlock(&clone_root->root_item_lock);
71787173
btrfs_put_root(clone_root);
7179-
srcu_read_unlock(&fs_info->subvol_srcu, index);
71807174
ret = -EAGAIN;
71817175
goto out;
71827176
}
71837177
clone_root->send_in_progress++;
71847178
spin_unlock(&clone_root->root_item_lock);
7185-
srcu_read_unlock(&fs_info->subvol_srcu, index);
71867179

71877180
sctx->clone_roots[i].root = clone_root;
71887181
clone_sources_to_rollback = i + 1;
@@ -7196,11 +7189,8 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
71967189
key.type = BTRFS_ROOT_ITEM_KEY;
71977190
key.offset = (u64)-1;
71987191

7199-
index = srcu_read_lock(&fs_info->subvol_srcu);
7200-
72017192
sctx->parent_root = btrfs_get_fs_root(fs_info, &key, true);
72027193
if (IS_ERR(sctx->parent_root)) {
7203-
srcu_read_unlock(&fs_info->subvol_srcu, index);
72047194
ret = PTR_ERR(sctx->parent_root);
72057195
goto out;
72067196
}
@@ -7210,20 +7200,16 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
72107200
if (!btrfs_root_readonly(sctx->parent_root) ||
72117201
btrfs_root_dead(sctx->parent_root)) {
72127202
spin_unlock(&sctx->parent_root->root_item_lock);
7213-
srcu_read_unlock(&fs_info->subvol_srcu, index);
72147203
ret = -EPERM;
72157204
goto out;
72167205
}
72177206
if (sctx->parent_root->dedupe_in_progress) {
72187207
dedupe_in_progress_warn(sctx->parent_root);
72197208
spin_unlock(&sctx->parent_root->root_item_lock);
7220-
srcu_read_unlock(&fs_info->subvol_srcu, index);
72217209
ret = -EAGAIN;
72227210
goto out;
72237211
}
72247212
spin_unlock(&sctx->parent_root->root_item_lock);
7225-
7226-
srcu_read_unlock(&fs_info->subvol_srcu, index);
72277213
}
72287214

72297215
/*

fs/btrfs/tests/btrfs-tests.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,6 @@ struct btrfs_fs_info *btrfs_alloc_dummy_fs_info(u32 nodesize, u32 sectorsize)
134134

135135
fs_info->nodesize = nodesize;
136136
fs_info->sectorsize = sectorsize;
137-
138-
if (init_srcu_struct(&fs_info->subvol_srcu)) {
139-
kfree(fs_info->fs_devices);
140-
kfree(fs_info->super_copy);
141-
kfree(fs_info);
142-
return NULL;
143-
}
144-
145137
set_bit(BTRFS_FS_STATE_DUMMY_FS_INFO, &fs_info->fs_state);
146138

147139
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)
191183
}
192184
btrfs_free_qgroup_config(fs_info);
193185
btrfs_free_fs_roots(fs_info);
194-
cleanup_srcu_struct(&fs_info->subvol_srcu);
195186
kfree(fs_info->super_copy);
196187
btrfs_check_leaked_roots(fs_info);
197188
btrfs_extent_buffer_leak_debug_check(fs_info);

0 commit comments

Comments
 (0)