[PATCH] btrfs: move async csum generation out of experimental features

[PATCH] btrfs: move async csum generation out of experimental features

[Qu Wenruo]

Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
introduced asynchronous data checksum generation for data writes.

That feature can improve write performance, and was introduced in v6.19.
We have not experienced bugs related to that, so it's time to move it out
of experimental features for end users.

Furthermore since the async checksum generation means
should_async_write() will always return false, we no longer need to
maintain the fs_info->workers workqueue, as the async checksum
generation is using the system_percpu_wq.

Suggested-by: Daniel Vacek <neelx@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/bio.c       | 136 +------------------------------------------
 fs/btrfs/disk-io.c   |  20 +------
 fs/btrfs/file-item.c |   6 +-
 fs/btrfs/file-item.h |   2 +-
 fs/btrfs/fs.h        |  10 ----
 fs/btrfs/super.c     |   1 -
 6 files changed, 4 insertions(+), 171 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 771b7d598aee..e08006543c3d 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
 {
 	if (bbio->bio.bi_opf & REQ_META)
 		return btree_csum_one_bio(bbio);
-#ifdef CONFIG_BTRFS_EXPERIMENTAL
-	return btrfs_csum_one_bio(bbio, true);
-#else
-	return btrfs_csum_one_bio(bbio, false);
-#endif
-}
-
-/*
- * Async submit bios are used to offload expensive checksumming onto the worker
- * threads.
- */
-struct async_submit_bio {
-	struct btrfs_bio *bbio;
-	struct btrfs_io_context *bioc;
-	struct btrfs_io_stripe smap;
-	int mirror_num;
-	struct btrfs_work work;
-};
-
-/*
- * 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 btree.
- */
-static void run_one_async_start(struct btrfs_work *work)
-{
-	struct async_submit_bio *async =
-		container_of(work, struct async_submit_bio, work);
-	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;
-	}
-
-	/* If an error occurred we just want to clean up the bio and move on. */
-	if (bio->bi_status) {
-		btrfs_bio_end_io(async->bbio, bio->bi_status);
-		return;
-	}
-
-	/*
-	 * All of the bios that pass through here are from async helpers.
-	 * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
-	 * 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);
-}
-
-static bool should_async_write(struct btrfs_bio *bbio)
-{
-	struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
-	bool auto_csum_mode = true;
-
-#ifdef CONFIG_BTRFS_EXPERIMENTAL
-	/*
-	 * Write bios will calculate checksum and submit bio at the same time.
-	 * Unless explicitly required don't offload serial csum calculate and bio
-	 * submit into a workqueue.
-	 */
-	return false;
-#endif
-
-	/* Submit synchronously if the checksum implementation is fast. */
-	if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
-		return false;
-
-	/*
-	 * Try to defer the submission to a workqueue to parallelize the
-	 * checksum calculation unless the I/O is issued synchronously.
-	 */
-	if (op_is_sync(bbio->bio.bi_opf))
-		return false;
-
-	/* Zoned devices require I/O to be submitted in order. */
-	if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
-		return false;
-
-	return true;
-}
-
-/*
- * Submit bio to an async queue.
- *
- * Return true if the work has been successfully submitted, else false.
- */
-static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
-				struct btrfs_io_context *bioc,
-				struct btrfs_io_stripe *smap, int mirror_num)
-{
-	struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
-	struct async_submit_bio *async;
-
-	async = kmalloc_obj(*async, GFP_NOFS);
-	if (!async)
-		return false;
-
-	async->bbio = bbio;
-	async->bioc = bioc;
-	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);
-	return true;
+	return btrfs_csum_one_bio(bbio);
 }
 
 static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
@@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 		if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
 		    !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
 		    !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
-			if (should_async_write(bbio) &&
-			    btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
-				goto done;
-
 			ret = btrfs_bio_csum(bbio);
 			status = errno_to_blk_status(ret);
 			if (status)
@@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 	}
 
 	btrfs_submit_bio(bio, bioc, &smap, mirror_num);
-done:
 	return map_length == length;
 
 fail:
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a1d83ad9a4c0..0046f3cb75a1 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1776,7 +1776,6 @@ 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->endio_workers)
 		destroy_workqueue(fs_info->endio_workers);
 	if (fs_info->rmw_workers)
@@ -1968,9 +1967,6 @@ 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->delalloc_workers =
 		btrfs_alloc_workqueue(fs_info, "delalloc",
 				      flags, max_active, 2);
@@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
 	fs_info->discard_ctl.discard_workers =
 		alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
 
-	if (!(fs_info->workers &&
-	      fs_info->delalloc_workers && fs_info->flush_workers &&
+	if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
 	      fs_info->endio_workers && fs_info->endio_meta_workers &&
 	      fs_info->endio_write_workers &&
 	      fs_info->endio_freespace_worker && fs_info->rmw_workers &&
@@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
 	 */
 	btrfs_flush_workqueue(fs_info->delalloc_workers);
 
-	/*
-	 * 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
-	 * 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.
-	 */
-	btrfs_flush_workqueue(fs_info->workers);
-
 	/*
 	 * When finishing a compressed write bio we schedule a work queue item
 	 * to finish an ordered extent - end_bbio_compressed_write()
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 0fed4e0d32d5..7dbd9b8eec11 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
 /*
  * Calculate checksums of the data contained inside a bio.
  */
-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;
@@ -849,10 +849,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);
 	schedule_work(&bbio->csum_work);
diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
index 6c678787c770..60a0eb17b3f6 100644
--- a/fs/btrfs/file-item.h
+++ b/fs/btrfs/file-item.h
@@ -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,
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 3eba8438593c..203f7131f737 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -696,16 +696,6 @@ struct btrfs_fs_info {
 	/* All fs/file tree roots that have delalloc inodes. */
 	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.
-	 */
-	struct btrfs_workqueue *workers;
 	struct btrfs_workqueue *delalloc_workers;
 	struct btrfs_workqueue *flush_workers;
 	struct workqueue_struct *endio_workers;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 464129b1b0d4..54f2da47c567 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1237,7 +1237,6 @@ 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);
 	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);
-- 
2.55.0

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Daniel Vacek]

On Thu, 3 Sept 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
> introduced asynchronous data checksum generation for data writes.
>
> That feature can improve write performance, and was introduced in v6.19.
> We have not experienced bugs related to that, so it's time to move it out
> of experimental features for end users.
>
> Furthermore since the async checksum generation means
> should_async_write() will always return false, we no longer need to
> maintain the fs_info->workers workqueue, as the async checksum
> generation is using the system_percpu_wq.

I understand these are two related changes in one patch here. If
removing async writes depends on async checksums, it would be nice to
split this into a patch series.

Though I guess can live with that. Looks good enough to me.

Reviewed-by: Daniel Vacek <neelx@suse.com>

> Suggested-by: Daniel Vacek <neelx@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/bio.c       | 136 +------------------------------------------
>  fs/btrfs/disk-io.c   |  20 +------
>  fs/btrfs/file-item.c |   6 +-
>  fs/btrfs/file-item.h |   2 +-
>  fs/btrfs/fs.h        |  10 ----
>  fs/btrfs/super.c     |   1 -
>  6 files changed, 4 insertions(+), 171 deletions(-)
>
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 771b7d598aee..e08006543c3d 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>  {
>         if (bbio->bio.bi_opf & REQ_META)
>                 return btree_csum_one_bio(bbio);
> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> -       return btrfs_csum_one_bio(bbio, true);
> -#else
> -       return btrfs_csum_one_bio(bbio, false);
> -#endif
> -}
> -
> -/*
> - * Async submit bios are used to offload expensive checksumming onto the worker
> - * threads.
> - */
> -struct async_submit_bio {
> -       struct btrfs_bio *bbio;
> -       struct btrfs_io_context *bioc;
> -       struct btrfs_io_stripe smap;
> -       int mirror_num;
> -       struct btrfs_work work;
> -};
> -
> -/*
> - * 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 btree.
> - */
> -static void run_one_async_start(struct btrfs_work *work)
> -{
> -       struct async_submit_bio *async =
> -               container_of(work, struct async_submit_bio, work);
> -       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;
> -       }
> -
> -       /* If an error occurred we just want to clean up the bio and move on. */
> -       if (bio->bi_status) {
> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
> -               return;
> -       }
> -
> -       /*
> -        * All of the bios that pass through here are from async helpers.
> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
> -        * 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);
> -}
> -
> -static bool should_async_write(struct btrfs_bio *bbio)
> -{
> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> -       bool auto_csum_mode = true;
> -
> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> -       /*
> -        * Write bios will calculate checksum and submit bio at the same time.
> -        * Unless explicitly required don't offload serial csum calculate and bio
> -        * submit into a workqueue.
> -        */
> -       return false;
> -#endif
> -
> -       /* Submit synchronously if the checksum implementation is fast. */
> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
> -               return false;
> -
> -       /*
> -        * Try to defer the submission to a workqueue to parallelize the
> -        * checksum calculation unless the I/O is issued synchronously.
> -        */
> -       if (op_is_sync(bbio->bio.bi_opf))
> -               return false;
> -
> -       /* Zoned devices require I/O to be submitted in order. */
> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
> -               return false;
> -
> -       return true;
> -}
> -
> -/*
> - * Submit bio to an async queue.
> - *
> - * Return true if the work has been successfully submitted, else false.
> - */
> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
> -                               struct btrfs_io_context *bioc,
> -                               struct btrfs_io_stripe *smap, int mirror_num)
> -{
> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> -       struct async_submit_bio *async;
> -
> -       async = kmalloc_obj(*async, GFP_NOFS);
> -       if (!async)
> -               return false;
> -
> -       async->bbio = bbio;
> -       async->bioc = bioc;
> -       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);
> -       return true;
> +       return btrfs_csum_one_bio(bbio);
>  }
>
>  static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>                 if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
>                     !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
>                     !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
> -                       if (should_async_write(bbio) &&
> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
> -                               goto done;
> -
>                         ret = btrfs_bio_csum(bbio);
>                         status = errno_to_blk_status(ret);
>                         if (status)
> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>         }
>
>         btrfs_submit_bio(bio, bioc, &smap, mirror_num);
> -done:
>         return map_length == length;
>
>  fail:
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a1d83ad9a4c0..0046f3cb75a1 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1776,7 +1776,6 @@ 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->endio_workers)
>                 destroy_workqueue(fs_info->endio_workers);
>         if (fs_info->rmw_workers)
> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
>                 btrfs_alloc_workqueue(fs_info, "delalloc",
>                                       flags, max_active, 2);
> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
>         fs_info->discard_ctl.discard_workers =
>                 alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
>
> -       if (!(fs_info->workers &&
> -             fs_info->delalloc_workers && fs_info->flush_workers &&
> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
>               fs_info->endio_workers && fs_info->endio_meta_workers &&
>               fs_info->endio_write_workers &&
>               fs_info->endio_freespace_worker && fs_info->rmw_workers &&
> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>          */
>         btrfs_flush_workqueue(fs_info->delalloc_workers);
>
> -       /*
> -        * 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
> -        * 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.
> -        */
> -       btrfs_flush_workqueue(fs_info->workers);
> -
>         /*
>          * When finishing a compressed write bio we schedule a work queue item
>          * to finish an ordered extent - end_bbio_compressed_write()
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 0fed4e0d32d5..7dbd9b8eec11 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
>  /*
>   * Calculate checksums of the data contained inside a bio.
>   */
> -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;
> @@ -849,10 +849,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);
>         schedule_work(&bbio->csum_work);
> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> index 6c678787c770..60a0eb17b3f6 100644
> --- a/fs/btrfs/file-item.h
> +++ b/fs/btrfs/file-item.h
> @@ -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,
> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> index 3eba8438593c..203f7131f737 100644
> --- a/fs/btrfs/fs.h
> +++ b/fs/btrfs/fs.h
> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
>         /* All fs/file tree roots that have delalloc inodes. */
>         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.
> -        */
> -       struct btrfs_workqueue *workers;
>         struct btrfs_workqueue *delalloc_workers;
>         struct btrfs_workqueue *flush_workers;
>         struct workqueue_struct *endio_workers;
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 464129b1b0d4..54f2da47c567 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1237,7 +1237,6 @@ 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);
>         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);
> --
> 2.55.0
>

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Qu Wenruo]

