Re: [PATCH 1/2] btrfs: consume the given iter directly instead of copying in csum_one_bio()

[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@xxxxxxxx>
> > ---
> >   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@xxxxxxxx>

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;