* [PATCH 0/2] btrfs: async checksumming cleanups
@ 2026-09-02 16:51 Daniel Vacek
2026-09-02 16:51 ` [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() Daniel Vacek
2026-09-02 16:51 ` [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization Daniel Vacek
0 siblings, 2 replies; 8+ messages in thread
From: Daniel Vacek @ 2026-09-02 16:51 UTC (permalink / raw)
To: David Sterba, Chris Mason
Cc: Daniel Vacek, linux-btrfs, linux-kernel, Qu Wenruo
While trying to deal with encryption bounce bios checksumming, a solution
came up which covers all writes (also the regular !encrypted ones). As a
result the code cleans up quite nicely. That's the second patch in this
series. The first one is just a nit. Details in the patches themselves.
Daniel Vacek (2):
btrfs: consume the given iter directly instead of copying in
csum_one_bio()
btrfs: use bio::remaining for async checksumming synchronization
fs/btrfs/bio.c | 4 ----
fs/btrfs/bio.h | 4 ----
fs/btrfs/file-item.c | 20 +++++++++-----------
3 files changed, 9 insertions(+), 19 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() 2026-09-02 16:51 [PATCH 0/2] btrfs: async checksumming cleanups Daniel Vacek @ 2026-09-02 16:51 ` Daniel Vacek 2026-09-02 21:51 ` Qu Wenruo 2026-09-03 4:49 ` Daniel Vacek 2026-09-02 16:51 ` [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization Daniel Vacek 1 sibling, 2 replies; 8+ messages in thread From: Daniel Vacek @ 2026-09-02 16:51 UTC (permalink / raw) To: David Sterba, Chris Mason Cc: Daniel Vacek, linux-btrfs, linux-kernel, Qu Wenruo This is just a small cleanup to avoid copying the iter twice in async case. We already have a copy csum_one_bio() can consume directly. No need to copy again the second time. In the sync case we can copy in the caller (as we did before dd57c78aec39 ("btrfs: introduce btrfs_bio::async_csum")). Signed-off-by: Daniel Vacek <neelx@suse.com> --- fs/btrfs/file-item.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index 581ca5653be9..5a5cffb18922 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -797,18 +797,16 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path, return ret; } -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src) +static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *iter) { struct btrfs_inode *inode = bbio->inode; struct btrfs_fs_info *fs_info = inode->root->fs_info; struct btrfs_ordered_sum *sums = bbio->sums; - struct bvec_iter iter; const u32 blocksize = fs_info->sectorsize; int index = 0; - for (iter = *src; iter.bi_size; bio_advance_iter(&bbio->bio, &iter, blocksize)) { - btrfs_csum_one_bio_block(fs_info, &bbio->bio, &iter, - sums->sums + index); + for (; iter->bi_size; bio_advance_iter(&bbio->bio, iter, blocksize)) { + btrfs_csum_one_bio_block(fs_info, &bbio->bio, iter, sums->sums + index); index += fs_info->csum_size; } @@ -851,12 +849,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async) btrfs_add_ordered_sum(ordered, sums); if (!async) { - csum_one_bio(bbio, &bbio->bio.bi_iter); + struct bvec_iter iter = bio->bi_iter; + + csum_one_bio(bbio, &iter); return 0; } init_completion(&bbio->csum_done); bbio->async_csum = true; - bbio->csum_saved_iter = bbio->bio.bi_iter; + bbio->csum_saved_iter = bio->bi_iter; INIT_WORK(&bbio->csum_work, csum_one_bio_work); schedule_work(&bbio->csum_work); return 0; -- 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() 2026-09-02 16:51 ` [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() Daniel Vacek @ 2026-09-02 21:51 ` Qu Wenruo 2026-09-02 22:47 ` Qu Wenruo 2026-09-03 4:49 ` Daniel Vacek 1 sibling, 1 reply; 8+ messages in thread From: Qu Wenruo @ 2026-09-02 21:51 UTC (permalink / raw) To: Daniel Vacek, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel, Qu Wenruo 在 2026/9/3 02:21, Daniel Vacek 写道: > This is just a small cleanup to avoid copying the iter twice in async case. > We already have a copy csum_one_bio() can consume directly. No need to copy > again the second time. > In the sync case we can copy in the caller (as we did before dd57c78aec39 > ("btrfs: introduce btrfs_bio::async_csum")). > > Signed-off-by: Daniel Vacek <neelx@suse.com> > --- > fs/btrfs/file-item.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c > index 581ca5653be9..5a5cffb18922 100644 > --- a/fs/btrfs/file-item.c > +++ b/fs/btrfs/file-item.c > @@ -797,18 +797,16 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path, > return ret; > } > > -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src) > +static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *iter) Since we're here, what about adding const prefix to iter? > { > struct btrfs_inode *inode = bbio->inode; > struct btrfs_fs_info *fs_info = inode->root->fs_info; > struct btrfs_ordered_sum *sums = bbio->sums; > - struct bvec_iter iter; > const u32 blocksize = fs_info->sectorsize; > int index = 0; > > - for (iter = *src; iter.bi_size; bio_advance_iter(&bbio->bio, &iter, blocksize)) { I'm thinking the opposite way. What about still keeping a local bvec_iter copy, but remove csum_saved_iter completely? Since at bio calculation time, the bio is not yet submitted, so the csum_saved_iter always match bio.bi_iter. I think removing csum_saved_iter would also reduce the size of btrfs_bio. Thanks, Qu > - btrfs_csum_one_bio_block(fs_info, &bbio->bio, &iter, > - sums->sums + index); > + for (; iter->bi_size; bio_advance_iter(&bbio->bio, iter, blocksize)) { > + btrfs_csum_one_bio_block(fs_info, &bbio->bio, iter, sums->sums + index); > > index += fs_info->csum_size; > } > @@ -851,12 +849,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async) > btrfs_add_ordered_sum(ordered, sums); > > if (!async) { > - csum_one_bio(bbio, &bbio->bio.bi_iter); > + struct bvec_iter iter = bio->bi_iter; > + > + csum_one_bio(bbio, &iter); > return 0; > } > init_completion(&bbio->csum_done); > bbio->async_csum = true; > - bbio->csum_saved_iter = bbio->bio.bi_iter; > + bbio->csum_saved_iter = bio->bi_iter; > INIT_WORK(&bbio->csum_work, csum_one_bio_work); > schedule_work(&bbio->csum_work); > return 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() 2026-09-02 21:51 ` Qu Wenruo @ 2026-09-02 22:47 ` Qu Wenruo 0 siblings, 0 replies; 8+ messages in thread From: Qu Wenruo @ 2026-09-02 22:47 UTC (permalink / raw) To: Daniel Vacek, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel, Qu Wenruo 在 2026/9/3 07:21, Qu Wenruo 写道: > > > 在 2026/9/3 02:21, Daniel Vacek 写道: >> This is just a small cleanup to avoid copying the iter twice in async >> case. >> We already have a copy csum_one_bio() can consume directly. No need to >> copy >> again the second time. >> In the sync case we can copy in the caller (as we did before dd57c78aec39 >> ("btrfs: introduce btrfs_bio::async_csum")). >> >> Signed-off-by: Daniel Vacek <neelx@suse.com> >> --- >> fs/btrfs/file-item.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c >> index 581ca5653be9..5a5cffb18922 100644 >> --- a/fs/btrfs/file-item.c >> +++ b/fs/btrfs/file-item.c >> @@ -797,18 +797,16 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root >> *root, struct btrfs_path *path, >> return ret; >> } >> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src) >> +static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *iter) > > Since we're here, what about adding const prefix to iter? My bad, we're advancing the iter, so it can not be const. > >> { >> struct btrfs_inode *inode = bbio->inode; >> struct btrfs_fs_info *fs_info = inode->root->fs_info; >> struct btrfs_ordered_sum *sums = bbio->sums; >> - struct bvec_iter iter; >> const u32 blocksize = fs_info->sectorsize; >> int index = 0; >> - for (iter = *src; iter.bi_size; bio_advance_iter(&bbio->bio, >> &iter, blocksize)) { > > I'm thinking the opposite way. > > What about still keeping a local bvec_iter copy, but remove > csum_saved_iter completely? > > Since at bio calculation time, the bio is not yet submitted, so the > csum_saved_iter always match bio.bi_iter. > > I think removing csum_saved_iter would also reduce the size of btrfs_bio. My bad, this won't work. During csum calculation the bio can be submitted already, thus its bio::bi_iter is no longer reliable. And since it's a data write, we do not have bbio::saved_iter to utilize either. Now it looks good to me. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > > Thanks, > Qu > >> - btrfs_csum_one_bio_block(fs_info, &bbio->bio, &iter, >> - sums->sums + index); >> + for (; iter->bi_size; bio_advance_iter(&bbio->bio, iter, >> blocksize)) { >> + btrfs_csum_one_bio_block(fs_info, &bbio->bio, iter, sums- >> >sums + index); >> index += fs_info->csum_size; >> } >> @@ -851,12 +849,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, >> bool async) >> btrfs_add_ordered_sum(ordered, sums); >> if (!async) { >> - csum_one_bio(bbio, &bbio->bio.bi_iter); >> + struct bvec_iter iter = bio->bi_iter; >> + >> + csum_one_bio(bbio, &iter); >> return 0; >> } >> init_completion(&bbio->csum_done); >> bbio->async_csum = true; >> - bbio->csum_saved_iter = bbio->bio.bi_iter; >> + bbio->csum_saved_iter = bio->bi_iter; >> INIT_WORK(&bbio->csum_work, csum_one_bio_work); >> schedule_work(&bbio->csum_work); >> return 0; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() 2026-09-02 16:51 ` [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() Daniel Vacek 2026-09-02 21:51 ` Qu Wenruo @ 2026-09-03 4:49 ` Daniel Vacek 1 sibling, 0 replies; 8+ messages in thread From: Daniel Vacek @ 2026-09-03 4:49 UTC (permalink / raw) To: David Sterba, Chris Mason; +Cc: linux-btrfs, linux-kernel, Qu Wenruo On Wed, 2 Sept 2026 at 18:51, Daniel Vacek <neelx@suse.com> wrote: > This is just a small cleanup to avoid copying the iter twice in async case. > We already have a copy csum_one_bio() can consume directly. No need to copy > again the second time. > In the sync case we can copy in the caller (as we did before dd57c78aec39 > ("btrfs: introduce btrfs_bio::async_csum")). > > Signed-off-by: Daniel Vacek <neelx@suse.com> > --- > fs/btrfs/file-item.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c > index 581ca5653be9..5a5cffb18922 100644 > --- a/fs/btrfs/file-item.c > +++ b/fs/btrfs/file-item.c > @@ -797,18 +797,16 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path, > return ret; > } > > -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src) > +static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *iter) > { > struct btrfs_inode *inode = bbio->inode; > struct btrfs_fs_info *fs_info = inode->root->fs_info; > struct btrfs_ordered_sum *sums = bbio->sums; > - struct bvec_iter iter; > const u32 blocksize = fs_info->sectorsize; > int index = 0; > > - for (iter = *src; iter.bi_size; bio_advance_iter(&bbio->bio, &iter, blocksize)) { > - btrfs_csum_one_bio_block(fs_info, &bbio->bio, &iter, > - sums->sums + index); > + for (; iter->bi_size; bio_advance_iter(&bbio->bio, iter, blocksize)) { > + btrfs_csum_one_bio_block(fs_info, &bbio->bio, iter, sums->sums + index); > > index += fs_info->csum_size; > } > @@ -851,12 +849,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async) > btrfs_add_ordered_sum(ordered, sums); > > if (!async) { > - csum_one_bio(bbio, &bbio->bio.bi_iter); > + struct bvec_iter iter = bio->bi_iter; > + > + csum_one_bio(bbio, &iter); > return 0; > } > init_completion(&bbio->csum_done); > bbio->async_csum = true; > - bbio->csum_saved_iter = bbio->bio.bi_iter; > + bbio->csum_saved_iter = bio->bi_iter; Actually we can use this one unconditionally in both cases and save the stack space. And unifying both cases will allow us getting rid of the paramert. I'll send a v2 shortly. --nX > INIT_WORK(&bbio->csum_work, csum_one_bio_work); > schedule_work(&bbio->csum_work); > return 0; > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization 2026-09-02 16:51 [PATCH 0/2] btrfs: async checksumming cleanups Daniel Vacek 2026-09-02 16:51 ` [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() Daniel Vacek @ 2026-09-02 16:51 ` Daniel Vacek 2026-09-02 22:02 ` Qu Wenruo 1 sibling, 1 reply; 8+ messages in thread From: Daniel Vacek @ 2026-09-02 16:51 UTC (permalink / raw) To: David Sterba, Chris Mason Cc: Daniel Vacek, linux-btrfs, linux-kernel, Qu Wenruo We can use bio::remaining counter to sync the offloaded checksuming. As a result we can slim down the btrfs_bio structure by 24 bytes and simplify the code a bit. $ pahole | diff - /* size: 328, cachelines: 6, members: 15 */ + /* size: 304, cachelines: 5, members: 14 */ Moreover this will allow us enabling async checksumming with encryption where we need to checksum the bounce bio instead of our regular one embedded in btrfs_bio. And so we need to extend it's lifetime. This is the preffered way to do so. Signed-off-by: Daniel Vacek <neelx@suse.com> --- fs/btrfs/bio.c | 4 ---- fs/btrfs/bio.h | 4 ---- fs/btrfs/file-item.c | 6 ++---- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c index 19b4855969f5..771b7d598aee 100644 --- a/fs/btrfs/bio.c +++ b/fs/btrfs/bio.c @@ -103,7 +103,6 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info, bbio->can_use_append = orig_bbio->can_use_append; bbio->is_scrub = orig_bbio->is_scrub; bbio->is_remap = orig_bbio->is_remap; - bbio->async_csum = orig_bbio->async_csum; atomic_inc(&orig_bbio->pending_ios); return bbio; @@ -114,9 +113,6 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status) /* Make sure we're already in task context. */ ASSERT(in_task()); - if (bbio->async_csum) - wait_for_completion(&bbio->csum_done); - bbio->bio.bi_status = status; if (bbio->bio.bi_pool == &btrfs_clone_bioset) { struct btrfs_bio *orig_bbio = bbio->private; diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h index b7bd377a0162..bbf362b8668b 100644 --- a/fs/btrfs/bio.h +++ b/fs/btrfs/bio.h @@ -58,7 +58,6 @@ struct btrfs_bio { struct btrfs_ordered_extent *ordered; struct btrfs_ordered_sum *sums; struct work_struct csum_work; - struct completion csum_done; struct bvec_iter csum_saved_iter; u64 orig_physical; u64 orig_logical; @@ -93,9 +92,6 @@ struct btrfs_bio { /* Whether the bio is coming from copy_remapped_data_io(). */ bool is_remap:1; - /* Whether the csum generation for data write is async. */ - bool async_csum:1; - /* Whether the bio is written using zone append. */ bool can_use_append:1; diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index 5a5cffb18922..50dcd448d936 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -817,9 +817,8 @@ static void csum_one_bio_work(struct work_struct *work) struct btrfs_bio *bbio = container_of(work, struct btrfs_bio, csum_work); ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE); - ASSERT(bbio->async_csum == true); csum_one_bio(bbio, &bbio->csum_saved_iter); - complete(&bbio->csum_done); + bio_endio(&bbio->bio); } /* @@ -854,8 +853,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async) csum_one_bio(bbio, &iter); return 0; } - init_completion(&bbio->csum_done); - bbio->async_csum = true; + bio_inc_remaining(bio); bbio->csum_saved_iter = bio->bi_iter; INIT_WORK(&bbio->csum_work, csum_one_bio_work); schedule_work(&bbio->csum_work); -- 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization 2026-09-02 16:51 ` [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization Daniel Vacek @ 2026-09-02 22:02 ` Qu Wenruo 2026-09-02 22:20 ` Qu Wenruo 0 siblings, 1 reply; 8+ messages in thread From: Qu Wenruo @ 2026-09-02 22:02 UTC (permalink / raw) To: Daniel Vacek, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel, Qu Wenruo 在 2026/9/3 02:21, Daniel Vacek 写道: > We can use bio::remaining counter to sync the offloaded checksuming. > As a result we can slim down the btrfs_bio structure by 24 bytes > and simplify the code a bit. > > $ pahole | diff > - /* size: 328, cachelines: 6, members: 15 */ > + /* size: 304, cachelines: 5, members: 14 */ > > Moreover this will allow us enabling async checksumming with encryption > where we need to checksum the bounce bio instead of our regular one > embedded in btrfs_bio. And so we need to extend it's lifetime. This is > the preffered way to do so. > > Signed-off-by: Daniel Vacek <neelx@suse.com> > --- > fs/btrfs/bio.c | 4 ---- > fs/btrfs/bio.h | 4 ---- > fs/btrfs/file-item.c | 6 ++---- > 3 files changed, 2 insertions(+), 12 deletions(-) > > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c > index 19b4855969f5..771b7d598aee 100644 > --- a/fs/btrfs/bio.c > +++ b/fs/btrfs/bio.c > @@ -103,7 +103,6 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info, > bbio->can_use_append = orig_bbio->can_use_append; > bbio->is_scrub = orig_bbio->is_scrub; > bbio->is_remap = orig_bbio->is_remap; > - bbio->async_csum = orig_bbio->async_csum; > > atomic_inc(&orig_bbio->pending_ios); > return bbio; > @@ -114,9 +113,6 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status) > /* Make sure we're already in task context. */ > ASSERT(in_task()); > > - if (bbio->async_csum) > - wait_for_completion(&bbio->csum_done); > - But what if there is still a running csum calculation at this stage? Without waiting for the completion, we are going to get incorrect csums. > bbio->bio.bi_status = status; > if (bbio->bio.bi_pool == &btrfs_clone_bioset) { > struct btrfs_bio *orig_bbio = bbio->private; > diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h > index b7bd377a0162..bbf362b8668b 100644 > --- a/fs/btrfs/bio.h > +++ b/fs/btrfs/bio.h > @@ -58,7 +58,6 @@ struct btrfs_bio { > struct btrfs_ordered_extent *ordered; > struct btrfs_ordered_sum *sums; > struct work_struct csum_work; > - struct completion csum_done; > struct bvec_iter csum_saved_iter; > u64 orig_physical; > u64 orig_logical; > @@ -93,9 +92,6 @@ struct btrfs_bio { > /* Whether the bio is coming from copy_remapped_data_io(). */ > bool is_remap:1; > > - /* Whether the csum generation for data write is async. */ > - bool async_csum:1; > - > /* Whether the bio is written using zone append. */ > bool can_use_append:1; > > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c > index 5a5cffb18922..50dcd448d936 100644 > --- a/fs/btrfs/file-item.c > +++ b/fs/btrfs/file-item.c > @@ -817,9 +817,8 @@ static void csum_one_bio_work(struct work_struct *work) > struct btrfs_bio *bbio = container_of(work, struct btrfs_bio, csum_work); > > ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE); > - ASSERT(bbio->async_csum == true); > csum_one_bio(bbio, &bbio->csum_saved_iter); > - complete(&bbio->csum_done); > + bio_endio(&bbio->bio); > } > > /* > @@ -854,8 +853,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async) > csum_one_bio(bbio, &iter); > return 0; > } > - init_completion(&bbio->csum_done); > - bbio->async_csum = true; > + bio_inc_remaining(bio); > bbio->csum_saved_iter = bio->bi_iter; > INIT_WORK(&bbio->csum_work, csum_one_bio_work); > schedule_work(&bbio->csum_work); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization 2026-09-02 22:02 ` Qu Wenruo @ 2026-09-02 22:20 ` Qu Wenruo 0 siblings, 0 replies; 8+ messages in thread From: Qu Wenruo @ 2026-09-02 22:20 UTC (permalink / raw) To: Qu Wenruo, Daniel Vacek, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel 在 2026/9/3 07:32, Qu Wenruo 写道: > > > 在 2026/9/3 02:21, Daniel Vacek 写道: >> We can use bio::remaining counter to sync the offloaded checksuming. >> As a result we can slim down the btrfs_bio structure by 24 bytes >> and simplify the code a bit. >> >> $ pahole | diff >> - /* size: 328, cachelines: 6, members: 15 */ >> + /* size: 304, cachelines: 5, members: 14 */ >> >> Moreover this will allow us enabling async checksumming with encryption >> where we need to checksum the bounce bio instead of our regular one >> embedded in btrfs_bio. And so we need to extend it's lifetime. This is >> the preffered way to do so. >> >> Signed-off-by: Daniel Vacek <neelx@suse.com> >> --- >> fs/btrfs/bio.c | 4 ---- >> fs/btrfs/bio.h | 4 ---- >> fs/btrfs/file-item.c | 6 ++---- >> 3 files changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c >> index 19b4855969f5..771b7d598aee 100644 >> --- a/fs/btrfs/bio.c >> +++ b/fs/btrfs/bio.c >> @@ -103,7 +103,6 @@ static struct btrfs_bio *btrfs_split_bio(struct >> btrfs_fs_info *fs_info, >> bbio->can_use_append = orig_bbio->can_use_append; >> bbio->is_scrub = orig_bbio->is_scrub; >> bbio->is_remap = orig_bbio->is_remap; >> - bbio->async_csum = orig_bbio->async_csum; >> atomic_inc(&orig_bbio->pending_ios); >> return bbio; >> @@ -114,9 +113,6 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, >> blk_status_t status) >> /* Make sure we're already in task context. */ >> ASSERT(in_task()); >> - if (bbio->async_csum) >> - wait_for_completion(&bbio->csum_done); >> - > > But what if there is still a running csum calculation at this stage? > > Without waiting for the completion, we are going to get incorrect csums. Forget this. We no longer need to wait because we won't even reach the bio->bi_end_io() until all remaining works (including the csum calculation and mirrored writes) finished. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > >> bbio->bio.bi_status = status; >> if (bbio->bio.bi_pool == &btrfs_clone_bioset) { >> struct btrfs_bio *orig_bbio = bbio->private; >> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h >> index b7bd377a0162..bbf362b8668b 100644 >> --- a/fs/btrfs/bio.h >> +++ b/fs/btrfs/bio.h >> @@ -58,7 +58,6 @@ struct btrfs_bio { >> struct btrfs_ordered_extent *ordered; >> struct btrfs_ordered_sum *sums; >> struct work_struct csum_work; >> - struct completion csum_done; >> struct bvec_iter csum_saved_iter; >> u64 orig_physical; >> u64 orig_logical; >> @@ -93,9 +92,6 @@ struct btrfs_bio { >> /* Whether the bio is coming from copy_remapped_data_io(). */ >> bool is_remap:1; >> - /* Whether the csum generation for data write is async. */ >> - bool async_csum:1; >> - >> /* Whether the bio is written using zone append. */ >> bool can_use_append:1; >> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c >> index 5a5cffb18922..50dcd448d936 100644 >> --- a/fs/btrfs/file-item.c >> +++ b/fs/btrfs/file-item.c >> @@ -817,9 +817,8 @@ static void csum_one_bio_work(struct work_struct >> *work) >> struct btrfs_bio *bbio = container_of(work, struct btrfs_bio, >> csum_work); >> ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE); >> - ASSERT(bbio->async_csum == true); >> csum_one_bio(bbio, &bbio->csum_saved_iter); >> - complete(&bbio->csum_done); >> + bio_endio(&bbio->bio); >> } >> /* >> @@ -854,8 +853,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, >> bool async) >> csum_one_bio(bbio, &iter); >> return 0; >> } >> - init_completion(&bbio->csum_done); >> - bbio->async_csum = true; >> + bio_inc_remaining(bio); >> bbio->csum_saved_iter = bio->bi_iter; >> INIT_WORK(&bbio->csum_work, csum_one_bio_work); >> schedule_work(&bbio->csum_work); > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-03 4:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-02 16:51 [PATCH 0/2] btrfs: async checksumming cleanups Daniel Vacek 2026-09-02 16:51 ` [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio() Daniel Vacek 2026-09-02 21:51 ` Qu Wenruo 2026-09-02 22:47 ` Qu Wenruo 2026-09-03 4:49 ` Daniel Vacek 2026-09-02 16:51 ` [PATCH 2/2] btrfs: use bio::remaining for async checksumming synchronization Daniel Vacek 2026-09-02 22:02 ` Qu Wenruo 2026-09-02 22:20 ` Qu Wenruo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®