在 2026/9/3 18:50, Daniel Vacek 写道:
> On Thu, 3 Sept 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
>> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
>> introduced asynchronous data checksum generation for data writes.
>>
>> That feature can improve write performance, and was introduced in v6.19.
>> We have not experienced bugs related to that, so it's time to move it out
>> of experimental features for end users.
>>
>> Furthermore since the async checksum generation means
>> should_async_write() will always return false, we no longer need to
>> maintain the fs_info->workers workqueue, as the async checksum
>> generation is using the system_percpu_wq.
> 
> I understand these are two related changes in one patch here. If
> removing async writes depends on async checksums, it would be nice to
> split this into a patch series.
> 
> Though I guess can live with that. Looks good enough to me.
> 
> Reviewed-by: Daniel Vacek <neelx@suse.com>

Thanks a lot for the review.


I just want to mention that, Sashiko mentioned a problem that I believe 
is valid, but not sure how realistic it will be in the real world.


Sashiko mentioned that, the old worker workqueue has WQ_MEM_RECLAIM 
flag, thus there will always be a rescuer to ensure forward progress.
(Although for x86_64 and other common archs or EXPERIMENTAL builds, we 
never utilize that worker anyway)

But the new schedule_work() is using system_percpu_wq, which doesn't has 
that flag.

And since the csum generation is needed for data writeback, if under 
very heavy memory pressure, there may be no worker to ensure the csum 
work can be queued, in that case, it will hang the writeback (which is 
triggered to reclaim memory).

If we really want to address that problem, your idea about splitting the 
patch will make a lot of sense.
We will need to keep the old infrastructure first, convert the worker to 
a regular workqueue, and call queue_work(), then fully remove unused code.


But on the other hand, I doubt how pratically it is for anyone to stall 
system_percpu_wq.
It's very common utilized across the whole kernel, and if it really 
stalls I think there are a lot of more things to bother before btrfs.

Another thing is, if we use fs_info::workers, we will follow the 
existing wq flags, which means no PERCPU flag.
And according to the existing docs for those flags, PERCPU wq has a 
better performance due to CPU locality.


So overall I think the current version is good enough, but not perfect.
On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will 
definitely introduce some extra pentalty.

If someone else has some idea on this Sashiko review, or very familiar 
with the WQ_MEM_RECLAIM situation, any comment will be appreciated.

Thanks,
Qu

> 
>> Suggested-by: Daniel Vacek <neelx@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/bio.c       | 136 +------------------------------------------
>>   fs/btrfs/disk-io.c   |  20 +------
>>   fs/btrfs/file-item.c |   6 +-
>>   fs/btrfs/file-item.h |   2 +-
>>   fs/btrfs/fs.h        |  10 ----
>>   fs/btrfs/super.c     |   1 -
>>   6 files changed, 4 insertions(+), 171 deletions(-)
>>
>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>> index 771b7d598aee..e08006543c3d 100644
>> --- a/fs/btrfs/bio.c
>> +++ b/fs/btrfs/bio.c
>> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>>   {
>>          if (bbio->bio.bi_opf & REQ_META)
>>                  return btree_csum_one_bio(bbio);
>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
>> -       return btrfs_csum_one_bio(bbio, true);
>> -#else
>> -       return btrfs_csum_one_bio(bbio, false);
>> -#endif
>> -}
>> -
>> -/*
>> - * Async submit bios are used to offload expensive checksumming onto the worker
>> - * threads.
>> - */
>> -struct async_submit_bio {
>> -       struct btrfs_bio *bbio;
>> -       struct btrfs_io_context *bioc;
>> -       struct btrfs_io_stripe smap;
>> -       int mirror_num;
>> -       struct btrfs_work work;
>> -};
>> -
>> -/*
>> - * 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 btree.
>> - */
>> -static void run_one_async_start(struct btrfs_work *work)
>> -{
>> -       struct async_submit_bio *async =
>> -               container_of(work, struct async_submit_bio, work);
>> -       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;
>> -       }
>> -
>> -       /* If an error occurred we just want to clean up the bio and move on. */
>> -       if (bio->bi_status) {
>> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
>> -               return;
>> -       }
>> -
>> -       /*
>> -        * All of the bios that pass through here are from async helpers.
>> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
>> -        * 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);
>> -}
>> -
>> -static bool should_async_write(struct btrfs_bio *bbio)
>> -{
>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
>> -       bool auto_csum_mode = true;
>> -
>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
>> -       /*
>> -        * Write bios will calculate checksum and submit bio at the same time.
>> -        * Unless explicitly required don't offload serial csum calculate and bio
>> -        * submit into a workqueue.
>> -        */
>> -       return false;
>> -#endif
>> -
>> -       /* Submit synchronously if the checksum implementation is fast. */
>> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
>> -               return false;
>> -
>> -       /*
>> -        * Try to defer the submission to a workqueue to parallelize the
>> -        * checksum calculation unless the I/O is issued synchronously.
>> -        */
>> -       if (op_is_sync(bbio->bio.bi_opf))
>> -               return false;
>> -
>> -       /* Zoned devices require I/O to be submitted in order. */
>> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
>> -               return false;
>> -
>> -       return true;
>> -}
>> -
>> -/*
>> - * Submit bio to an async queue.
>> - *
>> - * Return true if the work has been successfully submitted, else false.
>> - */
>> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
>> -                               struct btrfs_io_context *bioc,
>> -                               struct btrfs_io_stripe *smap, int mirror_num)
>> -{
>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
>> -       struct async_submit_bio *async;
>> -
>> -       async = kmalloc_obj(*async, GFP_NOFS);
>> -       if (!async)
>> -               return false;
>> -
>> -       async->bbio = bbio;
>> -       async->bioc = bioc;
>> -       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);
>> -       return true;
>> +       return btrfs_csum_one_bio(bbio);
>>   }
>>
>>   static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
>> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>                  if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
>>                      !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
>>                      !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
>> -                       if (should_async_write(bbio) &&
>> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
>> -                               goto done;
>> -
>>                          ret = btrfs_bio_csum(bbio);
>>                          status = errno_to_blk_status(ret);
>>                          if (status)
>> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>          }
>>
>>          btrfs_submit_bio(bio, bioc, &smap, mirror_num);
>> -done:
>>          return map_length == length;
>>
>>   fail:
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index a1d83ad9a4c0..0046f3cb75a1 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1776,7 +1776,6 @@ 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->endio_workers)
>>                  destroy_workqueue(fs_info->endio_workers);
>>          if (fs_info->rmw_workers)
>> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
>>                  btrfs_alloc_workqueue(fs_info, "delalloc",
>>                                        flags, max_active, 2);
>> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
>>          fs_info->discard_ctl.discard_workers =
>>                  alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
>>
>> -       if (!(fs_info->workers &&
>> -             fs_info->delalloc_workers && fs_info->flush_workers &&
>> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
>>                fs_info->endio_workers && fs_info->endio_meta_workers &&
>>                fs_info->endio_write_workers &&
>>                fs_info->endio_freespace_worker && fs_info->rmw_workers &&
>> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>>           */
>>          btrfs_flush_workqueue(fs_info->delalloc_workers);
>>
>> -       /*
>> -        * 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
>> -        * 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.
>> -        */
>> -       btrfs_flush_workqueue(fs_info->workers);
>> -
>>          /*
>>           * When finishing a compressed write bio we schedule a work queue item
>>           * to finish an ordered extent - end_bbio_compressed_write()
>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>> index 0fed4e0d32d5..7dbd9b8eec11 100644
>> --- a/fs/btrfs/file-item.c
>> +++ b/fs/btrfs/file-item.c
>> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
>>   /*
>>    * Calculate checksums of the data contained inside a bio.
>>    */
>> -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;
>> @@ -849,10 +849,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);
>>          schedule_work(&bbio->csum_work);
>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>> index 6c678787c770..60a0eb17b3f6 100644
>> --- a/fs/btrfs/file-item.h
>> +++ b/fs/btrfs/file-item.h
>> @@ -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,
>> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
>> index 3eba8438593c..203f7131f737 100644
>> --- a/fs/btrfs/fs.h
>> +++ b/fs/btrfs/fs.h
>> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
>>          /* All fs/file tree roots that have delalloc inodes. */
>>          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.
>> -        */
>> -       struct btrfs_workqueue *workers;
>>          struct btrfs_workqueue *delalloc_workers;
>>          struct btrfs_workqueue *flush_workers;
>>          struct workqueue_struct *endio_workers;
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 464129b1b0d4..54f2da47c567 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1237,7 +1237,6 @@ 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);
>>          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);
>> --
>> 2.55.0
>>
> 

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Daniel Vacek]

On Thu, 3 Sept 2026 at 13:27, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 在 2026/9/3 18:50, Daniel Vacek 写道:
> > On Thu, 3 Sept 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
> >> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
> >> introduced asynchronous data checksum generation for data writes.
> >>
> >> That feature can improve write performance, and was introduced in v6.19.
> >> We have not experienced bugs related to that, so it's time to move it out
> >> of experimental features for end users.
> >>
> >> Furthermore since the async checksum generation means
> >> should_async_write() will always return false, we no longer need to
> >> maintain the fs_info->workers workqueue, as the async checksum
> >> generation is using the system_percpu_wq.
> >
> > I understand these are two related changes in one patch here. If
> > removing async writes depends on async checksums, it would be nice to
> > split this into a patch series.
> >
> > Though I guess can live with that. Looks good enough to me.
> >
> > Reviewed-by: Daniel Vacek <neelx@suse.com>
>
> Thanks a lot for the review.
>
>
> I just want to mention that, Sashiko mentioned a problem that I believe
> is valid, but not sure how realistic it will be in the real world.
>
>
> Sashiko mentioned that, the old worker workqueue has WQ_MEM_RECLAIM
> flag, thus there will always be a rescuer to ensure forward progress.
> (Although for x86_64 and other common archs or EXPERIMENTAL builds, we
> never utilize that worker anyway)
>
> But the new schedule_work() is using system_percpu_wq, which doesn't has
> that flag.

Well, I think squashing this fixup to my patch is the right thing to do anyways:

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 0fed4e0d32d5..984d87ad79b0 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -855,7 +855,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);
-    schedule_work(&bbio->csum_work);
+    queue_work(fs_info->endio_workers, &bbio->csum_work);
     return 0;
 }


That should make sashiko happy as well as btrfs end_io work running on
btrfs end_io work queues.

And we should stay on the safe side wrt memory reclaim.

--nX


