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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/11/2 05:09, Mani Tofigh via B4 Relay 写道:
> From: Mani Tofigh <mt4022@columbia.edu>
> 
> 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@vger.kernel.org # 6.6+
> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>

Reviewed-by: Qu Wenruo <wqu@suse.com>

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.

> ---
> 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@columbia.edu>