* [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
* [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 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 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
* 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
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®