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

[zhangshida2026 <zhangshida2026@163.com>]

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) }