Skip to content

Commit f26c923

Browse files
adam900710kdave
authored andcommitted
btrfs: remove reada infrastructure
Currently there is only one user for btrfs metadata readahead, and that's scrub. But even for the single user, it's not providing the correct functionality it needs, as scrub needs reada for commit root, which current readahead can't provide. (Although it's pretty easy to add such feature). Despite this, there are some extra problems related to metadata readahead: - Duplicated feature with btrfs_path::reada - Partly duplicated feature of btrfs_fs_info::buffer_radix Btrfs already caches its metadata in buffer_radix, while readahead tries to read the tree block no matter if it's already cached. - Poor layer separation Metadata readahead works kinda at device level. This is definitely not the correct layer it should be, since metadata is at btrfs logical address space, it should not bother device at all. This brings extra chance for bugs to sneak in, while brings unnecessary complexity. - Dead code In the very beginning of scrub.c we have #undef DEBUG, rendering all the debug related code useless and unable to test. Thus here I purpose to remove the metadata readahead mechanism completely. [BENCHMARK] There is a full benchmark for the scrub performance difference using the old btrfs_reada_add() and btrfs_path::reada. For the worst case (no dirty metadata, slow HDD), there could be a 5% performance drop for scrub. For other cases (even SATA SSD), there is no distinguishable performance difference. The number is reported scrub speed, in MiB/s. The resolution is limited by the reported duration, which only has a resolution of 1 second. Old New Diff SSD 455.3 466.332 +2.42% HDD 103.927 98.012 -5.69% Comprehensive test methodology is in the cover letter of the patch. Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent dcf62b2 commit f26c923

File tree

10 files changed

+3
-1189
lines changed

10 files changed

+3
-1189
lines changed

fs/btrfs/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o root-tree.o dir-item.o \
2727
extent_io.o volumes.o async-thread.o ioctl.o locking.o orphan.o \
2828
export.o tree-log.o free-space-cache.o zlib.o lzo.o zstd.o \
2929
compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
30-
reada.o backref.o ulist.o qgroup.o send.o dev-replace.o raid56.o \
30+
backref.o ulist.o qgroup.o send.o dev-replace.o raid56.o \
3131
uuid-tree.o props.o free-space-tree.o tree-checker.o space-info.o \
3232
block-rsv.o delalloc-space.o block-group.o discard.o reflink.o \
3333
subpage.o tree-mod-log.o

fs/btrfs/ctree.h

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,6 @@ struct btrfs_fs_info {
821821
struct btrfs_workqueue *endio_write_workers;
822822
struct btrfs_workqueue *endio_freespace_worker;
823823
struct btrfs_workqueue *caching_workers;
824-
struct btrfs_workqueue *readahead_workers;
825824

826825
/*
827826
* fixup workers take dirty pages that didn't properly go through
@@ -958,13 +957,6 @@ struct btrfs_fs_info {
958957

959958
struct btrfs_delayed_root *delayed_root;
960959

961-
/* readahead tree */
962-
spinlock_t reada_lock;
963-
struct radix_tree_root reada_tree;
964-
965-
/* readahead works cnt */
966-
atomic_t reada_works_cnt;
967-
968960
/* Extent buffer radix tree */
969961
spinlock_t buffer_lock;
970962
/* Entries are eb->start / sectorsize */
@@ -3807,23 +3799,6 @@ static inline void btrfs_bio_counter_dec(struct btrfs_fs_info *fs_info)
38073799
btrfs_bio_counter_sub(fs_info, 1);
38083800
}
38093801

