* [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails
@ 2026-11-01 18:39 Mani Tofigh via B4 Relay
2026-11-01 20:38 ` Qu Wenruo
0 siblings, 1 reply; 4+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-01 18:39 UTC (permalink / raw)
To: David Sterba, Chris Mason
Cc: Qu Wenruo, Christoph Hellwig, Josef Bacik, Johannes Thumshirn,
linux-btrfs, linux-kernel, tz2294, stable, Mani Tofigh
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>
---
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails
2026-11-01 18:39 [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails Mani Tofigh via B4 Relay
@ 2026-11-01 20:38 ` Qu Wenruo
2026-11-01 20:53 ` Qu Wenruo
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2026-11-01 20:38 UTC (permalink / raw)
To: mt4022, David Sterba, Chris Mason
Cc: Qu Wenruo, Christoph Hellwig, Josef Bacik, Johannes Thumshirn,
linux-btrfs, linux-kernel, tz2294, stable
在 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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails
2026-11-01 20:38 ` Qu Wenruo
@ 2026-11-01 20:53 ` Qu Wenruo
2026-11-02 1:59 ` Mani Tofigh
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2026-11-01 20:53 UTC (permalink / raw)
To: Qu Wenruo, mt4022, David Sterba, Chris Mason
Cc: Christoph Hellwig, Josef Bacik, Johannes Thumshirn, linux-btrfs,
linux-kernel, tz2294, stable
在 2026/11/2 07:08, Qu Wenruo 写道:
>
>
> 在 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.
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@columbia.edu>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails
2026-11-01 20:53 ` Qu Wenruo
@ 2026-11-02 1:59 ` Mani Tofigh
0 siblings, 0 replies; 4+ messages in thread
From: Mani Tofigh @ 2026-11-02 1:59 UTC (permalink / raw)
To: Qu Wenruo, Qu Wenruo, David Sterba, Chris Mason
Cc: Christoph Hellwig, Josef Bacik, Johannes Thumshirn, linux-btrfs,
linux-kernel, tz2294, stable
On 11/1/26 3:53 PM, Qu Wenruo wrote:
> 在 2026/11/2 07:08, Qu Wenruo 写道:
>> 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.
Yep, it's a sha256 filesystem on a non-EXPERIMENTAL build, with
failslab failing the checksum allocation in the worker.
> 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.
Good catch, I'll put the bioc there too and send a v2 with the sha256
setup mentioned in the commit message.
Thanks for the review!
Mani
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-02 1:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-01 18:39 [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails Mani Tofigh via B4 Relay
2026-11-01 20:38 ` Qu Wenruo
2026-11-01 20:53 ` Qu Wenruo
2026-11-02 1:59 ` Mani Tofigh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®