[PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request

[PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request

[zhangshida2026]

From: Shida Zhang <zhangshida@kylinos.cn>

When a bcache device is in a detached state, iostat can show 100%
utilization even after I/O workload completion.

This happens because the caller, cached_dev_make_request(), calls
bio_start_io_acct() to begin accounting. However, if the bio hits an
early exit path in detached_dev_do_request()—either due to an
unsupported discard request or a bio_alloc_clone() failure—the
corresponding bio_end_io_acct() is never called. This leaves the
in-flight counter permanently incremented, causing the kernel to
report the device as 100% busy.

Add the missing bio_end_io_acct() calls to these error/early-exit
paths to ensure proper I/O accounting.

Fixes: d62e26b3ffd28 ("block: pass in queue to inflight accounting")
Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>
Acked-by: Coly Li <colyli@fnnas.com>
---
 drivers/md/bcache/request.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index a02aecac05c..7d855e66a10 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -1107,6 +1107,7 @@ static void detached_dev_do_request(struct bcache_device *d,
 
 	if (bio_op(orig_bio) == REQ_OP_DISCARD &&
 	    !bdev_max_discard_sectors(dc->bdev)) {
+		bio_end_io_acct(orig_bio, start_time);
 		bio_endio(orig_bio);
 		return;
 	}
@@ -1114,6 +1115,7 @@ static void detached_dev_do_request(struct bcache_device *d,
 	clone_bio = bio_alloc_clone(dc->bdev, orig_bio, GFP_NOIO,
 				    &d->bio_detached);
 	if (!clone_bio) {
+		bio_end_io_acct(orig_bio, start_time);
 		orig_bio->bi_status = BLK_STS_RESOURCE;
 		bio_endio(orig_bio);
 		return;
-- 
2.34.1

Re: [PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request

[Christoph Hellwig]

On Mon, Jan 26, 2026 at 05:28:54PM +0800, zhangshida2026@163.com wrote:
> From: Shida Zhang <zhangshida@kylinos.cn>
> 
> When a bcache device is in a detached state, iostat can show 100%
> utilization even after I/O workload completion.
> 
> This happens because the caller, cached_dev_make_request(), calls
> bio_start_io_acct() to begin accounting. However, if the bio hits an
> early exit path in detached_dev_do_request()—either due to an
> unsupported discard request or a bio_alloc_clone() failure—the
> corresponding bio_end_io_acct() is never called. This leaves the
> in-flight counter permanently incremented, causing the kernel to
> report the device as 100% busy.
> 
> Add the missing bio_end_io_acct() calls to these error/early-exit
> paths to ensure proper I/O accounting.
> 
> Fixes: d62e26b3ffd28 ("block: pass in queue to inflight accounting")

I don't think that is correct.  This was just a trivial calling
convention change.

From doing a quick git-blame chain this looks like the culprit:

bc082a55d25c837341709accaf11311c3a9af727
Author: Tang Junhui <tang.junhui@zte.com.cn>
Date:   Sun Mar 18 17:36:19 2018 -0700

    bcache: fix inaccurate io state for detached bcache devices


> +		bio_end_io_acct(orig_bio, start_time);
>  		bio_endio(orig_bio);
>  		return;
>  	}
> @@ -1114,6 +1115,7 @@ static void detached_dev_do_request(struct bcache_device *d,
>  	clone_bio = bio_alloc_clone(dc->bdev, orig_bio, GFP_NOIO,
>  				    &d->bio_detached);
>  	if (!clone_bio) {
> +		bio_end_io_acct(orig_bio, start_time);
>  		orig_bio->bi_status = BLK_STS_RESOURCE;
>  		bio_endio(orig_bio);
>  		return;

This is begging to use a goto label to share code, if it weren't for the
fact that bio_alloc_clone with GFP_NOIO will never return NULL because
both because the bio itself and the crypt or integrity information are
backed by mempool.

So this second copy of the code is actually dead and should be removed
in a prep patch before this one.  Sorry for not catching this earlier.

Re: [PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request

[Coly Li]

On Mon, Jan 26, 2026 at 04:52:58AM +0800, Christoph Hellwig wrote:
> On Mon, Jan 26, 2026 at 05:28:54PM +0800, zhangshida2026@163.com wrote:
> > From: Shida Zhang <zhangshida@kylinos.cn>
> > 
> > When a bcache device is in a detached state, iostat can show 100%
> > utilization even after I/O workload completion.
> > 
> > This happens because the caller, cached_dev_make_request(), calls
> > bio_start_io_acct() to begin accounting. However, if the bio hits an
> > early exit path in detached_dev_do_request()—either due to an
> > unsupported discard request or a bio_alloc_clone() failure—the
> > corresponding bio_end_io_acct() is never called. This leaves the
> > in-flight counter permanently incremented, causing the kernel to
> > report the device as 100% busy.
> > 
> > Add the missing bio_end_io_acct() calls to these error/early-exit
> > paths to ensure proper I/O accounting.
> > 
> > Fixes: d62e26b3ffd28 ("block: pass in queue to inflight accounting")
> 
> I don't think that is correct.  This was just a trivial calling
> convention change.
> 

Hi Shida,

This Fixes tag is misleading. The correct to-be-fixed patch is commit
3ef825dfd4e4 ("bcache: use bio cloning for detached device requests")
in linux-block/block-6.19 branch.

Since this patch is not upstreamed yet, the sha-1 commit id might be
changed, so reference the patch name might be fine IMHO.


> From doing a quick git-blame chain this looks like the culprit:
> 
> bc082a55d25c837341709accaf11311c3a9af727
> Author: Tang Junhui <tang.junhui@zte.com.cn>
> Date:   Sun Mar 18 17:36:19 2018 -0700
> 
>     bcache: fix inaccurate io state for detached bcache devices
> 
> 
> > +		bio_end_io_acct(orig_bio, start_time);
> >  		bio_endio(orig_bio);
> >  		return;
> >  	}
> > @@ -1114,6 +1115,7 @@ static void detached_dev_do_request(struct bcache_device *d,
> >  	clone_bio = bio_alloc_clone(dc->bdev, orig_bio, GFP_NOIO,
> >  				    &d->bio_detached);
> >  	if (!clone_bio) {
> > +		bio_end_io_acct(orig_bio, start_time);
> >  		orig_bio->bi_status = BLK_STS_RESOURCE;
> >  		bio_endio(orig_bio);
> >  		return;
> 
> This is begging to use a goto label to share code, if it weren't for the
> fact that bio_alloc_clone with GFP_NOIO will never return NULL because
> both because the bio itself and the crypt or integrity information are
> backed by mempool.
> 
> So this second copy of the code is actually dead and should be removed
> in a prep patch before this one.  Sorry for not catching this earlier.

Hi Christoph,

Do you mean after using a goto lebal to share code, the second part will
be dead code? Just make sure I don't misunderstand your text.

Thanks.

Coly Li

Re: [PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request

[Christoph Hellwig]

On Mon, Jan 26, 2026 at 11:00:56PM +0800, Coly Li wrote:
> > I don't think that is correct.  This was just a trivial calling
> > convention change.
> > 
> 
> Hi Shida,
> 
> This Fixes tag is misleading. The correct to-be-fixed patch is commit
> 3ef825dfd4e4 ("bcache: use bio cloning for detached device requests")
> in linux-block/block-6.19 branch.

Not really.  The missing accounting was present before for the no-op
discard case.  See my reply for what I think is the culprit.

Re:Re: [PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request

[zhangshida2026]

At 2026-01-26 20:52:58, "Christoph Hellwig" <hch@infradead.org> wrote:
>On Mon, Jan 26, 2026 at 05:28:54PM +0800, zhangshida2026@163.com wrote:
>> From: Shida Zhang <zhangshida@kylinos.cn>
>> 
>> When a bcache device is in a detached state, iostat can show 100%
>> utilization even after I/O workload completion.
>> 
>> This happens because the caller, cached_dev_make_request(), calls
>> bio_start_io_acct() to begin accounting. However, if the bio hits an
>> early exit path in detached_dev_do_request()—either due to an
>> unsupported discard request or a bio_alloc_clone() failure—the
>> corresponding bio_end_io_acct() is never called. This leaves the
>> in-flight counter permanently incremented, causing the kernel to
>> report the device as 100% busy.
>> 
>> Add the missing bio_end_io_acct() calls to these error/early-exit
>> paths to ensure proper I/O accounting.
>> 
>> Fixes: d62e26b3ffd28 ("block: pass in queue to inflight accounting")
>
>I don't think that is correct.  This was just a trivial calling
>convention change.
>
>From doing a quick git-blame chain this looks like the culprit:
>
>bc082a55d25c837341709accaf11311c3a9af727
>Author: Tang Junhui <tang.junhui@zte.com.cn>
>Date:   Sun Mar 18 17:36:19 2018 -0700
>
>    bcache: fix inaccurate io state for detached bcache devices
>
>
>> +		bio_end_io_acct(orig_bio, start_time);
>>  		bio_endio(orig_bio);
>>  		return;
>>  	}
>> @@ -1114,6 +1115,7 @@ static void detached_dev_do_request(struct bcache_device *d,
>>  	clone_bio = bio_alloc_clone(dc->bdev, orig_bio, GFP_NOIO,
>>  				    &d->bio_detached);
>>  	if (!clone_bio) {
>> +		bio_end_io_acct(orig_bio, start_time);
>>  		orig_bio->bi_status = BLK_STS_RESOURCE;
>>  		bio_endio(orig_bio);
>>  		return;
>
>This is begging to use a goto label to share code, if it weren't for the
>fact that bio_alloc_clone with GFP_NOIO will never return NULL because
>both because the bio itself and the crypt or integrity information are
>backed by mempool.
>
>So this second copy of the code is actually dead and should be removed

>in a prep patch before this one.  Sorry for not catching this earlier.


Hi Christoph and Coly,

Thanks for the review and the explanation.

I see your point about bio_alloc_clone with GFP_NOIO. I will split 
this into a two-patch series:

    1. A prep patch to remove the dead code .
    2. The fix for the I/O accounting leak in the remaining path.

Regarding the Fixes tag: I initially looked at the discard path which 
has existed since the initial commit cafe56359144 
("bcache: A block layer cache").

Thanks,
Shida
-----------------------
dece16353ef47 (Jens Axboe        2015-11-05 10:41:16 -0700  955) static blk_qc_t cached_dev_make_request(struct request_queue *q,
dece16353ef47 (Jens Axboe        2015-11-05 10:41:16 -0700  956)                                        struct bio *bio)
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  957) {
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  958)        struct search *s;
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  959)        struct bcache_device *d = bio->bi_bdev->bd_disk->private_data;
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  960)        struct cached_dev *dc = container_of(d, struct cached_dev, disk);
aae4933da9488 (Gu Zheng          2014-11-24 11:05:24 +0800  961)        int rw = bio_data_dir(bio);
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  962) 
d62e26b3ffd28 (Jens Axboe        2017-06-30 21:55:08 -0600  963)        generic_start_io_acct(q, rw, bio_sectors(bio), &d->disk->part0);
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  964) 
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  965)        bio->bi_bdev = dc->bdev;
4f024f3797c43 (Kent Overstreet   2013-10-11 15:44:27 -0700  966)        bio->bi_iter.bi_sector += dc->sb.data_offset;
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  967) 
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  968)        if (cached_dev_get(dc)) {
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  969)                s = search_alloc(bio, d);
220bb38c21b83 (Kent Overstreet   2013-09-10 19:02:45 -0700  970)                trace_bcache_request_start(s->d, bio);
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  971) 
4f024f3797c43 (Kent Overstreet   2013-10-11 15:44:27 -0700  972)                if (!bio->bi_iter.bi_size) {
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  973)                        /*
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  974)                         * can't call bch_journal_meta from under
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  975)                         * generic_make_request
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  976)                         */
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  977)                        continue_at_nobarrier(&s->cl,
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  978)                                              cached_dev_nodata,
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  979)                                              bcache_wq);
a34a8bfd4e635 (Kent Overstreet   2013-10-24 17:07:04 -0700  980)                } else {
220bb38c21b83 (Kent Overstreet   2013-09-10 19:02:45 -0700  981)                        s->iop.bypass = check_should_bypass(dc, bio);
84f0db03ea1e0 (Kent Overstreet   2013-07-24 17:24:52 -0700  982) 
84f0db03ea1e0 (Kent Overstreet   2013-07-24 17:24:52 -0700  983)                        if (rw)
cdd972b164be8 (Kent Overstreet   2013-09-10 17:06:17 -0700  984)                                cached_dev_write(dc, s);
84f0db03ea1e0 (Kent Overstreet   2013-07-24 17:24:52 -0700  985)                        else
cdd972b164be8 (Kent Overstreet   2013-09-10 17:06:17 -0700  986)                                cached_dev_read(dc, s);
84f0db03ea1e0 (Kent Overstreet   2013-07-24 17:24:52 -0700  987)                }
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  988)        } else {
ad0d9e76a4124 (Mike Christie     2016-06-05 14:32:05 -0500  989)                if ((bio_op(bio) == REQ_OP_DISCARD) &&
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  990)                    !blk_queue_discard(bdev_get_queue(dc->bdev)))
4246a0b63bd8f (Christoph Hellwig 2015-07-20 15:29:37 +0200  991)                        bio_endio(bio);
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  992)                else
749b61dab3073 (Kent Overstreet   2013-11-23 23:11:25 -0800  993)                        generic_make_request(bio);
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  994)        }
dece16353ef47 (Jens Axboe        2015-11-05 10:41:16 -0700  995) 
dece16353ef47 (Jens Axboe        2015-11-05 10:41:16 -0700  996)        return BLK_QC_T_NONE;
cafe563591446 (Kent Overstreet   2013-03-23 16:11:31 -0700  997) }