From: Josef Bacik <josef@toxicpanda.com>

We only ever needed the bbio in btrfs_csum_one_bio, since that has the
bio embedded in it.  However with encryption we'll have a different bio
with the encrypted data in it, and the original bbio.  Update
btrfs_csum_one_bio to take the bio we're going to csum as an argument,
which will allow us to csum the encrypted bio and stuff the csums into
the corresponding bbio to be used later when the IO completes.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Daniel Vacek <neelx@suse.com>
---
Compared to v5 this needed to adapt to recent async csum changes.
---
 fs/btrfs/bio.c       |  4 ++--
 fs/btrfs/bio.h       |  1 +
 fs/btrfs/file-item.c | 17 ++++++++---------
 fs/btrfs/file-item.h |  2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index a73652b8724a..a69174b2b6b6 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -542,9 +542,9 @@ 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);
+	return btrfs_csum_one_bio(bbio, &bbio->bio, true);
 #else
-	return btrfs_csum_one_bio(bbio, false);
+	return btrfs_csum_one_bio(bbio, &bbio->bio, false);
 #endif
 }
 
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index deaeea3becf4..c5a6c66d51a0 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -58,6 +58,7 @@ struct btrfs_bio {
 			struct btrfs_ordered_sum *sums;
 			struct work_struct csum_work;
 			struct completion csum_done;
+			struct bio *csum_bio;
 			struct bvec_iter csum_saved_iter;
 			u64 orig_physical;
 		};
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 72be3ede0edf..474949074da8 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
 {
 	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
-	struct bio *bio = &bbio->bio;
 	struct btrfs_ordered_sum *sums = bbio->sums;
-	struct bvec_iter iter = *src;
 	phys_addr_t paddr;
 	const u32 blocksize = fs_info->sectorsize;
 	int index = 0;
 
 	shash->tfm = fs_info->csum_shash;
 
-	btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
+	btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
 		btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
 		index += fs_info->csum_size;
 	}
@@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
 
 	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
 	ASSERT(bbio->async_csum == true);
