linux-kernel - Re: [PATCH] btrfs: simplify async csum synchronization

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPjX3FdSsL+-MYf2onDZSmdroppZre6o0Ciw8xFPg4qZUJhnQQ@mail.gmail.com>
Date: Thu, 13 Nov 2025 11:24:04 +0100
From: Daniel Vacek <neelx@...e.com>
To: Qu Wenruo <quwenruo.btrfs@....com>
Cc: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>, 
	Qu Wenruo <wqu@...e.com>, linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: simplify async csum synchronization

On Thu, 13 Nov 2025 at 10:44, Qu Wenruo <quwenruo.btrfs@....com> wrote:
> 在 2025/11/13 20:01, Daniel Vacek 写道:
> > We don't need the completion csum_done which marks the csum work
> > has been executed. We can simply flush_work() instead.
> >
> > This way we can slim down the btrfs_bio structure by 32 bytes matching
> > it's size to what it used to be before introducing the async csums.
> > Hence not making any change with respect to the structure size.
> > ---
> > This is a simple fixup for "btrfs: introduce btrfs_bio::async_csum" in
> > for-next and can be squashed into it.
> > ---
> >   fs/btrfs/bio.c       | 2 +-
> >   fs/btrfs/bio.h       | 1 -
> >   fs/btrfs/file-item.c | 6 +++---
> >   3 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > index a73652b8724a..fd6e4278a62f 100644
> > --- a/fs/btrfs/bio.c
> > +++ b/fs/btrfs/bio.c
> > @@ -106,7 +106,7 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
> >       ASSERT(in_task());
> >
> >       if (bbio->async_csum)
> > -             wait_for_completion(&bbio->csum_done);
> > +             flush_work(&bbio->csum_work);
> >
> >       bbio->bio.bi_status = status;
> >       if (bbio->bio.bi_pool == &btrfs_clone_bioset) {
> > diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> > index deaeea3becf4..0b09d9122fa2 100644
> > --- a/fs/btrfs/bio.h
> > +++ b/fs/btrfs/bio.h
> > @@ -57,7 +57,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;
> >               };
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 72be3ede0edf..3e9241f360c8 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -792,7 +792,6 @@ 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);
> > -     complete(&bbio->csum_done);
> >   }
> >
> >   /*
> > @@ -805,6 +804,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >       struct bio *bio = &bbio->bio;
> >       struct btrfs_ordered_sum *sums;
> > +     struct workqueue_struct *wq;
> >       unsigned nofs_flag;
> >
> >       nofs_flag = memalloc_nofs_save();
> > @@ -825,11 +825,11 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >               csum_one_bio(bbio, &bbio->bio.bi_iter);
> >               return 0;
> >       }
> > -     init_completion(&bbio->csum_done);
> >       bbio->async_csum = true;
> >       bbio->csum_saved_iter = bbio->bio.bi_iter;
> >       INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> > -     schedule_work(&bbio->csum_work);
> > +     wq = bio->bi_opf & REQ_META? fs_info->endio_meta_workers: fs_info->endio_workers;
>
> Metadata will not go into btrfs_csum_one_bio(), thus it's fixed to
> endio_workers().

I suspected, but I was not sure so rather be safe than sorry.
That simplifies it quite nicely. Thanks.

> > +     queue_work(wq, &bbio->csum_work);
> >       return 0;
> >   }
> >
>