> And since the csum generation is needed for data writeback, if under
> very heavy memory pressure, there may be no worker to ensure the csum
> work can be queued, in that case, it will hang the writeback (which is
> triggered to reclaim memory).
>
> If we really want to address that problem, your idea about splitting the
> patch will make a lot of sense.
> We will need to keep the old infrastructure first, convert the worker to
> a regular workqueue, and call queue_work(), then fully remove unused code.
>
>
> But on the other hand, I doubt how pratically it is for anyone to stall
> system_percpu_wq.
> It's very common utilized across the whole kernel, and if it really
> stalls I think there are a lot of more things to bother before btrfs.
>
> Another thing is, if we use fs_info::workers, we will follow the
> existing wq flags, which means no PERCPU flag.
> And according to the existing docs for those flags, PERCPU wq has a
> better performance due to CPU locality.
>
>
> So overall I think the current version is good enough, but not perfect.
> On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will
> definitely introduce some extra pentalty.
>
> If someone else has some idea on this Sashiko review, or very familiar
> with the WQ_MEM_RECLAIM situation, any comment will be appreciated.
>
> Thanks,
> Qu
>
> >
> >> Suggested-by: Daniel Vacek <neelx@suse.com>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >>   fs/btrfs/bio.c       | 136 +------------------------------------------
> >>   fs/btrfs/disk-io.c   |  20 +------
> >>   fs/btrfs/file-item.c |   6 +-
> >>   fs/btrfs/file-item.h |   2 +-
> >>   fs/btrfs/fs.h        |  10 ----
> >>   fs/btrfs/super.c     |   1 -
> >>   6 files changed, 4 insertions(+), 171 deletions(-)
> >>
> >> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >> index 771b7d598aee..e08006543c3d 100644
> >> --- a/fs/btrfs/bio.c
> >> +++ b/fs/btrfs/bio.c
> >> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> >>   {
> >>          if (bbio->bio.bi_opf & REQ_META)
> >>                  return btree_csum_one_bio(bbio);
> >> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> >> -       return btrfs_csum_one_bio(bbio, true);
> >> -#else
> >> -       return btrfs_csum_one_bio(bbio, false);
> >> -#endif
> >> -}
> >> -
> >> -/*
> >> - * Async submit bios are used to offload expensive checksumming onto the worker
> >> - * threads.
> >> - */
> >> -struct async_submit_bio {
> >> -       struct btrfs_bio *bbio;
> >> -       struct btrfs_io_context *bioc;
> >> -       struct btrfs_io_stripe smap;
> >> -       int mirror_num;
> >> -       struct btrfs_work work;
> >> -};
> >> -
> >> -/*
> >> - * 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 btree.
> >> - */
> >> -static void run_one_async_start(struct btrfs_work *work)
> >> -{
> >> -       struct async_submit_bio *async =
> >> -               container_of(work, struct async_submit_bio, work);
> >> -       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;
> >> -       }
> >> -
> >> -       /* If an error occurred we just want to clean up the bio and move on. */
> >> -       if (bio->bi_status) {
> >> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
> >> -               return;
> >> -       }
> >> -
> >> -       /*
> >> -        * All of the bios that pass through here are from async helpers.
> >> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
> >> -        * 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);
> >> -}
> >> -
> >> -static bool should_async_write(struct btrfs_bio *bbio)
> >> -{
> >> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> >> -       bool auto_csum_mode = true;
> >> -
> >> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> >> -       /*
> >> -        * Write bios will calculate checksum and submit bio at the same time.
> >> -        * Unless explicitly required don't offload serial csum calculate and bio
> >> -        * submit into a workqueue.
> >> -        */
> >> -       return false;
> >> -#endif
> >> -
> >> -       /* Submit synchronously if the checksum implementation is fast. */
> >> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
> >> -               return false;
> >> -
> >> -       /*
> >> -        * Try to defer the submission to a workqueue to parallelize the
> >> -        * checksum calculation unless the I/O is issued synchronously.
> >> -        */
> >> -       if (op_is_sync(bbio->bio.bi_opf))
> >> -               return false;
> >> -
> >> -       /* Zoned devices require I/O to be submitted in order. */
> >> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
> >> -               return false;
> >> -
> >> -       return true;
> >> -}
> >> -
> >> -/*
> >> - * Submit bio to an async queue.
> >> - *
> >> - * Return true if the work has been successfully submitted, else false.
> >> - */
> >> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
> >> -                               struct btrfs_io_context *bioc,
> >> -                               struct btrfs_io_stripe *smap, int mirror_num)
> >> -{
> >> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> >> -       struct async_submit_bio *async;
> >> -
> >> -       async = kmalloc_obj(*async, GFP_NOFS);
> >> -       if (!async)
> >> -               return false;
> >> -
> >> -       async->bbio = bbio;
> >> -       async->bioc = bioc;
> >> -       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);
> >> -       return true;
> >> +       return btrfs_csum_one_bio(bbio);
> >>   }
> >>
> >>   static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
> >> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >>                  if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
> >>                      !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
> >>                      !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
> >> -                       if (should_async_write(bbio) &&
> >> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
> >> -                               goto done;
> >> -
> >>                          ret = btrfs_bio_csum(bbio);
> >>                          status = errno_to_blk_status(ret);
> >>                          if (status)
> >> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >>          }
> >>
> >>          btrfs_submit_bio(bio, bioc, &smap, mirror_num);
> >> -done:
> >>          return map_length == length;
> >>
> >>   fail:
> >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> >> index a1d83ad9a4c0..0046f3cb75a1 100644
> >> --- a/fs/btrfs/disk-io.c
> >> +++ b/fs/btrfs/disk-io.c
> >> @@ -1776,7 +1776,6 @@ 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->endio_workers)
> >>                  destroy_workqueue(fs_info->endio_workers);
> >>          if (fs_info->rmw_workers)
> >> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
> >>                  btrfs_alloc_workqueue(fs_info, "delalloc",
> >>                                        flags, max_active, 2);
> >> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
> >>          fs_info->discard_ctl.discard_workers =
> >>                  alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
> >>
> >> -       if (!(fs_info->workers &&
> >> -             fs_info->delalloc_workers && fs_info->flush_workers &&
> >> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
> >>                fs_info->endio_workers && fs_info->endio_meta_workers &&
> >>                fs_info->endio_write_workers &&
> >>                fs_info->endio_freespace_worker && fs_info->rmw_workers &&
> >> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> >>           */
> >>          btrfs_flush_workqueue(fs_info->delalloc_workers);
> >>
> >> -       /*
> >> -        * 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
> >> -        * 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.
> >> -        */
> >> -       btrfs_flush_workqueue(fs_info->workers);
> >> -
> >>          /*
> >>           * When finishing a compressed write bio we schedule a work queue item
> >>           * to finish an ordered extent - end_bbio_compressed_write()
> >> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >> index 0fed4e0d32d5..7dbd9b8eec11 100644
> >> --- a/fs/btrfs/file-item.c
> >> +++ b/fs/btrfs/file-item.c
> >> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
> >>   /*
> >>    * Calculate checksums of the data contained inside a bio.
> >>    */
> >> -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;
> >> @@ -849,10 +849,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);
> >>          schedule_work(&bbio->csum_work);
> >> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >> index 6c678787c770..60a0eb17b3f6 100644
> >> --- a/fs/btrfs/file-item.h
> >> +++ b/fs/btrfs/file-item.h
> >> @@ -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,
> >> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> >> index 3eba8438593c..203f7131f737 100644
> >> --- a/fs/btrfs/fs.h
> >> +++ b/fs/btrfs/fs.h
> >> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
> >>          /* All fs/file tree roots that have delalloc inodes. */
> >>          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.
> >> -        */
> >> -       struct btrfs_workqueue *workers;
> >>          struct btrfs_workqueue *delalloc_workers;
> >>          struct btrfs_workqueue *flush_workers;
> >>          struct workqueue_struct *endio_workers;
> >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> >> index 464129b1b0d4..54f2da47c567 100644
> >> --- a/fs/btrfs/super.c
> >> +++ b/fs/btrfs/super.c
> >> @@ -1237,7 +1237,6 @@ 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);
> >>          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);
> >> --
> >> 2.55.0
> >>
> >
>

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Daniel Vacek]

On Thu, 3 Sept 2026 at 14:44, Daniel Vacek <neelx@suse.com> wrote:
> On Thu, 3 Sept 2026 at 13:27, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> > 在 2026/9/3 18:50, Daniel Vacek 写道:
> > > On Thu, 3 Sept 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
> > >> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
> > >> introduced asynchronous data checksum generation for data writes.
> > >>
> > >> That feature can improve write performance, and was introduced in v6.19.
> > >> We have not experienced bugs related to that, so it's time to move it out
> > >> of experimental features for end users.
> > >>
> > >> Furthermore since the async checksum generation means
> > >> should_async_write() will always return false, we no longer need to
> > >> maintain the fs_info->workers workqueue, as the async checksum
> > >> generation is using the system_percpu_wq.
> > >
> > > I understand these are two related changes in one patch here. If
> > > removing async writes depends on async checksums, it would be nice to
> > > split this into a patch series.
> > >
> > > Though I guess can live with that. Looks good enough to me.
> > >
> > > Reviewed-by: Daniel Vacek <neelx@suse.com>
> >
> > Thanks a lot for the review.
> >
> >
> > I just want to mention that, Sashiko mentioned a problem that I believe
> > is valid, but not sure how realistic it will be in the real world.
> >
> >
> > Sashiko mentioned that, the old worker workqueue has WQ_MEM_RECLAIM
> > flag, thus there will always be a rescuer to ensure forward progress.
> > (Although for x86_64 and other common archs or EXPERIMENTAL builds, we
> > never utilize that worker anyway)
> >
> > But the new schedule_work() is using system_percpu_wq, which doesn't has
> > that flag.
>
> Well, I think squashing this fixup to my patch is the right thing to do anyways:
>
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 0fed4e0d32d5..984d87ad79b0 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -855,7 +855,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);
> -    schedule_work(&bbio->csum_work);
> +    queue_work(fs_info->endio_workers, &bbio->csum_work);
>      return 0;
>  }
>

And in theory this snippet also:

Fixes: dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")

--nX

