Re:Re: [PATCH v2] bcache: fix I/O accounting leak in detached_dev_do_request
zhangshida2026 <[email protected]> Tue, 27 Jan 2026 10:11:27 +0800 (CST)
| Newsgroups | org.kernel.vger.linux-bcache,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
At 2026-01-26 20:52:58, "Christoph Hellwig" <[email protected]> wrote: >On Mon, Jan 26, 2026 at 05:28:54PM +0800, [email protected] wrote: >> From: Shida Zhang <[email protected]> >> >> 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 <[email protected]> >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) }