mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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®