>
> That should make sashiko happy as well as btrfs end_io work running on
> btrfs end_io work queues.
>
> And we should stay on the safe side wrt memory reclaim.
>
> --nX
>
>
> > And since the csum generation is needed for data writeback, if under
> > very heavy memory pressure, there may be no worker to ensure the csum
> > work can be queued, in that case, it will hang the writeback (which is
> > triggered to reclaim memory).
> >
> > If we really want to address that problem, your idea about splitting the
> > patch will make a lot of sense.
> > We will need to keep the old infrastructure first, convert the worker to
> > a regular workqueue, and call queue_work(), then fully remove unused code.
> >
> >
> > But on the other hand, I doubt how pratically it is for anyone to stall
> > system_percpu_wq.
> > It's very common utilized across the whole kernel, and if it really
> > stalls I think there are a lot of more things to bother before btrfs.
> >
> > Another thing is, if we use fs_info::workers, we will follow the
> > existing wq flags, which means no PERCPU flag.
> > And according to the existing docs for those flags, PERCPU wq has a
> > better performance due to CPU locality.
> >
> >
> > So overall I think the current version is good enough, but not perfect.
> > On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will
> > definitely introduce some extra pentalty.
> >
> > If someone else has some idea on this Sashiko review, or very familiar
> > with the WQ_MEM_RECLAIM situation, any comment will be appreciated.
> >
> > Thanks,
> > Qu
> >
> > >
> > >> Suggested-by: Daniel Vacek <neelx@suse.com>
> > >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> > >> ---
> > >>   fs/btrfs/bio.c       | 136 +------------------------------------------
> > >>   fs/btrfs/disk-io.c   |  20 +------
> > >>   fs/btrfs/file-item.c |   6 +-
> > >>   fs/btrfs/file-item.h |   2 +-
> > >>   fs/btrfs/fs.h        |  10 ----
> > >>   fs/btrfs/super.c     |   1 -
> > >>   6 files changed, 4 insertions(+), 171 deletions(-)
> > >>
> > >> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > >> index 771b7d598aee..e08006543c3d 100644
> > >> --- a/fs/btrfs/bio.c
> > >> +++ b/fs/btrfs/bio.c
> > >> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> > >>   {
> > >>          if (bbio->bio.bi_opf & REQ_META)
> > >>                  return btree_csum_one_bio(bbio);
> > >> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> > >> -       return btrfs_csum_one_bio(bbio, true);
> > >> -#else
> > >> -       return btrfs_csum_one_bio(bbio, false);
> > >> -#endif
> > >> -}
> > >> -
> > >> -/*
> > >> - * Async submit bios are used to offload expensive checksumming onto the worker
> > >> - * threads.
> > >> - */
> > >> -struct async_submit_bio {
> > >> -       struct btrfs_bio *bbio;
> > >> -       struct btrfs_io_context *bioc;
> > >> -       struct btrfs_io_stripe smap;
> > >> -       int mirror_num;
> > >> -       struct btrfs_work work;
> > >> -};
> > >> -
> > >> -/*
> > >> - * 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 btree.
> > >> - */
> > >> -static void run_one_async_start(struct btrfs_work *work)
> > >> -{
> > >> -       struct async_submit_bio *async =
> > >> -               container_of(work, struct async_submit_bio, work);
> > >> -       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;
> > >> -       }
> > >> -
> > >> -       /* If an error occurred we just want to clean up the bio and move on. */
> > >> -       if (bio->bi_status) {
> > >> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
> > >> -               return;
> > >> -       }
> > >> -
> > >> -       /*
> > >> -        * All of the bios that pass through here are from async helpers.
> > >> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
> > >> -        * 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);
> > >> -}
> > >> -
> > >> -static bool should_async_write(struct btrfs_bio *bbio)
> > >> -{
> > >> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> > >> -       bool auto_csum_mode = true;
> > >> -
> > >> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> > >> -       /*
> > >> -        * Write bios will calculate checksum and submit bio at the same time.
> > >> -        * Unless explicitly required don't offload serial csum calculate and bio
> > >> -        * submit into a workqueue.
> > >> -        */
> > >> -       return false;
> > >> -#endif
> > >> -
> > >> -       /* Submit synchronously if the checksum implementation is fast. */
> > >> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
> > >> -               return false;
> > >> -
> > >> -       /*
> > >> -        * Try to defer the submission to a workqueue to parallelize the
> > >> -        * checksum calculation unless the I/O is issued synchronously.
> > >> -        */
> > >> -       if (op_is_sync(bbio->bio.bi_opf))
> > >> -               return false;
> > >> -
> > >> -       /* Zoned devices require I/O to be submitted in order. */
> > >> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
> > >> -               return false;
> > >> -
> > >> -       return true;
> > >> -}
> > >> -
> > >> -/*
> > >> - * Submit bio to an async queue.
> > >> - *
> > >> - * Return true if the work has been successfully submitted, else false.
> > >> - */
> > >> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
> > >> -                               struct btrfs_io_context *bioc,
> > >> -                               struct btrfs_io_stripe *smap, int mirror_num)
> > >> -{
> > >> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> > >> -       struct async_submit_bio *async;
> > >> -
> > >> -       async = kmalloc_obj(*async, GFP_NOFS);
> > >> -       if (!async)
> > >> -               return false;
> > >> -
> > >> -       async->bbio = bbio;
> > >> -       async->bioc = bioc;
> > >> -       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);
> > >> -       return true;
> > >> +       return btrfs_csum_one_bio(bbio);
> > >>   }
> > >>
> > >>   static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
> > >> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> > >>                  if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
> > >>                      !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
> > >>                      !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
> > >> -                       if (should_async_write(bbio) &&
> > >> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
> > >> -                               goto done;
> > >> -
> > >>                          ret = btrfs_bio_csum(bbio);
> > >>                          status = errno_to_blk_status(ret);
> > >>                          if (status)
> > >> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> > >>          }
> > >>
> > >>          btrfs_submit_bio(bio, bioc, &smap, mirror_num);
> > >> -done:
> > >>          return map_length == length;
> > >>
> > >>   fail:
> > >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > >> index a1d83ad9a4c0..0046f3cb75a1 100644
> > >> --- a/fs/btrfs/disk-io.c
> > >> +++ b/fs/btrfs/disk-io.c
> > >> @@ -1776,7 +1776,6 @@ 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->endio_workers)
> > >>                  destroy_workqueue(fs_info->endio_workers);
> > >>          if (fs_info->rmw_workers)
> > >> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
> > >>                  btrfs_alloc_workqueue(fs_info, "delalloc",
> > >>                                        flags, max_active, 2);
> > >> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
> > >>          fs_info->discard_ctl.discard_workers =
> > >>                  alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
> > >>
> > >> -       if (!(fs_info->workers &&
> > >> -             fs_info->delalloc_workers && fs_info->flush_workers &&
> > >> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
> > >>                fs_info->endio_workers && fs_info->endio_meta_workers &&
> > >>                fs_info->endio_write_workers &&
> > >>                fs_info->endio_freespace_worker && fs_info->rmw_workers &&
> > >> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> > >>           */
> > >>          btrfs_flush_workqueue(fs_info->delalloc_workers);
> > >>
> > >> -       /*
> > >> -        * 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
> > >> -        * 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.
> > >> -        */
> > >> -       btrfs_flush_workqueue(fs_info->workers);
> > >> -
> > >>          /*
> > >>           * When finishing a compressed write bio we schedule a work queue item
> > >>           * to finish an ordered extent - end_bbio_compressed_write()
> > >> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > >> index 0fed4e0d32d5..7dbd9b8eec11 100644
> > >> --- a/fs/btrfs/file-item.c
> > >> +++ b/fs/btrfs/file-item.c
> > >> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
> > >>   /*
> > >>    * Calculate checksums of the data contained inside a bio.
> > >>    */
> > >> -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;
> > >> @@ -849,10 +849,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);
> > >>          schedule_work(&bbio->csum_work);
> > >> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> > >> index 6c678787c770..60a0eb17b3f6 100644
> > >> --- a/fs/btrfs/file-item.h
> > >> +++ b/fs/btrfs/file-item.h
> > >> @@ -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,
> > >> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> > >> index 3eba8438593c..203f7131f737 100644
> > >> --- a/fs/btrfs/fs.h
> > >> +++ b/fs/btrfs/fs.h
> > >> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
> > >>          /* All fs/file tree roots that have delalloc inodes. */
> > >>          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.
> > >> -        */
> > >> -       struct btrfs_workqueue *workers;
> > >>          struct btrfs_workqueue *delalloc_workers;
> > >>          struct btrfs_workqueue *flush_workers;
> > >>          struct workqueue_struct *endio_workers;
> > >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > >> index 464129b1b0d4..54f2da47c567 100644
> > >> --- a/fs/btrfs/super.c
> > >> +++ b/fs/btrfs/super.c
> > >> @@ -1237,7 +1237,6 @@ 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);
> > >>          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);
> > >> --
> > >> 2.55.0
> > >>
> > >
> >

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Qu Wenruo]

在 2026/9/3 22:14, Daniel Vacek 写道:
> On Thu, 3 Sept 2026 at 13:27, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>> 在 2026/9/3 18:50, Daniel Vacek 写道:
>>> On Thu, 3 Sept 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
>>>> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
>>>> introduced asynchronous data checksum generation for data writes.
>>>>
>>>> That feature can improve write performance, and was introduced in v6.19.
>>>> We have not experienced bugs related to that, so it's time to move it out
>>>> of experimental features for end users.
>>>>
>>>> Furthermore since the async checksum generation means
>>>> should_async_write() will always return false, we no longer need to
>>>> maintain the fs_info->workers workqueue, as the async checksum
>>>> generation is using the system_percpu_wq.
>>>
>>> I understand these are two related changes in one patch here. If
>>> removing async writes depends on async checksums, it would be nice to
>>> split this into a patch series.
>>>
>>> Though I guess can live with that. Looks good enough to me.
>>>
>>> Reviewed-by: Daniel Vacek <neelx@suse.com>
>>
>> Thanks a lot for the review.
>>
>>
>> I just want to mention that, Sashiko mentioned a problem that I believe
>> is valid, but not sure how realistic it will be in the real world.
>>
>>
>> Sashiko mentioned that, the old worker workqueue has WQ_MEM_RECLAIM
>> flag, thus there will always be a rescuer to ensure forward progress.
>> (Although for x86_64 and other common archs or EXPERIMENTAL builds, we
>> never utilize that worker anyway)
>>
>> But the new schedule_work() is using system_percpu_wq, which doesn't has
>> that flag.
> 
> Well, I think squashing this fixup to my patch is the right thing to do anyways:
> 
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 0fed4e0d32d5..984d87ad79b0 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -855,7 +855,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);
> -    schedule_work(&bbio->csum_work);
> +    queue_work(fs_info->endio_workers, &bbio->csum_work);
>       return 0;
>   }

This has one new problem though.

The csum_one_bio_work() can queue a new work into the same endio_workers.

It happens like this:

csum_one_bio_work()
|- bio_endio()
|  |- btrfs_simple_end_io()
       |- queue_work() into endio_workers again

This can cause problems with flush_workqueue() in close_ctree(), where 
we only flush the workqueue once, which doesn't ensure newly created 
work is also flushed.

We can use drain_workqueue() instead to fix it easily though.

Mind to send out a proper patch for it?

Thanks,
Qu