3810-
/* reada.c */
3811-
struct reada_control {
3812-
struct btrfs_fs_info *fs_info; /* tree to prefetch */
3813-
struct btrfs_key key_start;
3814-
struct btrfs_key key_end; /* exclusive */
3815-
atomic_t elems;
3816-
struct kref refcnt;
3817-
wait_queue_head_t wait;
3818-
};
3819-
struct reada_control *btrfs_reada_add(struct btrfs_root *root,
3820-
struct btrfs_key *start, struct btrfs_key *end);
3821-
int btrfs_reada_wait(void *handle);
3822-
void btrfs_reada_detach(void *handle);
3823-
int btree_readahead_hook(struct extent_buffer *eb, int err);
3824-
void btrfs_reada_remove_dev(struct btrfs_device *dev);
3825-
void btrfs_reada_undo_remove_dev(struct btrfs_device *dev);
3826-
38273802
static inline int is_fstree(u64 rootid)
38283803
{
38293804
if (rootid == BTRFS_FS_TREE_OBJECTID ||

fs/btrfs/dev-replace.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -906,9 +906,6 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
906906
}
907907
btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
908908

909-
if (!scrub_ret)
910-
btrfs_reada_remove_dev(src_device);
911-
912909
/*
913910
* We have to use this loop approach because at this point src_device
914911
* has to be available for transaction commit to complete, yet new
@@ -917,7 +914,6 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
917914
while (1) {
918915
trans = btrfs_start_transaction(root, 0);
919916
if (IS_ERR(trans)) {
920-
btrfs_reada_undo_remove_dev(src_device);
921917
mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
922918
return PTR_ERR(trans);
923919
}
@@ -968,7 +964,6 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
968964
up_write(&dev_replace->rwsem);
969965
mutex_unlock(&fs_info->chunk_mutex);
970966
mutex_unlock(&fs_info->fs_devices->device_list_mutex);
971-
btrfs_reada_undo_remove_dev(src_device);
972967
btrfs_rm_dev_replace_blocked(fs_info);
973968
if (tgt_device)
974969
btrfs_destroy_dev_replace_tgtdev(tgt_device);

fs/btrfs/disk-io.c

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -665,9 +665,6 @@ static int validate_subpage_buffer(struct page *page, u64 start, u64 end,
665665
if (ret < 0)
666666
goto err;
667667

668-
if (test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
669-
btree_readahead_hook(eb, ret);
670-
671668
set_extent_buffer_uptodate(eb);
672669

673670
free_extent_buffer(eb);
@@ -715,10 +712,6 @@ int btrfs_validate_metadata_buffer(struct btrfs_bio *bbio,
715712
}
716713
ret = validate_extent_buffer(eb);
717714
err:
718-
if (reads_done &&
719-
test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
720-
btree_readahead_hook(eb, ret);
721-
722715
if (ret) {
723716
/*
724717
* our io error hook is going to dec the io pages
@@ -2232,7 +2225,6 @@ static void btrfs_stop_all_workers(struct btrfs_fs_info *fs_info)
22322225
btrfs_destroy_workqueue(fs_info->endio_freespace_worker);
22332226
btrfs_destroy_workqueue(fs_info->delayed_workers);
22342227
btrfs_destroy_workqueue(fs_info->caching_workers);
2235-
btrfs_destroy_workqueue(fs_info->readahead_workers);
22362228
btrfs_destroy_workqueue(fs_info->flush_workers);
22372229
btrfs_destroy_workqueue(fs_info->qgroup_rescan_workers);
22382230
if (fs_info->discard_ctl.discard_workers)
@@ -2445,9 +2437,6 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
24452437
fs_info->delayed_workers =
24462438
btrfs_alloc_workqueue(fs_info, "delayed-meta", flags,
24472439
max_active, 0);
2448-
fs_info->readahead_workers =
2449-
btrfs_alloc_workqueue(fs_info, "readahead", flags,
2450-
max_active, 2);
24512440
fs_info->qgroup_rescan_workers =
24522441
btrfs_alloc_workqueue(fs_info, "qgroup-rescan", flags, 1, 0);
24532442
fs_info->discard_ctl.discard_workers =
@@ -2459,9 +2448,8 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
24592448
fs_info->endio_meta_write_workers &&
24602449
fs_info->endio_write_workers && fs_info->endio_raid56_workers &&
24612450
fs_info->endio_freespace_worker && fs_info->rmw_workers &&
2462-
fs_info->caching_workers && fs_info->readahead_workers &&
2463-
fs_info->fixup_workers && fs_info->delayed_workers &&
2464-
fs_info->qgroup_rescan_workers &&
2451+
fs_info->caching_workers && fs_info->fixup_workers &&
2452+
fs_info->delayed_workers && fs_info->qgroup_rescan_workers &&
24652453
fs_info->discard_ctl.discard_workers)) {
24662454
return -ENOMEM;
24672455
}
@@ -3091,7 +3079,6 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
30913079

30923080
atomic_set(&fs_info->async_delalloc_pages, 0);
30933081
atomic_set(&fs_info->defrag_running, 0);
3094-
atomic_set(&fs_info->reada_works_cnt, 0);
30953082
atomic_set(&fs_info->nr_delayed_iputs, 0);
30963083
atomic64_set(&fs_info->tree_mod_seq, 0);
30973084
fs_info->global_root_tree = RB_ROOT;
@@ -3102,9 +3089,6 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
31023089
fs_info->tree_mod_log = RB_ROOT;
31033090
fs_info->commit_interval = BTRFS_DEFAULT_COMMIT_INTERVAL;
31043091
fs_info->avg_delayed_ref_runtime = NSEC_PER_SEC >> 6; /* div by 64 */
3105-
/* readahead state */
3106-
INIT_RADIX_TREE(&fs_info->reada_tree, GFP_NOFS & ~__GFP_DIRECT_RECLAIM);
3107-
spin_lock_init(&fs_info->reada_lock);
31083092
btrfs_init_ref_verify(fs_info);
31093093

31103094
fs_info->thread_pool_size = min_t(unsigned long,

fs/btrfs/extent_io.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3087,9 +3087,6 @@ static void end_bio_extent_readpage(struct bio *bio)
30873087
set_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags);
30883088
eb->read_mirror = mirror;
30893089
atomic_dec(&eb->io_pages);
3090-
if (test_and_clear_bit(EXTENT_BUFFER_READAHEAD,
3091-
&eb->bflags))
3092-
btree_readahead_hook(eb, -EIO);
30933090
}
30943091
readpage_ok:
30953092
if (likely(uptodate)) {

0 commit comments

Comments
 (0)