-	csum_one_bio(bbio, &bbio->csum_saved_iter);
+	csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
 	complete(&bbio->csum_done);
 }
 
 /*
  * 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 bio *bio, bool async)
 {
 	struct btrfs_ordered_extent *ordered = bbio->ordered;
 	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
-	struct bio *bio = &bbio->bio;
 	struct btrfs_ordered_sum *sums;
 	unsigned nofs_flag;
 
@@ -822,12 +819,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, bio, &iter);
 		return 0;
 	}
 	init_completion(&bbio->csum_done);
 	bbio->async_csum = true;
-	bbio->csum_saved_iter = bbio->bio.bi_iter;
+	bbio->csum_bio = bio;
+	bbio->csum_saved_iter = bio->bi_iter;
 	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
 	schedule_work(&bbio->csum_work);
 	return 0;
diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
 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,
-- 
2.51.0

在 2025/11/13 06:06, Daniel Vacek 写道:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> We only ever needed the bbio in btrfs_csum_one_bio, since that has the
> bio embedded in it.  However with encryption we'll have a different bio
> with the encrypted data in it, and the original bbio.  Update
> btrfs_csum_one_bio to take the bio we're going to csum as an argument,
> which will allow us to csum the encrypted bio and stuff the csums into
> the corresponding bbio to be used later when the IO completes.

I'm wondering why we can not do it in a layered bio way.

E.g. on device-mapper based solutions, the upper layer send out the bio 
containing the plaintext data.
Then the dm-crypto send out a new bio, containing the encrypted data.

The storage layer doesn't need to bother the plaintext bio at all, they 
just write the encrypted one to disk.

And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.


So why we can not just create a new bio for the final csum caculation, 
just like compression?

Thanks,
Qu

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Daniel Vacek <neelx@suse.com>
> ---
> Compared to v5 this needed to adapt to recent async csum changes.
> ---
>   fs/btrfs/bio.c       |  4 ++--
>   fs/btrfs/bio.h       |  1 +
>   fs/btrfs/file-item.c | 17 ++++++++---------
>   fs/btrfs/file-item.h |  2 +-
>   4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index a73652b8724a..a69174b2b6b6 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -542,9 +542,9 @@ 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);
> +	return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>   #else
> -	return btrfs_csum_one_bio(bbio, false);
> +	return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>   #endif
>   }
>   
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index deaeea3becf4..c5a6c66d51a0 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -58,6 +58,7 @@ struct btrfs_bio {
>   			struct btrfs_ordered_sum *sums;
>   			struct work_struct csum_work;
>   			struct completion csum_done;
> +			struct bio *csum_bio;
>   			struct bvec_iter csum_saved_iter;
>   			u64 orig_physical;
>   		};
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 72be3ede0edf..474949074da8 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
>   {
>   	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> -	struct bio *bio = &bbio->bio;
>   	struct btrfs_ordered_sum *sums = bbio->sums;
> -	struct bvec_iter iter = *src;
>   	phys_addr_t paddr;
>   	const u32 blocksize = fs_info->sectorsize;
>   	int index = 0;
>   
>   	shash->tfm = fs_info->csum_shash;
>   
> -	btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> +	btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>   		btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>   		index += fs_info->csum_size;
>   	}
> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>   
>   	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>   	ASSERT(bbio->async_csum == true);
> -	csum_one_bio(bbio, &bbio->csum_saved_iter);
> +	csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>   	complete(&bbio->csum_done);
>   }
>   
>   /*
>    * 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 bio *bio, bool async)
>   {
>   	struct btrfs_ordered_extent *ordered = bbio->ordered;
>   	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
> -	struct bio *bio = &bbio->bio;
>   	struct btrfs_ordered_sum *sums;
>   	unsigned nofs_flag;
>   
> @@ -822,12 +819,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, bio, &iter);
>   		return 0;
>   	}
>   	init_completion(&bbio->csum_done);
>   	bbio->async_csum = true;
> -	bbio->csum_saved_iter = bbio->bio.bi_iter;
> +	bbio->csum_bio = bio;
> +	bbio->csum_saved_iter = bio->bi_iter;
>   	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>   	schedule_work(&bbio->csum_work);
>   	return 0;
> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
>   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,

On Wed, 12 Nov 2025 at 22:02, Qu Wenruo <wqu@suse.com> wrote:
> 在 2025/11/13 06:06, Daniel Vacek 写道:
> > From: Josef Bacik <josef@toxicpanda.com>
> >
> > We only ever needed the bbio in btrfs_csum_one_bio, since that has the
> > bio embedded in it.  However with encryption we'll have a different bio
> > with the encrypted data in it, and the original bbio.  Update
> > btrfs_csum_one_bio to take the bio we're going to csum as an argument,
> > which will allow us to csum the encrypted bio and stuff the csums into
> > the corresponding bbio to be used later when the IO completes.
>
> I'm wondering why we can not do it in a layered bio way.
>
> E.g. on device-mapper based solutions, the upper layer send out the bio
> containing the plaintext data.
> Then the dm-crypto send out a new bio, containing the encrypted data.

This is similar but fscrypt works on FS level. It gets the original
plaintext bio (the btrfs_bio::bio one) and gives us a callback with
the encrypted bio to calculate the checksum of the encrypted data. No
plaintext bio goes to the storage layer.

--nX

> The storage layer doesn't need to bother the plaintext bio at all, they
> just write the encrypted one to disk.
>
> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>
>
> So why we can not just create a new bio for the final csum caculation,
> just like compression?
>
> Thanks,
> Qu
>
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Daniel Vacek <neelx@suse.com>
> > ---
> > Compared to v5 this needed to adapt to recent async csum changes.
> > ---
> >   fs/btrfs/bio.c       |  4 ++--
> >   fs/btrfs/bio.h       |  1 +
> >   fs/btrfs/file-item.c | 17 ++++++++---------
> >   fs/btrfs/file-item.h |  2 +-
> >   4 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > index a73652b8724a..a69174b2b6b6 100644
> > --- a/fs/btrfs/bio.c
> > +++ b/fs/btrfs/bio.c
> > @@ -542,9 +542,9 @@ 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);
> > +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
> >   #else
> > -     return btrfs_csum_one_bio(bbio, false);
> > +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
> >   #endif
> >   }
> >
> > diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> > index deaeea3becf4..c5a6c66d51a0 100644
> > --- a/fs/btrfs/bio.h
> > +++ b/fs/btrfs/bio.h
> > @@ -58,6 +58,7 @@ struct btrfs_bio {
> >                       struct btrfs_ordered_sum *sums;
> >                       struct work_struct csum_work;
> >                       struct completion csum_done;
> > +                     struct bio *csum_bio;
> >                       struct bvec_iter csum_saved_iter;
> >                       u64 orig_physical;
> >               };
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 72be3ede0edf..474949074da8 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
> >   {
> >       struct btrfs_inode *inode = bbio->inode;
> >       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >       SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> > -     struct bio *bio = &bbio->bio;
> >       struct btrfs_ordered_sum *sums = bbio->sums;
> > -     struct bvec_iter iter = *src;
> >       phys_addr_t paddr;
> >       const u32 blocksize = fs_info->sectorsize;
> >       int index = 0;
> >
> >       shash->tfm = fs_info->csum_shash;
> >
> > -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> > +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
> >               btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
> >               index += fs_info->csum_size;
> >       }
> > @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
> >
> >       ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >       ASSERT(bbio->async_csum == true);
> > -     csum_one_bio(bbio, &bbio->csum_saved_iter);
> > +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
> >       complete(&bbio->csum_done);
> >   }
> >
> >   /*
> >    * 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 bio *bio, bool async)
> >   {
> >       struct btrfs_ordered_extent *ordered = bbio->ordered;
> >       struct btrfs_inode *inode = bbio->inode;
> >       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> > -     struct bio *bio = &bbio->bio;
> >       struct btrfs_ordered_sum *sums;
> >       unsigned nofs_flag;
> >
> > @@ -822,12 +819,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, bio, &iter);
> >               return 0;
> >       }
> >       init_completion(&bbio->csum_done);
> >       bbio->async_csum = true;
> > -     bbio->csum_saved_iter = bbio->bio.bi_iter;
> > +     bbio->csum_bio = bio;
> > +     bbio->csum_saved_iter = bio->bi_iter;
> >       INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> >       schedule_work(&bbio->csum_work);
> >       return 0;
> > diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> > index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
> >   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,
>

在 2025/11/14 05:37, Daniel Vacek 写道:
> On Wed, 12 Nov 2025 at 22:02, Qu Wenruo <wqu@suse.com> wrote:
>> 在 2025/11/13 06:06, Daniel Vacek 写道:
>>> From: Josef Bacik <josef@toxicpanda.com>
>>>
>>> We only ever needed the bbio in btrfs_csum_one_bio, since that has the
>>> bio embedded in it.  However with encryption we'll have a different bio
>>> with the encrypted data in it, and the original bbio.  Update
>>> btrfs_csum_one_bio to take the bio we're going to csum as an argument,
>>> which will allow us to csum the encrypted bio and stuff the csums into
>>> the corresponding bbio to be used later when the IO completes.
>>
>> I'm wondering why we can not do it in a layered bio way.
>>
>> E.g. on device-mapper based solutions, the upper layer send out the bio
>> containing the plaintext data.
>> Then the dm-crypto send out a new bio, containing the encrypted data.
> 
> This is similar but fscrypt works on FS level. It gets the original
> plaintext bio (the btrfs_bio::bio one) and gives us a callback with
> the encrypted bio to calculate the checksum of the encrypted data. No
> plaintext bio goes to the storage layer.

Then why put the original bio pointer into the super generic btrfs_bio?

I thought it's more common to put the original plaintext into the 
encryption specific structure, like what we did for compression.

Thanks,
Qu

> 
> --nX
> 
>> The storage layer doesn't need to bother the plaintext bio at all, they
>> just write the encrypted one to disk.
>>
>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>>
>>
>> So why we can not just create a new bio for the final csum caculation,
>> just like compression?
>>
>> Thanks,
>> Qu
>>
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
>>> ---
>>> Compared to v5 this needed to adapt to recent async csum changes.
>>> ---
>>>    fs/btrfs/bio.c       |  4 ++--
>>>    fs/btrfs/bio.h       |  1 +
>>>    fs/btrfs/file-item.c | 17 ++++++++---------
>>>    fs/btrfs/file-item.h |  2 +-
>>>    4 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>> index a73652b8724a..a69174b2b6b6 100644
>>> --- a/fs/btrfs/bio.c
>>> +++ b/fs/btrfs/bio.c
>>> @@ -542,9 +542,9 @@ 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);
>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>>>    #else
>>> -     return btrfs_csum_one_bio(bbio, false);
>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>>>    #endif
>>>    }
>>>
>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>>> index deaeea3becf4..c5a6c66d51a0 100644
>>> --- a/fs/btrfs/bio.h
>>> +++ b/fs/btrfs/bio.h
>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
>>>                        struct btrfs_ordered_sum *sums;
>>>                        struct work_struct csum_work;
>>>                        struct completion csum_done;
>>> +                     struct bio *csum_bio;
>>>                        struct bvec_iter csum_saved_iter;
>>>                        u64 orig_physical;
>>>                };
>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>> index 72be3ede0edf..474949074da8 100644
>>> --- a/fs/btrfs/file-item.c
>>> +++ b/fs/btrfs/file-item.c
>>> @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
>>>    {
>>>        struct btrfs_inode *inode = bbio->inode;
>>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>        SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>>> -     struct bio *bio = &bbio->bio;
>>>        struct btrfs_ordered_sum *sums = bbio->sums;
>>> -     struct bvec_iter iter = *src;
>>>        phys_addr_t paddr;
>>>        const u32 blocksize = fs_info->sectorsize;
>>>        int index = 0;
>>>
>>>        shash->tfm = fs_info->csum_shash;
>>>
>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>>>                btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>>>                index += fs_info->csum_size;
>>>        }
>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>>>
>>>        ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>>>        ASSERT(bbio->async_csum == true);
>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>>>        complete(&bbio->csum_done);
>>>    }
>>>
>>>    /*
>>>     * 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 bio *bio, bool async)
>>>    {
>>>        struct btrfs_ordered_extent *ordered = bbio->ordered;
>>>        struct btrfs_inode *inode = bbio->inode;
>>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>> -     struct bio *bio = &bbio->bio;
>>>        struct btrfs_ordered_sum *sums;
>>>        unsigned nofs_flag;
>>>
>>> @@ -822,12 +819,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, bio, &iter);
>>>                return 0;
>>>        }
>>>        init_completion(&bbio->csum_done);
>>>        bbio->async_csum = true;
>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
>>> +     bbio->csum_bio = bio;
>>> +     bbio->csum_saved_iter = bio->bi_iter;
>>>        INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>>>        schedule_work(&bbio->csum_work);
>>>        return 0;
>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>> index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
>>>    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,
>>

On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@suse.com> wrote:
> 在 2025/11/14 05:37, Daniel Vacek 写道:
> > On Wed, 12 Nov 2025 at 22:02, Qu Wenruo <wqu@suse.com> wrote:
> >> 在 2025/11/13 06:06, Daniel Vacek 写道:
> >>> From: Josef Bacik <josef@toxicpanda.com>
> >>>
> >>> We only ever needed the bbio in btrfs_csum_one_bio, since that has the
> >>> bio embedded in it.  However with encryption we'll have a different bio
> >>> with the encrypted data in it, and the original bbio.  Update
> >>> btrfs_csum_one_bio to take the bio we're going to csum as an argument,
> >>> which will allow us to csum the encrypted bio and stuff the csums into
> >>> the corresponding bbio to be used later when the IO completes.
> >>
> >> I'm wondering why we can not do it in a layered bio way.
> >>
> >> E.g. on device-mapper based solutions, the upper layer send out the bio
> >> containing the plaintext data.
> >> Then the dm-crypto send out a new bio, containing the encrypted data.
> >
> > This is similar but fscrypt works on FS level. It gets the original
> > plaintext bio (the btrfs_bio::bio one) and gives us a callback with
> > the encrypted bio to calculate the checksum of the encrypted data. No
> > plaintext bio goes to the storage layer.
>
> Then why put the original bio pointer into the super generic btrfs_bio?

When encryption is enabled, it's not going to be the original bio but
rather the encrypted one.

But giving it another thought and checking the related fscrypt code,
the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
and freed in blk_crypto_fallback_encrypt_endio() before calling
bio_endio() on our original plaintext bio.
This means we have no control over the bounce bio lifetime and we
cannot store the pointer and use it asynchronously. We'll need to
always compute the checksum synchronously for encrypted bios. In that
case we don't need to store it in btrfs_bio::csum_bio at all. For the
regular (unencrypted) case we can keep using the &bbio->bio.

I'll drop the csum_bio for there is no use really.

--nX

> I thought it's more common to put the original plaintext into the
> encryption specific structure, like what we did for compression.
>
> Thanks,
> Qu
>
> >
> > --nX
> >
> >> The storage layer doesn't need to bother the plaintext bio at all, they
> >> just write the encrypted one to disk.
> >>
> >> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
> >>
> >>
> >> So why we can not just create a new bio for the final csum caculation,
> >> just like compression?
> >>
> >> Thanks,
> >> Qu
> >>
> >>>
> >>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >>> Signed-off-by: Daniel Vacek <neelx@suse.com>
> >>> ---
> >>> Compared to v5 this needed to adapt to recent async csum changes.
> >>> ---
> >>>    fs/btrfs/bio.c       |  4 ++--
> >>>    fs/btrfs/bio.h       |  1 +
> >>>    fs/btrfs/file-item.c | 17 ++++++++---------
> >>>    fs/btrfs/file-item.h |  2 +-
> >>>    4 files changed, 12 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >>> index a73652b8724a..a69174b2b6b6 100644
> >>> --- a/fs/btrfs/bio.c
> >>> +++ b/fs/btrfs/bio.c
> >>> @@ -542,9 +542,9 @@ 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);
> >>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
> >>>    #else
> >>> -     return btrfs_csum_one_bio(bbio, false);
> >>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
> >>>    #endif
> >>>    }
> >>>
> >>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> >>> index deaeea3becf4..c5a6c66d51a0 100644
> >>> --- a/fs/btrfs/bio.h
> >>> +++ b/fs/btrfs/bio.h
> >>> @@ -58,6 +58,7 @@ struct btrfs_bio {
> >>>                        struct btrfs_ordered_sum *sums;
> >>>                        struct work_struct csum_work;
> >>>                        struct completion csum_done;
> >>> +                     struct bio *csum_bio;
> >>>                        struct bvec_iter csum_saved_iter;
> >>>                        u64 orig_physical;
> >>>                };
> >>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >>> index 72be3ede0edf..474949074da8 100644
> >>> --- a/fs/btrfs/file-item.c
> >>> +++ b/fs/btrfs/file-item.c
> >>> @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
> >>>    {
> >>>        struct btrfs_inode *inode = bbio->inode;
> >>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>>        SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> >>> -     struct bio *bio = &bbio->bio;
> >>>        struct btrfs_ordered_sum *sums = bbio->sums;
> >>> -     struct bvec_iter iter = *src;
> >>>        phys_addr_t paddr;
> >>>        const u32 blocksize = fs_info->sectorsize;
> >>>        int index = 0;
> >>>
> >>>        shash->tfm = fs_info->csum_shash;
> >>>
> >>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> >>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
> >>>                btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
> >>>                index += fs_info->csum_size;
> >>>        }
> >>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
> >>>
> >>>        ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >>>        ASSERT(bbio->async_csum == true);
> >>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
> >>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
> >>>        complete(&bbio->csum_done);
> >>>    }
> >>>
> >>>    /*
> >>>     * 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 bio *bio, bool async)
> >>>    {
> >>>        struct btrfs_ordered_extent *ordered = bbio->ordered;
> >>>        struct btrfs_inode *inode = bbio->inode;
> >>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>> -     struct bio *bio = &bbio->bio;
> >>>        struct btrfs_ordered_sum *sums;
> >>>        unsigned nofs_flag;
> >>>
> >>> @@ -822,12 +819,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, bio, &iter);
> >>>                return 0;
> >>>        }
> >>>        init_completion(&bbio->csum_done);
> >>>        bbio->async_csum = true;
> >>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
> >>> +     bbio->csum_bio = bio;
> >>> +     bbio->csum_saved_iter = bio->bi_iter;
> >>>        INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> >>>        schedule_work(&bbio->csum_work);
> >>>        return 0;
> >>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >>> index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
> >>>    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,
> >>
>

在 2025/11/19 00:35, Daniel Vacek 写道:
> On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@suse.com> wrote:
[...]
>> Then why put the original bio pointer into the super generic btrfs_bio?
> 
> When encryption is enabled, it's not going to be the original bio but
> rather the encrypted one.
> 
> But giving it another thought and checking the related fscrypt code,
> the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> and freed in blk_crypto_fallback_encrypt_endio() before calling
> bio_endio() on our original plaintext bio.
> This means we have no control over the bounce bio lifetime and we
> cannot store the pointer and use it asynchronously.

Sorry I didn't get the point why we can not calculate the csum async.

Higher layer just submit a btrfs_bio, its content is the encrypted contents.

As long as it's still a btrfs_bio, we have all the needed structures to 
do async csum.
We still need to submit the bio for writes, and that means we have 
enough time to calculate the csum async, and before the endio function 
called, we're able to do whatever we need, the bio won't be suddenly 
gone during the submission.

Unless you mean the encrypted bio is not encapsulated by btrfs_bio, but 
a vanilla bio.

In that case you can not even submit it through btrfs_submit_bbio().

Thanks,
Qu

> We'll need to
> always compute the checksum synchronously for encrypted bios. In that
> case we don't need to store it in btrfs_bio::csum_bio at all. For the
> regular (unencrypted) case we can keep using the &bbio->bio.
> 
> I'll drop the csum_bio for there is no use really.
> 
> --nX
> 
>> I thought it's more common to put the original plaintext into the
>> encryption specific structure, like what we did for compression.
>>
>> Thanks,
>> Qu
>>
>>>
>>> --nX
>>>
>>>> The storage layer doesn't need to bother the plaintext bio at all, they
>>>> just write the encrypted one to disk.
>>>>
>>>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>>>>
>>>>
>>>> So why we can not just create a new bio for the final csum caculation,
>>>> just like compression?
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>>
>>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
>>>>> ---
>>>>> Compared to v5 this needed to adapt to recent async csum changes.
>>>>> ---
>>>>>     fs/btrfs/bio.c       |  4 ++--
>>>>>     fs/btrfs/bio.h       |  1 +
>>>>>     fs/btrfs/file-item.c | 17 ++++++++---------
>>>>>     fs/btrfs/file-item.h |  2 +-
>>>>>     4 files changed, 12 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>>>> index a73652b8724a..a69174b2b6b6 100644
>>>>> --- a/fs/btrfs/bio.c
>>>>> +++ b/fs/btrfs/bio.c
>>>>> @@ -542,9 +542,9 @@ 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);
>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>>>>>     #else
>>>>> -     return btrfs_csum_one_bio(bbio, false);
>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>>>>>     #endif
>>>>>     }
>>>>>
>>>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>>>>> index deaeea3becf4..c5a6c66d51a0 100644
>>>>> --- a/fs/btrfs/bio.h
>>>>> +++ b/fs/btrfs/bio.h
>>>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
>>>>>                         struct btrfs_ordered_sum *sums;
>>>>>                         struct work_struct csum_work;
>>>>>                         struct completion csum_done;
>>>>> +                     struct bio *csum_bio;
>>>>>                         struct bvec_iter csum_saved_iter;
>>>>>                         u64 orig_physical;
>>>>>                 };
>>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>>>> index 72be3ede0edf..474949074da8 100644
>>>>> --- a/fs/btrfs/file-item.c
>>>>> +++ b/fs/btrfs/file-item.c
>>>>> @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
>>>>>     {
>>>>>         struct btrfs_inode *inode = bbio->inode;
>>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>>         SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>>>>> -     struct bio *bio = &bbio->bio;
>>>>>         struct btrfs_ordered_sum *sums = bbio->sums;
>>>>> -     struct bvec_iter iter = *src;
>>>>>         phys_addr_t paddr;
>>>>>         const u32 blocksize = fs_info->sectorsize;
>>>>>         int index = 0;
>>>>>
>>>>>         shash->tfm = fs_info->csum_shash;
>>>>>
>>>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
>>>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>>>>>                 btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>>>>>                 index += fs_info->csum_size;
>>>>>         }
>>>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>>>>>
>>>>>         ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>>>>>         ASSERT(bbio->async_csum == true);
>>>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
>>>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>>>>>         complete(&bbio->csum_done);
>>>>>     }
>>>>>
>>>>>     /*
>>>>>      * 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 bio *bio, bool async)
>>>>>     {
>>>>>         struct btrfs_ordered_extent *ordered = bbio->ordered;
>>>>>         struct btrfs_inode *inode = bbio->inode;
>>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>> -     struct bio *bio = &bbio->bio;
>>>>>         struct btrfs_ordered_sum *sums;
>>>>>         unsigned nofs_flag;
>>>>>
>>>>> @@ -822,12 +819,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, bio, &iter);
>>>>>                 return 0;
>>>>>         }
>>>>>         init_completion(&bbio->csum_done);
>>>>>         bbio->async_csum = true;
>>>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
>>>>> +     bbio->csum_bio = bio;
>>>>> +     bbio->csum_saved_iter = bio->bi_iter;
>>>>>         INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>>>>>         schedule_work(&bbio->csum_work);
>>>>>         return 0;
>>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>>>> index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
>>>>>     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,
>>>>
>>
> 

On Tue, 18 Nov 2025 at 22:05, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 在 2025/11/19 00:35, Daniel Vacek 写道:
> > On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@suse.com> wrote:
> [...]
> >> Then why put the original bio pointer into the super generic btrfs_bio?
> >
> > When encryption is enabled, it's not going to be the original bio but
> > rather the encrypted one.
> >
> > But giving it another thought and checking the related fscrypt code,
> > the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> > and freed in blk_crypto_fallback_encrypt_endio() before calling
> > bio_endio() on our original plaintext bio.
> > This means we have no control over the bounce bio lifetime and we
> > cannot store the pointer and use it asynchronously.
>
> Sorry I didn't get the point why we can not calculate the csum async.
>
> Higher layer just submit a btrfs_bio, its content is the encrypted contents.
>
> As long as it's still a btrfs_bio, we have all the needed structures to
> do async csum.
> We still need to submit the bio for writes, and that means we have
> enough time to calculate the csum async, and before the endio function
> called, we're able to do whatever we need, the bio won't be suddenly
> gone during the submission.
>
> Unless you mean the encrypted bio is not encapsulated by btrfs_bio, but
> a vanilla bio.

That's the case. The bounce bio is created when you submit the
original one. The data is encrypted by fscrypt, then the csum hook is
called and the new bio submitted instead of the original one. Later
the endio frees the new one and calls endio on the original bio. This
means we don't have control over the bounce bio and cannot use it
asynchronously at the moment. The csum needs to be finished directly
in the hook.

Anyways, the hook changes are not upstreamed yet, so you can only see
it on the mailing list. And that's also why this patch makes more
sense to be sent later together with those changes.

--nX

> In that case you can not even submit it through btrfs_submit_bbio().
>
> Thanks,
> Qu
>
> > We'll need to
> > always compute the checksum synchronously for encrypted bios. In that
> > case we don't need to store it in btrfs_bio::csum_bio at all. For the
> > regular (unencrypted) case we can keep using the &bbio->bio.
> >
> > I'll drop the csum_bio for there is no use really.
> >
> > --nX
> >
> >> I thought it's more common to put the original plaintext into the
> >> encryption specific structure, like what we did for compression.
> >>
> >> Thanks,
> >> Qu
> >>
> >>>
> >>> --nX
> >>>
> >>>> The storage layer doesn't need to bother the plaintext bio at all, they
> >>>> just write the encrypted one to disk.
> >>>>
> >>>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
> >>>>
> >>>>
> >>>> So why we can not just create a new bio for the final csum caculation,
> >>>> just like compression?
> >>>>
> >>>> Thanks,
> >>>> Qu
> >>>>
> >>>>>
> >>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >>>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
> >>>>> ---
> >>>>> Compared to v5 this needed to adapt to recent async csum changes.
> >>>>> ---
> >>>>>     fs/btrfs/bio.c       |  4 ++--
> >>>>>     fs/btrfs/bio.h       |  1 +
> >>>>>     fs/btrfs/file-item.c | 17 ++++++++---------
> >>>>>     fs/btrfs/file-item.h |  2 +-
> >>>>>     4 files changed, 12 insertions(+), 12 deletions(-)
> >>>>>
> >>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >>>>> index a73652b8724a..a69174b2b6b6 100644
> >>>>> --- a/fs/btrfs/bio.c
> >>>>> +++ b/fs/btrfs/bio.c
> >>>>> @@ -542,9 +542,9 @@ 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);
> >>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
> >>>>>     #else
> >>>>> -     return btrfs_csum_one_bio(bbio, false);
> >>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
> >>>>>     #endif
> >>>>>     }
> >>>>>
> >>>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> >>>>> index deaeea3becf4..c5a6c66d51a0 100644
> >>>>> --- a/fs/btrfs/bio.h
> >>>>> +++ b/fs/btrfs/bio.h
> >>>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
> >>>>>                         struct btrfs_ordered_sum *sums;
> >>>>>                         struct work_struct csum_work;
> >>>>>                         struct completion csum_done;
> >>>>> +                     struct bio *csum_bio;
> >>>>>                         struct bvec_iter csum_saved_iter;
> >>>>>                         u64 orig_physical;
> >>>>>                 };
> >>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >>>>> index 72be3ede0edf..474949074da8 100644
> >>>>> --- a/fs/btrfs/file-item.c
> >>>>> +++ b/fs/btrfs/file-item.c
> >>>>> @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
> >>>>>     {
> >>>>>         struct btrfs_inode *inode = bbio->inode;
> >>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>>>>         SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> >>>>> -     struct bio *bio = &bbio->bio;
> >>>>>         struct btrfs_ordered_sum *sums = bbio->sums;
> >>>>> -     struct bvec_iter iter = *src;
> >>>>>         phys_addr_t paddr;
> >>>>>         const u32 blocksize = fs_info->sectorsize;
> >>>>>         int index = 0;
> >>>>>
> >>>>>         shash->tfm = fs_info->csum_shash;
> >>>>>
> >>>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> >>>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
> >>>>>                 btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
> >>>>>                 index += fs_info->csum_size;
> >>>>>         }
> >>>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
> >>>>>
> >>>>>         ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >>>>>         ASSERT(bbio->async_csum == true);
> >>>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
> >>>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
> >>>>>         complete(&bbio->csum_done);
> >>>>>     }
> >>>>>
> >>>>>     /*
> >>>>>      * 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 bio *bio, bool async)
> >>>>>     {
> >>>>>         struct btrfs_ordered_extent *ordered = bbio->ordered;
> >>>>>         struct btrfs_inode *inode = bbio->inode;
> >>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>>>> -     struct bio *bio = &bbio->bio;
> >>>>>         struct btrfs_ordered_sum *sums;
> >>>>>         unsigned nofs_flag;
> >>>>>
> >>>>> @@ -822,12 +819,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, bio, &iter);
> >>>>>                 return 0;
> >>>>>         }
> >>>>>         init_completion(&bbio->csum_done);
> >>>>>         bbio->async_csum = true;
> >>>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
> >>>>> +     bbio->csum_bio = bio;
> >>>>> +     bbio->csum_saved_iter = bio->bi_iter;
> >>>>>         INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> >>>>>         schedule_work(&bbio->csum_work);
> >>>>>         return 0;
> >>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >>>>> index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
> >>>>>     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,
> >>>>
> >>
> >
>

On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> That's the case. The bounce bio is created when you submit the
> original one. The data is encrypted by fscrypt, then the csum hook is
> called and the new bio submitted instead of the original one. Later
> the endio frees the new one and calls endio on the original bio. This
> means we don't have control over the bounce bio and cannot use it
> asynchronously at the moment. The csum needs to be finished directly
> in the hook.

And as I told you that can be changed.  Please get your entire series
out of review to allow people to try to review what you're trying to
do.

On Wed, 19 Nov 2025 at 09:22, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> > That's the case. The bounce bio is created when you submit the
> > original one. The data is encrypted by fscrypt, then the csum hook is
> > called and the new bio submitted instead of the original one. Later
> > the endio frees the new one and calls endio on the original bio. This
> > means we don't have control over the bounce bio and cannot use it
> > asynchronously at the moment. The csum needs to be finished directly
> > in the hook.
>
> And as I told you that can be changed.  Please get your entire series
> out of review to allow people to try to review what you're trying to
> do.

It's coming. Stay tuned! I'm just finishing a bit of re-design to
btrfs crypt context metadata storing which was suggested in code
review of matching changes in btrfs-progs. The fscrypt part is mostly
without any changes to the old v5 series from Josef.

--nX

On Wed, Nov 19, 2025 at 10:28:55AM +0100, Daniel Vacek wrote:
> On Wed, 19 Nov 2025 at 09:22, Christoph Hellwig <hch@infradead.org> wrote:
> > On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> > > That's the case. The bounce bio is created when you submit the
> > > original one. The data is encrypted by fscrypt, then the csum hook is
> > > called and the new bio submitted instead of the original one. Later
> > > the endio frees the new one and calls endio on the original bio. This
> > > means we don't have control over the bounce bio and cannot use it
> > > asynchronously at the moment. The csum needs to be finished directly
> > > in the hook.
> >
> > And as I told you that can be changed.  Please get your entire series
> > out of review to allow people to try to review what you're trying to
> > do.
> 
> It's coming. Stay tuned! I'm just finishing a bit of re-design to
> btrfs crypt context metadata storing which was suggested in code
> review of matching changes in btrfs-progs. The fscrypt part is mostly
> without any changes to the old v5 series from Josef.

The point is that anything directly related should be presented
together.  Patches 1-3 don't make sense without the rest.  And
especially for patch 3 I'm really doubtful it is a good idea to
start with, but that can only be argued when the reset is shown.

On Wed, 19 Nov 2025 at 10:32, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Nov 19, 2025 at 10:28:55AM +0100, Daniel Vacek wrote:
> > On Wed, 19 Nov 2025 at 09:22, Christoph Hellwig <hch@infradead.org> wrote:
> > > On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> > > > That's the case. The bounce bio is created when you submit the
> > > > original one. The data is encrypted by fscrypt, then the csum hook is
> > > > called and the new bio submitted instead of the original one. Later
> > > > the endio frees the new one and calls endio on the original bio. This
> > > > means we don't have control over the bounce bio and cannot use it
> > > > asynchronously at the moment. The csum needs to be finished directly
> > > > in the hook.
> > >
> > > And as I told you that can be changed.  Please get your entire series
> > > out of review to allow people to try to review what you're trying to
> > > do.
> >
> > It's coming. Stay tuned! I'm just finishing a bit of re-design to
> > btrfs crypt context metadata storing which was suggested in code
> > review of matching changes in btrfs-progs. The fscrypt part is mostly
> > without any changes to the old v5 series from Josef.
>
> The point is that anything directly related should be presented
> together.  Patches 1-3 don't make sense without the rest.  And
> especially for patch 3 I'm really doubtful it is a good idea to
> start with, but that can only be argued when the reset is shown.

Oh, sorry I didn't say that out loud. Maybe you missed it, I already
dropped this patch (and another one) from v7 yesterday.

The patch itself is still the same as it was in v4 or v5 where you did
not object. I'd say let's discuss it again when I'll send out the rest
of the series so that we have the full picture. Maybe the refactoring
you mentioned could provide us with a better way for checksumming.

About patches 1 and 2, it's really up to Dave. He picked them and
asked me to send them ahead. I'm perfectly fine either way.

--nX

在 2025/11/19 18:04, Daniel Vacek 写道:
[...]
>> Unless you mean the encrypted bio is not encapsulated by btrfs_bio, but
>> a vanilla bio.
> 

Now it's completely unrelated to the patchset, but I'm still very 
interested in the workflow of fscrypt.

Is there high level docs about it?

> That's the case. The bounce bio is created when you submit the
> original one. The data is encrypted by fscrypt, then the csum hook is
> called and the new bio submitted instead of the original one.

I guess by "submit" you didn't mean btrfs_submit_bio(), but something 
else right?
Or the plaintext bio will be submitted to the disks.

> Later
> the endio frees the new one and calls endio on the original bio.

But the encrypted bio still needs to be handled by btrfs_submit_bio(), 
as we still have a lot of extra works like splitting due to stripe 
boundary and duplicating to different mirrors.

The end io function of the encrypted bio won't be called until all of 
our btrfs specific work/IO is done.

> This
> means we don't have control over the bounce bio and cannot use it
> asynchronously at the moment. The csum needs to be finished directly
> in the hook.

Any code I can look into the "hook"?

I checked the ext4's code (althought it doesn't support csum), they just 
call fscrypt_encrypt_pagecache_blocks() on the plaintext folios, and 
submit the encrypted folios as the bio payload.

It's completely fine to just add those encrypted folios into a 
btrfs_bio, and submit as usual.

I believe we can handle it inside the endio function, end_bbio_data_write().

In fact, currently for write bios, we use an empty btrfs_bio::private, 
we can definitely allocate some extra structure for encrypted writes to 
track the plain text pages (mostly just the start/end file pos).


So my uneducated guess of the encyrpted write would be something like 
the following, mostly happenin in the function submit_extent_folio():

submit_extent_folio()
{
	/* setup bio_ctrl for encrypted, involving the
          * btrfs_bio::private.
	 * So that alloc_new_bio() will allocate a new
	 * private for encrypted write.
          */
	do {
		if (is_encrypted) {
			data_folio = fscrypt_encrypt();
			ret = bio_add_folio();
		} else {
			ret = bio_add_folio();
		}
		/* the remaining as usual */
	}
}