> 
> 
> That should make sashiko happy as well as btrfs end_io work running on
> btrfs end_io work queues.
> 
> And we should stay on the safe side wrt memory reclaim.
> 
> --nX
> 
> 
>> And since the csum generation is needed for data writeback, if under
>> very heavy memory pressure, there may be no worker to ensure the csum
>> work can be queued, in that case, it will hang the writeback (which is
>> triggered to reclaim memory).
>>
>> If we really want to address that problem, your idea about splitting the
>> patch will make a lot of sense.
>> We will need to keep the old infrastructure first, convert the worker to
>> a regular workqueue, and call queue_work(), then fully remove unused code.
>>
>>
>> But on the other hand, I doubt how pratically it is for anyone to stall
>> system_percpu_wq.
>> It's very common utilized across the whole kernel, and if it really
>> stalls I think there are a lot of more things to bother before btrfs.
>>
>> Another thing is, if we use fs_info::workers, we will follow the
>> existing wq flags, which means no PERCPU flag.
>> And according to the existing docs for those flags, PERCPU wq has a
>> better performance due to CPU locality.
>>
>>
>> So overall I think the current version is good enough, but not perfect.
>> On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will
>> definitely introduce some extra pentalty.
>>
>> If someone else has some idea on this Sashiko review, or very familiar
>> with the WQ_MEM_RECLAIM situation, any comment will be appreciated.
>>
>> Thanks,
>> Qu
>>
>>>
>>>> Suggested-by: Daniel Vacek <neelx@suse.com>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>>    fs/btrfs/bio.c       | 136 +------------------------------------------
>>>>    fs/btrfs/disk-io.c   |  20 +------
>>>>    fs/btrfs/file-item.c |   6 +-
>>>>    fs/btrfs/file-item.h |   2 +-
>>>>    fs/btrfs/fs.h        |  10 ----
>>>>    fs/btrfs/super.c     |   1 -
>>>>    6 files changed, 4 insertions(+), 171 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>>> index 771b7d598aee..e08006543c3d 100644
>>>> --- a/fs/btrfs/bio.c
>>>> +++ b/fs/btrfs/bio.c
>>>> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>>>>    {
>>>>           if (bbio->bio.bi_opf & REQ_META)
>>>>                   return btree_csum_one_bio(bbio);
>>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
>>>> -       return btrfs_csum_one_bio(bbio, true);
>>>> -#else
>>>> -       return btrfs_csum_one_bio(bbio, false);
>>>> -#endif
>>>> -}
>>>> -
>>>> -/*
>>>> - * Async submit bios are used to offload expensive checksumming onto the worker
>>>> - * threads.
>>>> - */
>>>> -struct async_submit_bio {
>>>> -       struct btrfs_bio *bbio;
>>>> -       struct btrfs_io_context *bioc;
>>>> -       struct btrfs_io_stripe smap;
>>>> -       int mirror_num;
>>>> -       struct btrfs_work work;
>>>> -};
>>>> -
>>>> -/*
>>>> - * 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 btree.
>>>> - */
>>>> -static void run_one_async_start(struct btrfs_work *work)
>>>> -{
>>>> -       struct async_submit_bio *async =
>>>> -               container_of(work, struct async_submit_bio, work);
>>>> -       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;
>>>> -       }
>>>> -
>>>> -       /* If an error occurred we just want to clean up the bio and move on. */
>>>> -       if (bio->bi_status) {
>>>> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
>>>> -               return;
>>>> -       }
>>>> -
>>>> -       /*
>>>> -        * All of the bios that pass through here are from async helpers.
>>>> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
>>>> -        * 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);
>>>> -}
>>>> -
>>>> -static bool should_async_write(struct btrfs_bio *bbio)
>>>> -{
>>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
>>>> -       bool auto_csum_mode = true;
>>>> -
>>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
>>>> -       /*
>>>> -        * Write bios will calculate checksum and submit bio at the same time.
>>>> -        * Unless explicitly required don't offload serial csum calculate and bio
>>>> -        * submit into a workqueue.
>>>> -        */
>>>> -       return false;
>>>> -#endif
>>>> -
>>>> -       /* Submit synchronously if the checksum implementation is fast. */
>>>> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
>>>> -               return false;
>>>> -
>>>> -       /*
>>>> -        * Try to defer the submission to a workqueue to parallelize the
>>>> -        * checksum calculation unless the I/O is issued synchronously.
>>>> -        */
>>>> -       if (op_is_sync(bbio->bio.bi_opf))
>>>> -               return false;
>>>> -
>>>> -       /* Zoned devices require I/O to be submitted in order. */
>>>> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
>>>> -               return false;
>>>> -
>>>> -       return true;
>>>> -}
>>>> -
>>>> -/*
>>>> - * Submit bio to an async queue.
>>>> - *
>>>> - * Return true if the work has been successfully submitted, else false.
>>>> - */
>>>> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
>>>> -                               struct btrfs_io_context *bioc,
>>>> -                               struct btrfs_io_stripe *smap, int mirror_num)
>>>> -{
>>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
>>>> -       struct async_submit_bio *async;
>>>> -
>>>> -       async = kmalloc_obj(*async, GFP_NOFS);
>>>> -       if (!async)
>>>> -               return false;
>>>> -
>>>> -       async->bbio = bbio;
>>>> -       async->bioc = bioc;
>>>> -       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);
>>>> -       return true;
>>>> +       return btrfs_csum_one_bio(bbio);
>>>>    }
>>>>
>>>>    static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
>>>> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>>>                   if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
>>>>                       !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
>>>>                       !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
>>>> -                       if (should_async_write(bbio) &&
>>>> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
>>>> -                               goto done;
>>>> -
>>>>                           ret = btrfs_bio_csum(bbio);
>>>>                           status = errno_to_blk_status(ret);
>>>>                           if (status)
>>>> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>>>           }
>>>>
>>>>           btrfs_submit_bio(bio, bioc, &smap, mirror_num);
>>>> -done:
>>>>           return map_length == length;
>>>>
>>>>    fail:
>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>> index a1d83ad9a4c0..0046f3cb75a1 100644
>>>> --- a/fs/btrfs/disk-io.c
>>>> +++ b/fs/btrfs/disk-io.c
>>>> @@ -1776,7 +1776,6 @@ 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->endio_workers)
>>>>                   destroy_workqueue(fs_info->endio_workers);
>>>>           if (fs_info->rmw_workers)
>>>> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
>>>>                   btrfs_alloc_workqueue(fs_info, "delalloc",
>>>>                                         flags, max_active, 2);
>>>> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
>>>>           fs_info->discard_ctl.discard_workers =
>>>>                   alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
>>>>
>>>> -       if (!(fs_info->workers &&
>>>> -             fs_info->delalloc_workers && fs_info->flush_workers &&
>>>> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
>>>>                 fs_info->endio_workers && fs_info->endio_meta_workers &&
>>>>                 fs_info->endio_write_workers &&
>>>>                 fs_info->endio_freespace_worker && fs_info->rmw_workers &&
>>>> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>>>>            */
>>>>           btrfs_flush_workqueue(fs_info->delalloc_workers);
>>>>
>>>> -       /*
>>>> -        * 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
>>>> -        * 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.
>>>> -        */
>>>> -       btrfs_flush_workqueue(fs_info->workers);
>>>> -
>>>>           /*
>>>>            * When finishing a compressed write bio we schedule a work queue item
>>>>            * to finish an ordered extent - end_bbio_compressed_write()
>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>>> index 0fed4e0d32d5..7dbd9b8eec11 100644
>>>> --- a/fs/btrfs/file-item.c
>>>> +++ b/fs/btrfs/file-item.c
>>>> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
>>>>    /*
>>>>     * Calculate checksums of the data contained inside a bio.
>>>>     */
>>>> -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;
>>>> @@ -849,10 +849,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);
>>>>           schedule_work(&bbio->csum_work);
>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>>> index 6c678787c770..60a0eb17b3f6 100644
>>>> --- a/fs/btrfs/file-item.h
>>>> +++ b/fs/btrfs/file-item.h
>>>> @@ -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,
>>>> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
>>>> index 3eba8438593c..203f7131f737 100644
>>>> --- a/fs/btrfs/fs.h
>>>> +++ b/fs/btrfs/fs.h
>>>> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
>>>>           /* All fs/file tree roots that have delalloc inodes. */
>>>>           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.
>>>> -        */
>>>> -       struct btrfs_workqueue *workers;
>>>>           struct btrfs_workqueue *delalloc_workers;
>>>>           struct btrfs_workqueue *flush_workers;
>>>>           struct workqueue_struct *endio_workers;
>>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>>>> index 464129b1b0d4..54f2da47c567 100644
>>>> --- a/fs/btrfs/super.c
>>>> +++ b/fs/btrfs/super.c
>>>> @@ -1237,7 +1237,6 @@ 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);
>>>>           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);
>>>> --
>>>> 2.55.0
>>>>
>>>
>>

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Daniel Vacek]

On Sat, 5 Sept 2026 at 02:57, Qu Wenruo <wqu@suse.com> wrote:
> 在 2026/9/3 22:14, Daniel Vacek 写道:
> > On Thu, 3 Sept 2026 at 13:27, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> >> 在 2026/9/3 18:50, Daniel Vacek 写道:
> >>> On Thu, 3 Sept 2026 at 10:40, Qu Wenruo <wqu@suse.com> wrote:
> >>>> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
> >>>> introduced asynchronous data checksum generation for data writes.
> >>>>
> >>>> That feature can improve write performance, and was introduced in v6.19.
> >>>> We have not experienced bugs related to that, so it's time to move it out
> >>>> of experimental features for end users.
> >>>>
> >>>> Furthermore since the async checksum generation means
> >>>> should_async_write() will always return false, we no longer need to
> >>>> maintain the fs_info->workers workqueue, as the async checksum
> >>>> generation is using the system_percpu_wq.
> >>>
> >>> I understand these are two related changes in one patch here. If
> >>> removing async writes depends on async checksums, it would be nice to
> >>> split this into a patch series.
> >>>
> >>> Though I guess can live with that. Looks good enough to me.
> >>>
> >>> Reviewed-by: Daniel Vacek <neelx@suse.com>
> >>
> >> Thanks a lot for the review.
> >>
> >>
> >> I just want to mention that, Sashiko mentioned a problem that I believe
> >> is valid, but not sure how realistic it will be in the real world.
> >>
> >>
> >> Sashiko mentioned that, the old worker workqueue has WQ_MEM_RECLAIM
> >> flag, thus there will always be a rescuer to ensure forward progress.
> >> (Although for x86_64 and other common archs or EXPERIMENTAL builds, we
> >> never utilize that worker anyway)
> >>
> >> But the new schedule_work() is using system_percpu_wq, which doesn't has
> >> that flag.
> >
> > Well, I think squashing this fixup to my patch is the right thing to do anyways:
> >
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 0fed4e0d32d5..984d87ad79b0 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -855,7 +855,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);
> > -    schedule_work(&bbio->csum_work);
> > +    queue_work(fs_info->endio_workers, &bbio->csum_work);
> >       return 0;
> >   }
>
> This has one new problem though.
>
> The csum_one_bio_work() can queue a new work into the same endio_workers.
>
> It happens like this:
>
> csum_one_bio_work()
> |- bio_endio()
> |  |- btrfs_simple_end_io()
>        |- queue_work() into endio_workers again
>
> This can cause problems with flush_workqueue() in close_ctree(), where
> we only flush the workqueue once, which doesn't ensure newly created
> work is also flushed.

Do you know of any situation where this can happen?

If btrfs_simple_end_io() can queue a work after close_ctree() has
flushed the wq, that would be buggy even before. IIUC, it does not
matter whether btrfs_simple_end_io() is running from interrupt (or
interrupt thread) or from the worker thread.

I understand that at the point when close_ctree() flushes
endio_workers we know there will be no new interrupt ending the io
request (that could queue an additional work). And since we know there
won't be a new interrupt, we also know there is no csum_one_bio_work()
queued. Right?

So can it even happen? Coz unless I'm mistaken in the reasoning above,
I think we're safe regarding your suspicion.

--nX

