| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <84128576-17f4-460a-9d2f-9e40f43f2ef7@gmx.com>
Date: Wed, 19 Nov 2025 07:35:04 +1030
From: Qu Wenruo <quwenruo.btrfs@....com>
To: Daniel Vacek <neelx@...e.com>, Qu Wenruo <wqu@...e.com>
Cc: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio
在 2025/11/19 00:35, Daniel Vacek 写道:
> On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@...e.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@...icpanda.com>
>>>>> Signed-off-by: Daniel Vacek <neelx@...e.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,
>>>>
>>
>
Powered by blists - more mailing lists