Then in the endio do the special handling:

end_bbio_data_write()
{
	if (!bbio->private) {
		/* The existing one. */
		end_bbio_data_write_plaintext();
	} else {
		/*
		 * Using bbio->private to grab the page cache folios
		 * and free the encrypted folios.
		 */
	}
	bio_put(&bbio->bio);
}

Thus that's why I didn't see the point to trace the original bbio inside 
btrfs_bio, and also didn't see why we have any problem doing async csum.

BTW, the same can be applied to data read, as we didn't utilize 
btrfs_bio::private either.

Or did I miss something?

Thanks,
Qu

> 
> Anyways, the hook changes are not upstreamed yet, so you can only see
> it on the mailing list. And that's also why this patch makes more
> sense to be sent later together with those changes.
> 
> --nX
> 
>> In that case you can not even submit it through btrfs_submit_bbio().
>>
>> Thanks,
>> Qu
>>
>>> We'll need to
>>> always compute the checksum synchronously for encrypted bios. In that
>>> case we don't need to store it in btrfs_bio::csum_bio at all. For the
>>> regular (unencrypted) case we can keep using the &bbio->bio.
>>>
>>> I'll drop the csum_bio for there is no use really.
>>>
>>> --nX
>>>
>>>> I thought it's more common to put the original plaintext into the
>>>> encryption specific structure, like what we did for compression.
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>>
>>>>> --nX
>>>>>
>>>>>> The storage layer doesn't need to bother the plaintext bio at all, they
>>>>>> just write the encrypted one to disk.
>>>>>>
>>>>>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>>>>>>
>>>>>>
>>>>>> So why we can not just create a new bio for the final csum caculation,
>>>>>> just like compression?
>>>>>>
>>>>>> Thanks,
>>>>>> Qu
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>>>>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
>>>>>>> ---
>>>>>>> Compared to v5 this needed to adapt to recent async csum changes.
>>>>>>> ---
>>>>>>>      fs/btrfs/bio.c       |  4 ++--
>>>>>>>      fs/btrfs/bio.h       |  1 +
>>>>>>>      fs/btrfs/file-item.c | 17 ++++++++---------
>>>>>>>      fs/btrfs/file-item.h |  2 +-
>>>>>>>      4 files changed, 12 insertions(+), 12 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>>>>>> index a73652b8724a..a69174b2b6b6 100644
>>>>>>> --- a/fs/btrfs/bio.c
>>>>>>> +++ b/fs/btrfs/bio.c
>>>>>>> @@ -542,9 +542,9 @@ 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);
>>>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>>>>>>>      #else
>>>>>>> -     return btrfs_csum_one_bio(bbio, false);
>>>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>>>>>>>      #endif
>>>>>>>      }
>>>>>>>
>>>>>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>>>>>>> index deaeea3becf4..c5a6c66d51a0 100644
>>>>>>> --- a/fs/btrfs/bio.h
>>>>>>> +++ b/fs/btrfs/bio.h
>>>>>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
>>>>>>>                          struct btrfs_ordered_sum *sums;
>>>>>>>                          struct work_struct csum_work;
>>>>>>>                          struct completion csum_done;
>>>>>>> +                     struct bio *csum_bio;
>>>>>>>                          struct bvec_iter csum_saved_iter;
>>>>>>>                          u64 orig_physical;
>>>>>>>                  };
>>>>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>>>>>> index 72be3ede0edf..474949074da8 100644
>>>>>>> --- a/fs/btrfs/file-item.c
>>>>>>> +++ b/fs/btrfs/file-item.c
>>>>>>> @@ -765,21 +765,19 @@ 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 bio *bio, struct bvec_iter *iter)
>>>>>>>      {
>>>>>>>          struct btrfs_inode *inode = bbio->inode;
>>>>>>>          struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>>>>          SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>>>>>>> -     struct bio *bio = &bbio->bio;
>>>>>>>          struct btrfs_ordered_sum *sums = bbio->sums;
>>>>>>> -     struct bvec_iter iter = *src;
>>>>>>>          phys_addr_t paddr;
>>>>>>>          const u32 blocksize = fs_info->sectorsize;
>>>>>>>          int index = 0;
>>>>>>>
>>>>>>>          shash->tfm = fs_info->csum_shash;
>>>>>>>
>>>>>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
>>>>>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>>>>>>>                  btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>>>>>>>                  index += fs_info->csum_size;
>>>>>>>          }
>>>>>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>>>>>>>
>>>>>>>          ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>>>>>>>          ASSERT(bbio->async_csum == true);
>>>>>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
>>>>>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>>>>>>>          complete(&bbio->csum_done);
>>>>>>>      }
>>>>>>>
>>>>>>>      /*
>>>>>>>       * 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 bio *bio, bool async)
>>>>>>>      {
>>>>>>>          struct btrfs_ordered_extent *ordered = bbio->ordered;
>>>>>>>          struct btrfs_inode *inode = bbio->inode;
>>>>>>>          struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>>>> -     struct bio *bio = &bbio->bio;
>>>>>>>          struct btrfs_ordered_sum *sums;
>>>>>>>          unsigned nofs_flag;
>>>>>>>
>>>>>>> @@ -822,12 +819,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, bio, &iter);
>>>>>>>                  return 0;
>>>>>>>          }
>>>>>>>          init_completion(&bbio->csum_done);
>>>>>>>          bbio->async_csum = true;
>>>>>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
>>>>>>> +     bbio->csum_bio = bio;
>>>>>>> +     bbio->csum_saved_iter = bio->bi_iter;
>>>>>>>          INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>>>>>>>          schedule_work(&bbio->csum_work);
>>>>>>>          return 0;
>>>>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>>>>>> index 5645c5e3abdb..d16fd2144552 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_csum_file_blocks(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, struct bio *bio, bool async);
>>>>>>>      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,
>>>>>>
>>>>
>>>
>>

On Tue, Nov 18, 2025 at 03:05:25PM +0100, Daniel Vacek wrote:
> But giving it another thought and checking the related fscrypt code,
> the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> and freed in blk_crypto_fallback_encrypt_endio() before calling
> bio_endio() on our original plaintext bio.

That code is getting major refactoring right now, and allowing the
file system to hook into the submission is a possibility.

The problem is that I have no idea what you're trying to do as the
context is missing.

In general prep serious should be self contained and at least borderline
useful by themselves.  Adding random dead code checks or weird arguments
as done here are not useful in a prep series without context, they
should be close to the code making use of them to be understandable.

On Tue, 18 Nov 2025 at 16:08, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Nov 18, 2025 at 03:05:25PM +0100, Daniel Vacek wrote:
> > But giving it another thought and checking the related fscrypt code,
> > the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> > and freed in blk_crypto_fallback_encrypt_endio() before calling
> > bio_endio() on our original plaintext bio.
>
> That code is getting major refactoring right now, and allowing the
> file system to hook into the submission is a possibility.
>
> The problem is that I have no idea what you're trying to do as the
> context is missing.
>
> In general prep series should be self contained and at least borderline
> useful by themselves.  Adding random dead code checks or weird arguments
> as done here are not useful in a prep series without context, they
> should be close to the code making use of them to be understandable.

Yeah, agreed. It would be better to send this one together with the
fscrypt changes later. I was suggested by Dave to pick this one ahead
and I did not argue while I probably should have.

--nX

On Tue, 18 Nov 2025 at 16:08, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Nov 18, 2025 at 03:05:25PM +0100, Daniel Vacek wrote:
> > But giving it another thought and checking the related fscrypt code,
> > the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> > and freed in blk_crypto_fallback_encrypt_endio() before calling
> > bio_endio() on our original plaintext bio.
>
> That code is getting major refactoring right now, and allowing the
> file system to hook into the submission is a possibility.
>
> The problem is that I have no idea what you're trying to do as the
> context is missing.
>
> In general prep serious should be self contained and at least borderline
> useful by themselves.  Adding random dead code checks or weird arguments
> as done here are not useful in a prep series without context, they
> should be close to the code making use of them to be understandable.