| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6f494dc.1ce2.19bfd3858c2.Coremail.zhangshida2026@163.com>
Date: Tue, 27 Jan 2026 10:11:27 +0800 (CST)
From: zhangshida2026 <zhangshida2026@....com>
To: "Christoph Hellwig" <hch@...radead.org>
Cc: colyli@...as.com, kent.overstreet@...ux.dev, axboe@...nel.dk,
osandov@...com, bvanassche@....org, linux-bcache@...r.kernel.org,
linux-kernel@...r.kernel.org, zhangshida@...inos.cn,
starzhangzsd@...il.com
Subject: Re:Re: [PATCH v2] bcache: fix I/O accounting leak in
detached_dev_do_request
At 2026-01-26 20:52:58, "Christoph Hellwig" <hch@...radead.org> wrote:
>On Mon, Jan 26, 2026 at 05:28:54PM +0800, zhangshida2026@....com wrote:
>> From: Shida Zhang <zhangshida@...inos.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@....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) }
Powered by blists - more mailing lists