btrfs: prepare and move async csum generation out of experimental feautres
cover btrfs: prepare and move async csum generation out of experimental feautres
Sashiko exposed several problems related to previous default async csum generation, one fix related to encoded writes is already merged.
The remaining problem is related to the endio_workers reuse, which can lead to problems as there are two different workloads in one workqueue, and one workload (btrfs_bio_csum()) can even queue a new work into the same workqueue.
The remaining problem is fixed by the first patch, by changing btrfs_fs_info::workers into a regular workqueue, and let the experimental csum generation to reuse it.
Then we can finally enable async data csum generation by default to improve the data write performance, and completely get rid of the old csum offloading behavior.
Qu Wenruo (2):
btrfs: make btrfs_fs_info::workers to be a regular workqueue
btrfs: move async data csum generation out of experimental features
fs/btrfs/Kconfig | 2 - fs/btrfs/bio.c | 139 +------------------------------------------ fs/btrfs/disk-io.c | 47 ++++++--------- fs/btrfs/file-item.c | 12 ++-- fs/btrfs/file-item.h | 2 +- fs/btrfs/fs.h | 17 +----- fs/btrfs/super.c | 2 +- 7 files changed, 30 insertions(+), 191 deletions(-)
Signature · 1 line
2.56.0
PATCH 1/2 btrfs: make btrfs_fs_info::workers to be a regular workqueue
The workers workqueue is only utilized by csum generation offloading path, which has quite some requirements to meet:
- Non-experimental builds
Experimental builds are already submitting bios before csum generation is finished, which has more concurrency than this method, and still ensures the submission sequence is followed.
- Slow csum algorithms
- Non-sync IOs
Otherwise offloading into a workqueue is only to cause more context switch and overhead.
- Not metadata zoned writes
The biggest one that requires sequential submission is already excluded.
So there is not really much need to use btrfs workqueue for the sequential submission.
Change from a btrfs workqueue to a regular workqueue.
This will make some bios submission out of their original order, but the order will be back after the async csum generation feature is out of experimental, which is very soon.
The main objective of this change is to prepare for the incomine async csum generation as default.
As the previous endio_workers sharing can still cause problems related to the workqueue flush.
--- fs/btrfs/bio.c | 53 ++++++++++++++------------------------------ fs/btrfs/disk-io.c | 14 +++++++----- fs/btrfs/file-item.c | 2 +- fs/btrfs/fs.h | 2 +- fs/btrfs/super.c | 2 +- 5 files changed, 29 insertions(+), 44 deletions(-)
fs/btrfs/bio.c +17 -36
@@ -10,7 +10,6 @@#include "ctree.h" #include "volumes.h" #include "raid56.h" -#include "async-thread.h" #include "dev-replace.h" #include "zoned.h" #include "file-item.h"
@@ -579,7 +578,7 @@ struct async_submit_bio {struct btrfs_io_context *bioc;struct btrfs_io_stripe smap;int mirror_num; -struct btrfs_work work; +struct work_struct work; };/*
@@ -588,45 +587,27 @@ struct async_submit_bio {* sums are attached onto the ordered extent record. * * At IO completion time the csums attached on the ordered extent record are -* inserted into the btree. +* inserted into the tree. */ -static void run_one_async_start (struct btrfs_work *work) +static void run_one_async_submit (struct work_struct *work) {struct async_submit_bio *async = container_of(work, struct async_submit_bio, work); +struct btrfs_bio *bbio = async->bbio; +struct btrfs_io_context *bioc = async->bioc; +struct btrfs_io_stripe smap = async->smap; +const int mirror_num = async->mirror_num; +struct bio *bio = &bbio->bio;int ret; - ret =btrfs_bio_csum (async->bbio); -if (ret) - async->bbio->bio.bi_status =errno_to_blk_status (ret); -} - -/* -* In order to insert checksums into the metadata in large chunks, we wait -* until bio submission time. All the pages in the bio are checksummed and -* sums are attached onto the ordered extent record. -* -* At IO completion time the csums attached on the ordered extent record are -* inserted into the tree. -* -* If called with @do_free == true, then it will free the work struct. -*/ -static void run_one_async_done (struct btrfs_work *work,bool do_free) -{ -struct async_submit_bio *async = - container_of(work, struct async_submit_bio, work); -struct bio *bio = &async->bbio->bio; - -if (do_free) { -kfree (container_of(work ,struct async_submit_bio ,work )); -return ; - } +kfree (async);/* If an error occurred we just want to clean up the bio and move on. */ -if (bio->bi_status) { -btrfs_put_bioc (async->bioc); -btrfs_bio_counter_dec (async->bbio->inode->root->fs_info); -btrfs_bio_end_io (async->bbio, bio->bi_status); + ret =btrfs_bio_csum (bbio); +if (ret) { +btrfs_put_bioc (bioc); +btrfs_bio_counter_dec (bbio->inode->root->fs_info); +btrfs_bio_end_io (bbio,errno_to_blk_status (ret));return ; }
@@ -636,7 +617,7 @@ static void run_one_async_done(struct btrfs_work *work, bool do_free)* context. This changes nothing when cgroups aren't in use. */ bio->bi_opf |=REQ_BTRFS_CGROUP_PUNT ; -btrfs_submit_bio (bio, async->bioc, &async->smap, async->mirror_num); +btrfs_submit_bio (bio, bioc, &smap, mirror_num); }static bool should_async_write (struct btrfs_bio *bbio)
@@ -692,8 +673,8 @@ static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio, async->smap = *smap; async->mirror_num = mirror_num; -btrfs_init_work (&async->work, run_one_async_start, run_one_async_done); -btrfs_queue_work (fs_info->workers, &async->work); +INIT_WORK (&async->work, run_one_async_submit); +queue_work (fs_info->workers, &async->work);return true; }
fs/btrfs/disk-io.c +9 -5
@@ -1829,7 +1829,8 @@ static void btrfs_stop_all_workers(struct btrfs_fs_info *fs_info)if (fs_info->fixup_workers)destroy_workqueue (fs_info->fixup_workers);btrfs_destroy_workqueue (fs_info->delalloc_workers); -btrfs_destroy_workqueue (fs_info->workers); +if (fs_info->workers) +destroy_workqueue (fs_info->workers);if (fs_info->endio_workers)destroy_workqueue (fs_info->endio_workers);if (fs_info->rmw_workers)
@@ -2040,8 +2041,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)unsigned int flags =WQ_MEM_RECLAIM |WQ_FREEZABLE |WQ_UNBOUND ;unsigned int ordered_flags =WQ_MEM_RECLAIM |WQ_FREEZABLE ; - fs_info->workers = -btrfs_alloc_workqueue (fs_info,"worker" , flags, max_active,16 ); + fs_info->workers =alloc_workqueue ("btrfs-worker" , flags, max_active); fs_info->delalloc_workers = btrfs_alloc_workqueue(fs_info, "delalloc",
@@ -4469,14 +4469,18 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)* We can have ordered extents getting their last reference dropped from * the fs_info->workers queue because for async writes for data bios we * queue a work for that queue, at btrfs_wq_submit_bio(), that runs -* run_one_async_done() which calls btrfs_bio_end_io() in case the bio +* run_one_async_submit() which calls btrfs_bio_end_io() in case the bio * has an error, and that later function can do the final * btrfs_put_ordered_extent() on the ordered extent attached to the bio, * which adds a delayed iput for the inode. So we must flush the queue * so that we don't have delayed iputs after committing the current * transaction below and stopping the cleaner and transaction kthreads. +* +* That queue also runs the async checksum work, csum_one_bio_work(), +* whose bio_endio() can lead to work being queued on the endio queues +* flushed below, so it must be flushed before them. */ -btrfs_flush_workqueue (fs_info->workers); +flush_workqueue (fs_info->workers);/* * When finishing a compressed write bio we schedule a work queue item
fs/btrfs/file-item.c +1 -1
@@ -845,7 +845,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async) }bio_inc_remaining (bio);INIT_WORK (&bbio->csum_work, csum_one_bio_work); -queue_work (fs_info->endio_workers, &bbio->csum_work); +queue_work (fs_info->workers, &bbio->csum_work);return 0 ; }
fs/btrfs/fs.h +1 -1
@@ -704,7 +704,7 @@ struct btrfs_fs_info {* * A third pool does submit_bio to avoid deadlocking with the other two. */ -struct btrfs_workqueue *workers; +struct workqueue_struct *workers;struct btrfs_workqueue *delalloc_workers;struct btrfs_workqueue *flush_workers;struct workqueue_struct *endio_workers;
fs/btrfs/super.c +1 -1
@@ -1235,7 +1235,7 @@ static void btrfs_resize_thread_pool(struct btrfs_fs_info *fs_info,btrfs_info (fs_info,"resize thread pool %d -> %d" , old_pool_size, new_pool_size); -btrfs_workqueue_set_max (fs_info->workers, new_pool_size); +workqueue_set_max_active (fs_info->workers, new_pool_size);btrfs_workqueue_set_max (fs_info->delalloc_workers, new_pool_size);btrfs_workqueue_set_max (fs_info->caching_workers, new_pool_size);workqueue_set_max_active (fs_info->endio_workers, new_pool_size);
Signature · 1 line
2.56.0
PATCH 2/2 btrfs: move async data csum generation out of experimental features
In reply to Qu WenruoThe feature has been under experimental features for a long time, it's time to enable it by default for end users.
This also means we can completely get rid of the current csum offload for slow checksum algorithms, as there is no difference between slow and fast csum generation anyway.
There is a minor behavior change for metadata writes, previously if the checksum algorithm is slow, we will offload the metadata checksum generation into a workqueue, and wait for the csum generation to finish before submission.
That brings no concurrency improvement, and will only introduce extra context switches as overhead. Now metadata checksum generation is always inlined.
--- fs/btrfs/Kconfig | 2 - fs/btrfs/bio.c | 120 +------------------------------------------ fs/btrfs/disk-io.c | 41 +++++---------- fs/btrfs/file-item.c | 10 ++-- fs/btrfs/file-item.h | 2 +- fs/btrfs/fs.h | 15 +----- 6 files changed, 22 insertions(+), 168 deletions(-)
fs/btrfs/Kconfig +0 -2
@@ -111,8 +111,6 @@ config BTRFS_EXPERIMENTAL - huge folios for data - folios can be as large as 2MiB now - - asynchronous checksum generation for data writes - - remap-tree - logical address remapping tree If unsure, say N.
fs/btrfs/bio.c +1 -119
138 lines not on this page · raw message
fs/btrfs/disk-io.c +13 -28
70 lines not on this page · raw message
fs/btrfs/file-item.c +5 -5
@@ -814,8 +814,12 @@ static void csum_one_bio_work(struct work_struct *work)/* * Calculate checksums of the data contained inside a bio. +* +* The checksums are calculated by a work queued on fs_info->workers, so they +* are not valid on return. This must be called before the bio is submitted, as +* the extra bio_inc_remaining() makes the bio completion wait for the work. */ -int btrfs_csum_one_bio (struct btrfs_bio *bbio,bool async) +int btrfs_csum_one_bio (struct btrfs_bio *bbio) {struct btrfs_ordered_extent *ordered = bbio->ordered;struct btrfs_inode *inode = bbio->inode;
@@ -839,10 +843,6 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)btrfs_add_ordered_sum (ordered, sums); bbio->csum_saved_iter = bio->bi_iter; -if (!async) { -csum_one_bio (bbio); -return 0 ; - }bio_inc_remaining (bio);INIT_WORK (&bbio->csum_work, csum_one_bio_work);queue_work (fs_info->workers, &bbio->csum_work);
fs/btrfs/file-item.h +1 -1
@@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,int btrfs_insert_data_csums (struct btrfs_trans_handle *trans,struct btrfs_root *root,struct btrfs_ordered_sum *sums); -int btrfs_csum_one_bio (struct btrfs_bio *bbio,bool async); +int btrfs_csum_one_bio (struct btrfs_bio *bbio);int btrfs_alloc_dummy_sum (struct btrfs_bio *bbio); int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end, struct list_head *list, int search_commit,
fs/btrfs/fs.h +2 -13
@@ -189,12 +189,6 @@ enum {/* Indicate that the cleaner thread is awake and doing something. */ BTRFS_FS_CLEANER_RUNNING , -/* -* The checksumming has an optimized version and is considered fast, -* so we don't need to offload checksums to workqueues. -*/ -BTRFS_FS_CSUM_IMPL_FAST , -/* Indicate that the discard workqueue can service discards. */ BTRFS_FS_DISCARD_RUNNING,
@@ -696,13 +690,8 @@ struct btrfs_fs_info {struct list_head delalloc_roots;/* -* There is a pool of worker threads for checksumming during writes and -* a pool for checksumming after reads. This is because readers can -* run with FS locks held, and the writers may be waiting for those -* locks. We don't want ordering in the pending list to cause -* deadlocks, and so the two are serviced separately. -* -* A third pool does submit_bio to avoid deadlocking with the other two. +* @workers is the pool of worker threads for checksumming during +* data writes. */ struct workqueue_struct *workers;struct btrfs_workqueue *delalloc_workers;
Signature · 1 line
2.56.0