> We can use drain_workqueue() instead to fix it easily though.
>
> Mind to send out a proper patch for it?
>
> Thanks,
> Qu
>
> >
> >
> > That should make sashiko happy as well as btrfs end_io work running on
> > btrfs end_io work queues.
> >
> > And we should stay on the safe side wrt memory reclaim.
> >
> > --nX
> >
> >
> >> And since the csum generation is needed for data writeback, if under
> >> very heavy memory pressure, there may be no worker to ensure the csum
> >> work can be queued, in that case, it will hang the writeback (which is
> >> triggered to reclaim memory).
> >>
> >> If we really want to address that problem, your idea about splitting the
> >> patch will make a lot of sense.
> >> We will need to keep the old infrastructure first, convert the worker to
> >> a regular workqueue, and call queue_work(), then fully remove unused code.
> >>
> >>
> >> But on the other hand, I doubt how pratically it is for anyone to stall
> >> system_percpu_wq.
> >> It's very common utilized across the whole kernel, and if it really
> >> stalls I think there are a lot of more things to bother before btrfs.
> >>
> >> Another thing is, if we use fs_info::workers, we will follow the
> >> existing wq flags, which means no PERCPU flag.
> >> And according to the existing docs for those flags, PERCPU wq has a
> >> better performance due to CPU locality.
> >>
> >>
> >> So overall I think the current version is good enough, but not perfect.
> >> On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will
> >> definitely introduce some extra pentalty.
> >>
> >> If someone else has some idea on this Sashiko review, or very familiar
> >> with the WQ_MEM_RECLAIM situation, any comment will be appreciated.
> >>
> >> Thanks,
> >> Qu
> >>
> >>>
> >>>> Suggested-by: Daniel Vacek <neelx@suse.com>
> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >>>> ---
> >>>>    fs/btrfs/bio.c       | 136 +------------------------------------------
> >>>>    fs/btrfs/disk-io.c   |  20 +------
> >>>>    fs/btrfs/file-item.c |   6 +-
> >>>>    fs/btrfs/file-item.h |   2 +-
> >>>>    fs/btrfs/fs.h        |  10 ----
> >>>>    fs/btrfs/super.c     |   1 -
> >>>>    6 files changed, 4 insertions(+), 171 deletions(-)
> >>>>
> >>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >>>> index 771b7d598aee..e08006543c3d 100644
> >>>> --- a/fs/btrfs/bio.c
> >>>> +++ b/fs/btrfs/bio.c
> >>>> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> >>>>    {
> >>>>           if (bbio->bio.bi_opf & REQ_META)
> >>>>                   return btree_csum_one_bio(bbio);
> >>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> >>>> -       return btrfs_csum_one_bio(bbio, true);
> >>>> -#else
> >>>> -       return btrfs_csum_one_bio(bbio, false);
> >>>> -#endif
> >>>> -}
> >>>> -
> >>>> -/*
> >>>> - * Async submit bios are used to offload expensive checksumming onto the worker
> >>>> - * threads.
> >>>> - */
> >>>> -struct async_submit_bio {
> >>>> -       struct btrfs_bio *bbio;
> >>>> -       struct btrfs_io_context *bioc;
> >>>> -       struct btrfs_io_stripe smap;
> >>>> -       int mirror_num;
> >>>> -       struct btrfs_work work;
> >>>> -};
> >>>> -
> >>>> -/*
> >>>> - * 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 btree.
> >>>> - */
> >>>> -static void run_one_async_start(struct btrfs_work *work)
> >>>> -{
> >>>> -       struct async_submit_bio *async =
> >>>> -               container_of(work, struct async_submit_bio, work);
> >>>> -       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;
> >>>> -       }
> >>>> -
> >>>> -       /* If an error occurred we just want to clean up the bio and move on. */
> >>>> -       if (bio->bi_status) {
> >>>> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
> >>>> -               return;
> >>>> -       }
> >>>> -
> >>>> -       /*
> >>>> -        * All of the bios that pass through here are from async helpers.
> >>>> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
> >>>> -        * 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);
> >>>> -}
> >>>> -
> >>>> -static bool should_async_write(struct btrfs_bio *bbio)
> >>>> -{
> >>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> >>>> -       bool auto_csum_mode = true;
> >>>> -
> >>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> >>>> -       /*
> >>>> -        * Write bios will calculate checksum and submit bio at the same time.
> >>>> -        * Unless explicitly required don't offload serial csum calculate and bio
> >>>> -        * submit into a workqueue.
> >>>> -        */
> >>>> -       return false;
> >>>> -#endif
> >>>> -
> >>>> -       /* Submit synchronously if the checksum implementation is fast. */
> >>>> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
> >>>> -               return false;
> >>>> -
> >>>> -       /*
> >>>> -        * Try to defer the submission to a workqueue to parallelize the
> >>>> -        * checksum calculation unless the I/O is issued synchronously.
> >>>> -        */
> >>>> -       if (op_is_sync(bbio->bio.bi_opf))
> >>>> -               return false;
> >>>> -
> >>>> -       /* Zoned devices require I/O to be submitted in order. */
> >>>> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
> >>>> -               return false;
> >>>> -
> >>>> -       return true;
> >>>> -}
> >>>> -
> >>>> -/*
> >>>> - * Submit bio to an async queue.
> >>>> - *
> >>>> - * Return true if the work has been successfully submitted, else false.
> >>>> - */
> >>>> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
> >>>> -                               struct btrfs_io_context *bioc,
> >>>> -                               struct btrfs_io_stripe *smap, int mirror_num)
> >>>> -{
> >>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> >>>> -       struct async_submit_bio *async;
> >>>> -
> >>>> -       async = kmalloc_obj(*async, GFP_NOFS);
> >>>> -       if (!async)
> >>>> -               return false;
> >>>> -
> >>>> -       async->bbio = bbio;
> >>>> -       async->bioc = bioc;
> >>>> -       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);
> >>>> -       return true;
> >>>> +       return btrfs_csum_one_bio(bbio);
> >>>>    }
> >>>>
> >>>>    static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
> >>>> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >>>>                   if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
> >>>>                       !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
> >>>>                       !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
> >>>> -                       if (should_async_write(bbio) &&
> >>>> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
> >>>> -                               goto done;
> >>>> -
> >>>>                           ret = btrfs_bio_csum(bbio);
> >>>>                           status = errno_to_blk_status(ret);
> >>>>                           if (status)
> >>>> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >>>>           }
> >>>>
> >>>>           btrfs_submit_bio(bio, bioc, &smap, mirror_num);
> >>>> -done:
> >>>>           return map_length == length;
> >>>>
> >>>>    fail:
> >>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> >>>> index a1d83ad9a4c0..0046f3cb75a1 100644
> >>>> --- a/fs/btrfs/disk-io.c
> >>>> +++ b/fs/btrfs/disk-io.c
> >>>> @@ -1776,7 +1776,6 @@ 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->endio_workers)
> >>>>                   destroy_workqueue(fs_info->endio_workers);
> >>>>           if (fs_info->rmw_workers)
> >>>> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
> >>>>                   btrfs_alloc_workqueue(fs_info, "delalloc",
> >>>>                                         flags, max_active, 2);
> >>>> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
> >>>>           fs_info->discard_ctl.discard_workers =
> >>>>                   alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
> >>>>
> >>>> -       if (!(fs_info->workers &&
> >>>> -             fs_info->delalloc_workers && fs_info->flush_workers &&
> >>>> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
> >>>>                 fs_info->endio_workers && fs_info->endio_meta_workers &&
> >>>>                 fs_info->endio_write_workers &&
> >>>>                 fs_info->endio_freespace_worker && fs_info->rmw_workers &&
> >>>> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> >>>>            */
> >>>>           btrfs_flush_workqueue(fs_info->delalloc_workers);
> >>>>
> >>>> -       /*
> >>>> -        * 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
> >>>> -        * 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.
> >>>> -        */
> >>>> -       btrfs_flush_workqueue(fs_info->workers);
> >>>> -
> >>>>           /*
> >>>>            * When finishing a compressed write bio we schedule a work queue item
> >>>>            * to finish an ordered extent - end_bbio_compressed_write()
> >>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >>>> index 0fed4e0d32d5..7dbd9b8eec11 100644
> >>>> --- a/fs/btrfs/file-item.c
> >>>> +++ b/fs/btrfs/file-item.c
> >>>> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
> >>>>    /*
> >>>>     * Calculate checksums of the data contained inside a bio.
> >>>>     */
> >>>> -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;
> >>>> @@ -849,10 +849,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);
> >>>>           schedule_work(&bbio->csum_work);
> >>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >>>> index 6c678787c770..60a0eb17b3f6 100644
> >>>> --- a/fs/btrfs/file-item.h
> >>>> +++ b/fs/btrfs/file-item.h
> >>>> @@ -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,
> >>>> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> >>>> index 3eba8438593c..203f7131f737 100644
> >>>> --- a/fs/btrfs/fs.h
> >>>> +++ b/fs/btrfs/fs.h
> >>>> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
> >>>>           /* All fs/file tree roots that have delalloc inodes. */
> >>>>           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.
> >>>> -        */
> >>>> -       struct btrfs_workqueue *workers;
> >>>>           struct btrfs_workqueue *delalloc_workers;
> >>>>           struct btrfs_workqueue *flush_workers;
> >>>>           struct workqueue_struct *endio_workers;
> >>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> >>>> index 464129b1b0d4..54f2da47c567 100644
> >>>> --- a/fs/btrfs/super.c
> >>>> +++ b/fs/btrfs/super.c
> >>>> @@ -1237,7 +1237,6 @@ 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);
> >>>>           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);
> >>>> --
> >>>> 2.55.0
> >>>>
> >>>
> >>
>

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Qu Wenruo]

在 2026/9/5 19:39, Daniel Vacek 写道:
> On Sat, 5 Sept 2026 at 02:57, Qu Wenruo <wqu@suse.com> wrote:
[...]
>> This has one new problem though.
>>
>> The csum_one_bio_work() can queue a new work into the same endio_workers.
>>
>> It happens like this:
>>
>> csum_one_bio_work()
>> |- bio_endio()
>> |  |- btrfs_simple_end_io()
>>         |- queue_work() into endio_workers again
>>
>> This can cause problems with flush_workqueue() in close_ctree(), where
>> we only flush the workqueue once, which doesn't ensure newly created
>> work is also flushed.
> 
> Do you know of any situation where this can happen?

Check the commit message of cda76788f8b0 ("btrfs: fix non-empty delayed 
iputs list on unmount due to async workers").

With your change, we just move the csum generation and endio workload 
from workers to endio_workers.
Otherwise it's the same.

> 
> If btrfs_simple_end_io() can queue a work after close_ctree() has
> flushed the wq, that would be buggy even before. IIUC, it does not
> matter whether btrfs_simple_end_io() is running from interrupt (or
> interrupt thread) or from the worker thread.
> 
> I understand that at the point when close_ctree() flushes
> endio_workers we know there will be no new interrupt ending the io
> request (that could queue an additional work). And since we know there
> won't be a new interrupt, we also know there is no csum_one_bio_work()
> queued. Right?
> 
> So can it even happen? Coz unless I'm mistaken in the reasoning above,
> I think we're safe regarding your suspicion.
> 
> --nX
> 
>> We can use drain_workqueue() instead to fix it easily though.
>>
>> Mind to send out a proper patch for it?
>>
>> Thanks,
>> Qu
>>
>>>
>>>
>>> That should make sashiko happy as well as btrfs end_io work running on
>>> btrfs end_io work queues.
>>>
>>> And we should stay on the safe side wrt memory reclaim.
>>>
>>> --nX
>>>
>>>
>>>> And since the csum generation is needed for data writeback, if under
>>>> very heavy memory pressure, there may be no worker to ensure the csum
>>>> work can be queued, in that case, it will hang the writeback (which is
>>>> triggered to reclaim memory).
>>>>
>>>> If we really want to address that problem, your idea about splitting the
>>>> patch will make a lot of sense.
>>>> We will need to keep the old infrastructure first, convert the worker to
>>>> a regular workqueue, and call queue_work(), then fully remove unused code.
>>>>
>>>>
>>>> But on the other hand, I doubt how pratically it is for anyone to stall
>>>> system_percpu_wq.
>>>> It's very common utilized across the whole kernel, and if it really
>>>> stalls I think there are a lot of more things to bother before btrfs.
>>>>
>>>> Another thing is, if we use fs_info::workers, we will follow the
>>>> existing wq flags, which means no PERCPU flag.
>>>> And according to the existing docs for those flags, PERCPU wq has a
>>>> better performance due to CPU locality.
>>>>
>>>>
>>>> So overall I think the current version is good enough, but not perfect.
>>>> On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will
>>>> definitely introduce some extra pentalty.
>>>>
>>>> If someone else has some idea on this Sashiko review, or very familiar
>>>> with the WQ_MEM_RECLAIM situation, any comment will be appreciated.
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>>
>>>>>> Suggested-by: Daniel Vacek <neelx@suse.com>
>>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>>>> ---
>>>>>>     fs/btrfs/bio.c       | 136 +------------------------------------------
>>>>>>     fs/btrfs/disk-io.c   |  20 +------
>>>>>>     fs/btrfs/file-item.c |   6 +-
>>>>>>     fs/btrfs/file-item.h |   2 +-
>>>>>>     fs/btrfs/fs.h        |  10 ----
>>>>>>     fs/btrfs/super.c     |   1 -
>>>>>>     6 files changed, 4 insertions(+), 171 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>>>>> index 771b7d598aee..e08006543c3d 100644
>>>>>> --- a/fs/btrfs/bio.c
>>>>>> +++ b/fs/btrfs/bio.c
>>>>>> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>>>>>>     {
>>>>>>            if (bbio->bio.bi_opf & REQ_META)
>>>>>>                    return btree_csum_one_bio(bbio);
>>>>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
>>>>>> -       return btrfs_csum_one_bio(bbio, true);
>>>>>> -#else
>>>>>> -       return btrfs_csum_one_bio(bbio, false);
>>>>>> -#endif
>>>>>> -}
>>>>>> -
>>>>>> -/*
>>>>>> - * Async submit bios are used to offload expensive checksumming onto the worker
>>>>>> - * threads.
>>>>>> - */
>>>>>> -struct async_submit_bio {
>>>>>> -       struct btrfs_bio *bbio;
>>>>>> -       struct btrfs_io_context *bioc;
>>>>>> -       struct btrfs_io_stripe smap;
>>>>>> -       int mirror_num;
>>>>>> -       struct btrfs_work work;
>>>>>> -};
>>>>>> -
>>>>>> -/*
>>>>>> - * 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 btree.
>>>>>> - */
>>>>>> -static void run_one_async_start(struct btrfs_work *work)
>>>>>> -{
>>>>>> -       struct async_submit_bio *async =
>>>>>> -               container_of(work, struct async_submit_bio, work);
>>>>>> -       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;
>>>>>> -       }
>>>>>> -
>>>>>> -       /* If an error occurred we just want to clean up the bio and move on. */
>>>>>> -       if (bio->bi_status) {
>>>>>> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
>>>>>> -               return;
>>>>>> -       }
>>>>>> -
>>>>>> -       /*
>>>>>> -        * All of the bios that pass through here are from async helpers.
>>>>>> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
>>>>>> -        * 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);
>>>>>> -}
>>>>>> -
>>>>>> -static bool should_async_write(struct btrfs_bio *bbio)
>>>>>> -{
>>>>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
>>>>>> -       bool auto_csum_mode = true;
>>>>>> -
>>>>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
>>>>>> -       /*
>>>>>> -        * Write bios will calculate checksum and submit bio at the same time.
>>>>>> -        * Unless explicitly required don't offload serial csum calculate and bio
>>>>>> -        * submit into a workqueue.
>>>>>> -        */
>>>>>> -       return false;
>>>>>> -#endif
>>>>>> -
>>>>>> -       /* Submit synchronously if the checksum implementation is fast. */
>>>>>> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
>>>>>> -               return false;
>>>>>> -
>>>>>> -       /*
>>>>>> -        * Try to defer the submission to a workqueue to parallelize the
>>>>>> -        * checksum calculation unless the I/O is issued synchronously.
>>>>>> -        */
>>>>>> -       if (op_is_sync(bbio->bio.bi_opf))
>>>>>> -               return false;
>>>>>> -
>>>>>> -       /* Zoned devices require I/O to be submitted in order. */
>>>>>> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
>>>>>> -               return false;
>>>>>> -
>>>>>> -       return true;
>>>>>> -}
>>>>>> -
>>>>>> -/*
>>>>>> - * Submit bio to an async queue.
>>>>>> - *
>>>>>> - * Return true if the work has been successfully submitted, else false.
>>>>>> - */
>>>>>> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
>>>>>> -                               struct btrfs_io_context *bioc,
>>>>>> -                               struct btrfs_io_stripe *smap, int mirror_num)
>>>>>> -{
>>>>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
>>>>>> -       struct async_submit_bio *async;
>>>>>> -
>>>>>> -       async = kmalloc_obj(*async, GFP_NOFS);
>>>>>> -       if (!async)
>>>>>> -               return false;
>>>>>> -
>>>>>> -       async->bbio = bbio;
>>>>>> -       async->bioc = bioc;
>>>>>> -       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);
>>>>>> -       return true;
>>>>>> +       return btrfs_csum_one_bio(bbio);
>>>>>>     }
>>>>>>
>>>>>>     static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
>>>>>> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>>>>>                    if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
>>>>>>                        !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
>>>>>>                        !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
>>>>>> -                       if (should_async_write(bbio) &&
>>>>>> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
>>>>>> -                               goto done;
>>>>>> -
>>>>>>                            ret = btrfs_bio_csum(bbio);
>>>>>>                            status = errno_to_blk_status(ret);
>>>>>>                            if (status)
>>>>>> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>>>>>            }
>>>>>>
>>>>>>            btrfs_submit_bio(bio, bioc, &smap, mirror_num);
>>>>>> -done:
>>>>>>            return map_length == length;
>>>>>>
>>>>>>     fail:
>>>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>>>> index a1d83ad9a4c0..0046f3cb75a1 100644
>>>>>> --- a/fs/btrfs/disk-io.c
>>>>>> +++ b/fs/btrfs/disk-io.c
>>>>>> @@ -1776,7 +1776,6 @@ 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->endio_workers)
>>>>>>                    destroy_workqueue(fs_info->endio_workers);
>>>>>>            if (fs_info->rmw_workers)
>>>>>> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
>>>>>>                    btrfs_alloc_workqueue(fs_info, "delalloc",
>>>>>>                                          flags, max_active, 2);
>>>>>> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
>>>>>>            fs_info->discard_ctl.discard_workers =
>>>>>>                    alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
>>>>>>
>>>>>> -       if (!(fs_info->workers &&
>>>>>> -             fs_info->delalloc_workers && fs_info->flush_workers &&
>>>>>> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
>>>>>>                  fs_info->endio_workers && fs_info->endio_meta_workers &&
>>>>>>                  fs_info->endio_write_workers &&
>>>>>>                  fs_info->endio_freespace_worker && fs_info->rmw_workers &&
>>>>>> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>>>>>>             */
>>>>>>            btrfs_flush_workqueue(fs_info->delalloc_workers);
>>>>>>
>>>>>> -       /*
>>>>>> -        * 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
>>>>>> -        * 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.
>>>>>> -        */
>>>>>> -       btrfs_flush_workqueue(fs_info->workers);
>>>>>> -
>>>>>>            /*
>>>>>>             * When finishing a compressed write bio we schedule a work queue item
>>>>>>             * to finish an ordered extent - end_bbio_compressed_write()
>>>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>>>>> index 0fed4e0d32d5..7dbd9b8eec11 100644
>>>>>> --- a/fs/btrfs/file-item.c
>>>>>> +++ b/fs/btrfs/file-item.c
>>>>>> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
>>>>>>     /*
>>>>>>      * Calculate checksums of the data contained inside a bio.
>>>>>>      */
>>>>>> -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;
>>>>>> @@ -849,10 +849,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);
>>>>>>            schedule_work(&bbio->csum_work);
>>>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>>>>> index 6c678787c770..60a0eb17b3f6 100644
>>>>>> --- a/fs/btrfs/file-item.h
>>>>>> +++ b/fs/btrfs/file-item.h
>>>>>> @@ -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,
>>>>>> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
>>>>>> index 3eba8438593c..203f7131f737 100644
>>>>>> --- a/fs/btrfs/fs.h
>>>>>> +++ b/fs/btrfs/fs.h
>>>>>> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
>>>>>>            /* All fs/file tree roots that have delalloc inodes. */
>>>>>>            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.
>>>>>> -        */
>>>>>> -       struct btrfs_workqueue *workers;
>>>>>>            struct btrfs_workqueue *delalloc_workers;
>>>>>>            struct btrfs_workqueue *flush_workers;
>>>>>>            struct workqueue_struct *endio_workers;
>>>>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>>>>>> index 464129b1b0d4..54f2da47c567 100644
>>>>>> --- a/fs/btrfs/super.c
>>>>>> +++ b/fs/btrfs/super.c
>>>>>> @@ -1237,7 +1237,6 @@ 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);
>>>>>>            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);
>>>>>> --
>>>>>> 2.55.0
>>>>>>
>>>>>
>>>>
>>
> 

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Daniel Vacek]

