Re: [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails

[Qu Wenruo]

在 2026/11/2 07:08, Qu Wenruo 写道:
> 在 2026/11/2 05:09, Mani Tofigh via B4 Relay 写道:
> > From: Mani Tofigh <mt4022@xxxxxxxxxxxx>
> >
> > A single failed checksum allocation can make a later device replace
> > hang forever.  btrfs_submit_chunk() takes a dev-replace bio counter
> > reference and a bioc before it hands a write to the checksum worker.
> > If the checksum fails there (eg its allocation fails),
> > run_one_async_done() ends the bio but drops neither.  The counter then
> > never gets back to zero and the next replace waits for it:
> >
> >    [    8.369171] BTRFS info (device ram0): dev_replace from /dev/ram0 
> > (devid 1) to /dev/ram1 finished
> >    [   41.534247] INFO: task btrfscsum:80 blocked for more than 20 
> > seconds.
> >    [   41.534391]  btrfs_rm_dev_replace_blocked+0x135/0x180
> >    [   41.534463]  btrfs_dev_replace_finishing+0xefe/0x13b0
> >
> > Put the bioc and drop the counter in that error branch as the
> > synchronous error path does.  Also put the bioc on the synchronous
> > path, which only drops the counter.  With the fix, the replace
> > finishes.
> >
> > This bug was identified and confirmed with the help of fault-injection
> > testing on a deterministic hypervisor currently under development.  The
> > C reproducer below however hits it within about 35s.
> >
> > Fixes: f8a53bb58ec7 ("btrfs: handle checksum generation in the storage 
> > layer")
> > Cc: stable@xxxxxxxxxxxxxxx # 6.6+
> > Signed-off-by: Mani Tofigh <mt4022@xxxxxxxxxxxx>
>
> Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
>
> Although it's not that common to hit this path.
>
> On EXPERIMENTAL builds you are already running with async csum 
> generation, other than the offload path.
>
> Otherwise on most x86_64, the default CRC32 is considered fast, so is 
> xxhash on all archs, and they will not go through the offload mode.
>
> So I'm wondering what's your setup? Are you running SHA256 algo for 
> checksum? I can add it to the commit message during merge.

And Sashiko exposed an existing problem, even with this patch applied, 
the error handling of btrfs_split_bio() is not going to release the bioc.

https://sashiko.dev/#/map/223655

Although it's an existing bug, since you're touching all the bioc 
releasing, it would be better to handle them all in one patch.

Thanks,
Qu

> > ---
> > Reproducer: https://github.com/manitofigh/files/blob/main/linux- 
> > repros/btrfscsum.c
> > ---
> >   fs/btrfs/bio.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > index 2c6234f6182f..a508f4abd3a3 100644
> > --- a/fs/btrfs/bio.c
> > +++ b/fs/btrfs/bio.c
> > @@ -624,6 +624,8 @@ static void run_one_async_done(struct btrfs_work 
> > *work, bool do_free)
> >       /* If an error occurred we just want to clean up the bio and 
> > move on. */
> >       if (bio->bi_status) {
> > +        btrfs_put_bioc(async->bioc);
> > +        btrfs_bio_counter_dec(async->bbio->inode->root->fs_info);
> >           btrfs_bio_end_io(async->bbio, bio->bi_status);
> >           return;
> >       }
> > @@ -822,6 +824,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio 
> > *bbio, int mirror_num)
> >       return map_length == length;
> >   fail:
> > +    btrfs_put_bioc(bioc);
> >       btrfs_bio_counter_dec(fs_info);
> >       /*
> >        * We have split the original bbio, now we have to end both the 
> > current
> >
> > ---
> > base-commit: 5e6c45aaaf93343c7f8a1cde9797080238af5c1b
> > change-id: 20261030-btrfs-async-csum-leak-1772df5f0c32
> >
> > Best regards,
> > --
> > Mani Tofigh <mt4022@xxxxxxxxxxxx>