On Sat, 5 Sept 2026 at 12:47, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 在 2026/9/5 19:39, Daniel Vacek 写道:
> > On Sat, 5 Sept 2026 at 02:57, Qu Wenruo <wqu@suse.com> wrote:
> [...]
> >> This has one new problem though.
> >>
> >> The csum_one_bio_work() can queue a new work into the same endio_workers.
> >>
> >> It happens like this:
> >>
> >> csum_one_bio_work()
> >> |- bio_endio()
> >> |  |- btrfs_simple_end_io()
> >>         |- queue_work() into endio_workers again
> >>
> >> This can cause problems with flush_workqueue() in close_ctree(), where
> >> we only flush the workqueue once, which doesn't ensure newly created
> >> work is also flushed.
> >
> > Do you know of any situation where this can happen?
>
> Check the commit message of cda76788f8b0 ("btrfs: fix non-empty delayed
> iputs list on unmount due to async workers").
>
> With your change, we just move the csum generation and endio workload
> from workers to endio_workers.
> Otherwise it's the same.

Perhaps I'm getting lost here, but csums were never in workers and
nothing else moved. Or what am I missing?

edit: Nah, I see. I'm still thinking experimental. As basically that's
the default for me.
      But still, I don't see an issue (yet??). Perhaps Filipe can have a look?

> >
> > If btrfs_simple_end_io() can queue a work after close_ctree() has
> > flushed the wq, that would be buggy even before. IIUC, it does not
> > matter whether btrfs_simple_end_io() is running from interrupt (or
> > interrupt thread) or from the worker thread.
> >
> > I understand that at the point when close_ctree() flushes
> > endio_workers we know there will be no new interrupt ending the io
> > request (that could queue an additional work). And since we know there
> > won't be a new interrupt, we also know there is no csum_one_bio_work()
> > queued. Right?

Does this reasoning make sense to you? I still think the logic proves
the change is correct and safe.

--nX


> > So can it even happen? Coz unless I'm mistaken in the reasoning above,
> > I think we're safe regarding your suspicion.
> >
> > --nX
> >
> >> We can use drain_workqueue() instead to fix it easily though.
> >>
> >> Mind to send out a proper patch for it?
> >>
> >> Thanks,
> >> Qu
> >>
> >>>
> >>>
> >>> That should make sashiko happy as well as btrfs end_io work running on
> >>> btrfs end_io work queues.
> >>>
> >>> And we should stay on the safe side wrt memory reclaim.
> >>>
> >>> --nX
> >>>
> >>>
> >>>> And since the csum generation is needed for data writeback, if under
> >>>> very heavy memory pressure, there may be no worker to ensure the csum
> >>>> work can be queued, in that case, it will hang the writeback (which is
> >>>> triggered to reclaim memory).
> >>>>
> >>>> If we really want to address that problem, your idea about splitting the
> >>>> patch will make a lot of sense.
> >>>> We will need to keep the old infrastructure first, convert the worker to
> >>>> a regular workqueue, and call queue_work(), then fully remove unused code.
> >>>>
> >>>>
> >>>> But on the other hand, I doubt how pratically it is for anyone to stall
> >>>> system_percpu_wq.
> >>>> It's very common utilized across the whole kernel, and if it really
> >>>> stalls I think there are a lot of more things to bother before btrfs.
> >>>>
> >>>> Another thing is, if we use fs_info::workers, we will follow the
> >>>> existing wq flags, which means no PERCPU flag.
> >>>> And according to the existing docs for those flags, PERCPU wq has a
> >>>> better performance due to CPU locality.
> >>>>
> >>>>
> >>>> So overall I think the current version is good enough, but not perfect.
> >>>> On the other hand, using a WQ_MEM_RECLAIM seems more "correct", but will
> >>>> definitely introduce some extra pentalty.
> >>>>
> >>>> If someone else has some idea on this Sashiko review, or very familiar
> >>>> with the WQ_MEM_RECLAIM situation, any comment will be appreciated.
> >>>>
> >>>> Thanks,
> >>>> Qu
> >>>>
> >>>>>
> >>>>>> Suggested-by: Daniel Vacek <neelx@suse.com>
> >>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >>>>>> ---
> >>>>>>     fs/btrfs/bio.c       | 136 +------------------------------------------
> >>>>>>     fs/btrfs/disk-io.c   |  20 +------
> >>>>>>     fs/btrfs/file-item.c |   6 +-
> >>>>>>     fs/btrfs/file-item.h |   2 +-
> >>>>>>     fs/btrfs/fs.h        |  10 ----
> >>>>>>     fs/btrfs/super.c     |   1 -
> >>>>>>     6 files changed, 4 insertions(+), 171 deletions(-)
> >>>>>>
> >>>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >>>>>> index 771b7d598aee..e08006543c3d 100644
> >>>>>> --- a/fs/btrfs/bio.c
> >>>>>> +++ b/fs/btrfs/bio.c
> >>>>>> @@ -569,136 +569,7 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> >>>>>>     {
> >>>>>>            if (bbio->bio.bi_opf & REQ_META)
> >>>>>>                    return btree_csum_one_bio(bbio);
> >>>>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> >>>>>> -       return btrfs_csum_one_bio(bbio, true);
> >>>>>> -#else
> >>>>>> -       return btrfs_csum_one_bio(bbio, false);
> >>>>>> -#endif
> >>>>>> -}
> >>>>>> -
> >>>>>> -/*
> >>>>>> - * Async submit bios are used to offload expensive checksumming onto the worker
> >>>>>> - * threads.
> >>>>>> - */
> >>>>>> -struct async_submit_bio {
> >>>>>> -       struct btrfs_bio *bbio;
> >>>>>> -       struct btrfs_io_context *bioc;
> >>>>>> -       struct btrfs_io_stripe smap;
> >>>>>> -       int mirror_num;
> >>>>>> -       struct btrfs_work work;
> >>>>>> -};
> >>>>>> -
> >>>>>> -/*
> >>>>>> - * 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 btree.
> >>>>>> - */
> >>>>>> -static void run_one_async_start(struct btrfs_work *work)
> >>>>>> -{
> >>>>>> -       struct async_submit_bio *async =
> >>>>>> -               container_of(work, struct async_submit_bio, work);
> >>>>>> -       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;
> >>>>>> -       }
> >>>>>> -
> >>>>>> -       /* If an error occurred we just want to clean up the bio and move on. */
> >>>>>> -       if (bio->bi_status) {
> >>>>>> -               btrfs_bio_end_io(async->bbio, bio->bi_status);
> >>>>>> -               return;
> >>>>>> -       }
> >>>>>> -
> >>>>>> -       /*
> >>>>>> -        * All of the bios that pass through here are from async helpers.
> >>>>>> -        * Use REQ_BTRFS_CGROUP_PUNT to issue them from the owning cgroup's
> >>>>>> -        * 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);
> >>>>>> -}
> >>>>>> -
> >>>>>> -static bool should_async_write(struct btrfs_bio *bbio)
> >>>>>> -{
> >>>>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> >>>>>> -       bool auto_csum_mode = true;
> >>>>>> -
> >>>>>> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> >>>>>> -       /*
> >>>>>> -        * Write bios will calculate checksum and submit bio at the same time.
> >>>>>> -        * Unless explicitly required don't offload serial csum calculate and bio
> >>>>>> -        * submit into a workqueue.
> >>>>>> -        */
> >>>>>> -       return false;
> >>>>>> -#endif
> >>>>>> -
> >>>>>> -       /* Submit synchronously if the checksum implementation is fast. */
> >>>>>> -       if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
> >>>>>> -               return false;
> >>>>>> -
> >>>>>> -       /*
> >>>>>> -        * Try to defer the submission to a workqueue to parallelize the
> >>>>>> -        * checksum calculation unless the I/O is issued synchronously.
> >>>>>> -        */
> >>>>>> -       if (op_is_sync(bbio->bio.bi_opf))
> >>>>>> -               return false;
> >>>>>> -
> >>>>>> -       /* Zoned devices require I/O to be submitted in order. */
> >>>>>> -       if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
> >>>>>> -               return false;
> >>>>>> -
> >>>>>> -       return true;
> >>>>>> -}
> >>>>>> -
> >>>>>> -/*
> >>>>>> - * Submit bio to an async queue.
> >>>>>> - *
> >>>>>> - * Return true if the work has been successfully submitted, else false.
> >>>>>> - */
> >>>>>> -static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
> >>>>>> -                               struct btrfs_io_context *bioc,
> >>>>>> -                               struct btrfs_io_stripe *smap, int mirror_num)
> >>>>>> -{
> >>>>>> -       struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> >>>>>> -       struct async_submit_bio *async;
> >>>>>> -
> >>>>>> -       async = kmalloc_obj(*async, GFP_NOFS);
> >>>>>> -       if (!async)
> >>>>>> -               return false;
> >>>>>> -
> >>>>>> -       async->bbio = bbio;
> >>>>>> -       async->bioc = bioc;
> >>>>>> -       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);
> >>>>>> -       return true;
> >>>>>> +       return btrfs_csum_one_bio(bbio);
> >>>>>>     }
> >>>>>>
> >>>>>>     static u64 btrfs_append_map_length(struct btrfs_bio *bbio, u64 map_length)
> >>>>>> @@ -806,10 +677,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >>>>>>                    if (!(inode->flags & BTRFS_INODE_NODATASUM) &&
> >>>>>>                        !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
> >>>>>>                        !btrfs_is_data_reloc_root(inode->root) && !bbio->is_remap) {
> >>>>>> -                       if (should_async_write(bbio) &&
> >>>>>> -                           btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
> >>>>>> -                               goto done;
> >>>>>> -
> >>>>>>                            ret = btrfs_bio_csum(bbio);
> >>>>>>                            status = errno_to_blk_status(ret);
> >>>>>>                            if (status)
> >>>>>> @@ -824,7 +691,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >>>>>>            }
> >>>>>>
> >>>>>>            btrfs_submit_bio(bio, bioc, &smap, mirror_num);
> >>>>>> -done:
> >>>>>>            return map_length == length;
> >>>>>>
> >>>>>>     fail:
> >>>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> >>>>>> index a1d83ad9a4c0..0046f3cb75a1 100644
> >>>>>> --- a/fs/btrfs/disk-io.c
> >>>>>> +++ b/fs/btrfs/disk-io.c
> >>>>>> @@ -1776,7 +1776,6 @@ 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->endio_workers)
> >>>>>>                    destroy_workqueue(fs_info->endio_workers);
> >>>>>>            if (fs_info->rmw_workers)
> >>>>>> @@ -1968,9 +1967,6 @@ 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->delalloc_workers =
> >>>>>>                    btrfs_alloc_workqueue(fs_info, "delalloc",
> >>>>>>                                          flags, max_active, 2);
> >>>>>> @@ -2005,8 +2001,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
> >>>>>>            fs_info->discard_ctl.discard_workers =
> >>>>>>                    alloc_ordered_workqueue("btrfs-discard", WQ_FREEZABLE);
> >>>>>>
> >>>>>> -       if (!(fs_info->workers &&
> >>>>>> -             fs_info->delalloc_workers && fs_info->flush_workers &&
> >>>>>> +       if (!(fs_info->delalloc_workers && fs_info->flush_workers &&
> >>>>>>                  fs_info->endio_workers && fs_info->endio_meta_workers &&
> >>>>>>                  fs_info->endio_write_workers &&
> >>>>>>                  fs_info->endio_freespace_worker && fs_info->rmw_workers &&
> >>>>>> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> >>>>>>             */
> >>>>>>            btrfs_flush_workqueue(fs_info->delalloc_workers);
> >>>>>>
> >>>>>> -       /*
> >>>>>> -        * 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
> >>>>>> -        * 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.
> >>>>>> -        */
> >>>>>> -       btrfs_flush_workqueue(fs_info->workers);
> >>>>>> -
> >>>>>>            /*
> >>>>>>             * When finishing a compressed write bio we schedule a work queue item
> >>>>>>             * to finish an ordered extent - end_bbio_compressed_write()
> >>>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >>>>>> index 0fed4e0d32d5..7dbd9b8eec11 100644
> >>>>>> --- a/fs/btrfs/file-item.c
> >>>>>> +++ b/fs/btrfs/file-item.c
> >>>>>> @@ -825,7 +825,7 @@ static void csum_one_bio_work(struct work_struct *work)
> >>>>>>     /*
> >>>>>>      * Calculate checksums of the data contained inside a bio.
> >>>>>>      */
> >>>>>> -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;
> >>>>>> @@ -849,10 +849,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);
> >>>>>>            schedule_work(&bbio->csum_work);
> >>>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >>>>>> index 6c678787c770..60a0eb17b3f6 100644
> >>>>>> --- a/fs/btrfs/file-item.h
> >>>>>> +++ b/fs/btrfs/file-item.h
> >>>>>> @@ -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,
> >>>>>> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> >>>>>> index 3eba8438593c..203f7131f737 100644
> >>>>>> --- a/fs/btrfs/fs.h
> >>>>>> +++ b/fs/btrfs/fs.h
> >>>>>> @@ -696,16 +696,6 @@ struct btrfs_fs_info {
> >>>>>>            /* All fs/file tree roots that have delalloc inodes. */
> >>>>>>            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.
> >>>>>> -        */
> >>>>>> -       struct btrfs_workqueue *workers;
> >>>>>>            struct btrfs_workqueue *delalloc_workers;
> >>>>>>            struct btrfs_workqueue *flush_workers;
> >>>>>>            struct workqueue_struct *endio_workers;
> >>>>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> >>>>>> index 464129b1b0d4..54f2da47c567 100644
> >>>>>> --- a/fs/btrfs/super.c
> >>>>>> +++ b/fs/btrfs/super.c
> >>>>>> @@ -1237,7 +1237,6 @@ 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);
> >>>>>>            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);
> >>>>>> --
> >>>>>> 2.55.0
> >>>>>>
> >>>>>
> >>>>
> >>
> >
>

Re: [PATCH] btrfs: move async csum generation out of experimental features

[David Sterba]

On Thu, Sep 03, 2026 at 06:09:41PM +0930, Qu Wenruo wrote:
> Commit dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")
> introduced asynchronous data checksum generation for data writes.
> 
> That feature can improve write performance, and was introduced in v6.19.
> We have not experienced bugs related to that, so it's time to move it out
> of experimental features for end users.

Agreed, we'll have enough time to test it until the code freeze.

Re: [PATCH] btrfs: move async csum generation out of experimental features

[David Sterba]

On Thu, Sep 03, 2026 at 06:09:41PM +0930, Qu Wenruo wrote:
> -static bool should_async_write(struct btrfs_bio *bbio)
> -{
> -	struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
> -	bool auto_csum_mode = true;
> -
> -#ifdef CONFIG_BTRFS_EXPERIMENTAL
> -	/*
> -	 * Write bios will calculate checksum and submit bio at the same time.
> -	 * Unless explicitly required don't offload serial csum calculate and bio
> -	 * submit into a workqueue.
> -	 */
> -	return false;
> -#endif
> -
> -	/* Submit synchronously if the checksum implementation is fast. */
> -	if (auto_csum_mode && test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))

The fast csum implementation is not used anymore, so the
BTRFS_FS_CSUM_IMPL_FAST definition and related code can be deleted.

> -		return false;
> -
> -	/*
> -	 * Try to defer the submission to a workqueue to parallelize the
> -	 * checksum calculation unless the I/O is issued synchronously.
> -	 */
> -	if (op_is_sync(bbio->bio.bi_opf))
> -		return false;
> -
> -	/* Zoned devices require I/O to be submitted in order. */
> -	if ((bbio->bio.bi_opf & REQ_META) && btrfs_is_zoned(fs_info))
> -		return false;
> -
> -	return true;
> -}

Re: [PATCH] btrfs: move async csum generation out of experimental features

[David Sterba]

On Thu, Sep 03, 2026 at 06:09:41PM +0930, Qu Wenruo wrote:
> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>  	 */
>  	btrfs_flush_workqueue(fs_info->delalloc_workers);
>  
> -	/*
> -	 * 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
> -	 * 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.
> -	 */
> -	btrfs_flush_workqueue(fs_info->workers);

In the final fs closing sequence there's always some possible
interaction between the workers, but I don't think we need an explicit
flush anymore. The work offloaded to the system workers will be done
before we get here.

Re: [PATCH] btrfs: move async csum generation out of experimental features

[Qu Wenruo]

在 2026/9/5 02:30, David Sterba 写道:
> On Thu, Sep 03, 2026 at 06:09:41PM +0930, Qu Wenruo wrote:
>> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>>   	 */
>>   	btrfs_flush_workqueue(fs_info->delalloc_workers);
>>   
>> -	/*
>> -	 * 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
>> -	 * 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.
>> -	 */
>> -	btrfs_flush_workqueue(fs_info->workers);
> 
> In the final fs closing sequence there's always some possible
> interaction between the workers, but I don't think we need an explicit
> flush anymore. The work offloaded to the system workers will be done
> before we get here.

In fact, Daniel's commit ("btrfs: use bio::remaining for async 
checksumming synchronization") changed the behavior back, and we may 
need to be more cautious now.

Previously, the csum generation workload will never call endio in its 
context, the only work it really bothers is just to wake the completion.

But now his commit changed to use bio_endio(), which means we can again 
run the workload from csum generation workqueue.

In fact, I do not believe it's the proper timing to move async csum out 
of experimental anymore, after his commit.

I'd prefer more the next merge window at least.

Thanks,
Qu

Re: [PATCH] btrfs: move async csum generation out of experimental features

[David Sterba]

On Sat, Sep 05, 2026 at 07:51:42AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/9/5 02:30, David Sterba 写道:
> > On Thu, Sep 03, 2026 at 06:09:41PM +0930, Qu Wenruo wrote:
> >> @@ -4437,19 +4432,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
> >>   	 */
> >>   	btrfs_flush_workqueue(fs_info->delalloc_workers);
> >>   
> >> -	/*
> >> -	 * 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
> >> -	 * 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.
> >> -	 */
> >> -	btrfs_flush_workqueue(fs_info->workers);
> > 
> > In the final fs closing sequence there's always some possible
> > interaction between the workers, but I don't think we need an explicit
> > flush anymore. The work offloaded to the system workers will be done
> > before we get here.
> 
> In fact, Daniel's commit ("btrfs: use bio::remaining for async 
> checksumming synchronization") changed the behavior back, and we may 
> need to be more cautious now.
> 
> Previously, the csum generation workload will never call endio in its 
> context, the only work it really bothers is just to wake the completion.
> 
> But now his commit changed to use bio_endio(), which means we can again 
> run the workload from csum generation workqueue.

Yeah, this kind of problems I'm expecting but haven't done a deeper
analysis to have concrete examples like that.

> In fact, I do not believe it's the proper timing to move async csum out 
> of experimental anymore, after his commit.
> 
> I'd prefer more the next merge window at least.

We can do that, the main part, otherwise any preparatory or cleanup work
can be done now, as usual.