[PATCH openEuler-1.0-LTS 1/2] block: fix blk-iolatency accounting underflow

From: Dennis Zhou <dennis@kernel.org> mainline inclusion from mainline-v5.0-rc1 commit 13369816cb648f897ce9cbf57e55eeb742ce4eb3 category: bugfix bugzilla: 182234 CVE: NA --------------------------- The blk-iolatency controller measures the time from rq_qos_throttle() to rq_qos_done_bio() and attributes this time to the first bio that needs to create the request. This means if a bio is plug-mergeable or bio-mergeable, it gets to bypass the blk-iolatency controller. The recent series [1], to tag all bios w/ blkgs undermined how iolatency was determining which bios it was charging and should process in rq_qos_done_bio(). Because all bios are being tagged, this caused the atomic_t for the struct rq_wait inflight count to underflow and result in a stall. This patch adds a new flag BIO_TRACKED to let controllers know that a bio is going through the rq_qos path. blk-iolatency now checks if this flag is set to see if it should process the bio in rq_qos_done_bio(). Overloading BLK_QUEUE_ENTERED works, but makes the flag rules confusing. BIO_THROTTLED was another candidate, but the flag is set for all bios that have gone through blk-throttle code. Overloading a flag comes with the burden of making sure that when either implementation changes, a change in setting rules for one doesn't cause a bug in the other. So here, we unfortunately opt for adding a new flag. [1] https://lore.kernel.org/lkml/20181205171039.73066-1-dennis@kernel.org/ Fixes: 5cdf2e3fea5e ("blkcg: associate blkg when associating a device") Signed-off-by: Dennis Zhou <dennis@kernel.org> Cc: Josef Bacik <josef@toxicpanda.com> Signed-off-by: Jens Axboe <axboe@kernel.dk> Conflicts: block/blk-iolatency.c block/blk-rq-qos.c Signed-off-by: Yu Kuai <yukuai3@huawei.com> Reviewed-by: Jason Yan <yanaijie@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- block/blk-iolatency.c | 2 +- block/blk-rq-qos.c | 6 ++++++ include/linux/blk_types.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index 0529e94a20f7f..ae6ae7f50c6f7 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -563,7 +563,7 @@ static void blkcg_iolatency_done_bio(struct rq_qos *rqos, struct bio *bio) int inflight = 0; blkg = bio->bi_blkg; - if (!blkg) + if (!blkg || !bio_flagged(bio, BIO_TRACKED)) return; iolat = blkg_to_lat(bio->bi_blkg); diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c index 43bcd4e7a7f9a..d1eaa118ce309 100644 --- a/block/blk-rq-qos.c +++ b/block/blk-rq-qos.c @@ -72,6 +72,12 @@ void rq_qos_throttle(struct request_queue *q, struct bio *bio, { struct rq_qos *rqos; + /* + * BIO_TRACKED lets controllers know that a bio went through the + * normal rq_qos path. + */ + bio_set_flag(bio, BIO_TRACKED); + for(rqos = q->rq_qos; rqos; rqos = rqos->next) { if (rqos->ops->throttle) rqos->ops->throttle(rqos, bio, lock); diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index c07caa2a28429..8075b9955bb3c 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -233,6 +233,7 @@ struct bio { #define BIO_TRACE_COMPLETION 10 /* bio_endio() should trace the final completion * of this bio. */ #define BIO_QUEUE_ENTERED 11 /* can use blk_queue_enter_live() */ +#define BIO_TRACKED 12 /* set if bio goes through the rq_qos path */ /* See BVEC_POOL_OFFSET below before adding new flags */ -- 2.25.1

From: Ming Lei <ming.lei@redhat.com> mainline inclusion from mainline-v5.15-rc3 commit a647a524a46736786c95cdb553a070322ca096e3 category: bugfix bugzilla: 182234 CVE: NA --------------------------- rq_qos framework is only applied on request based driver, so: 1) rq_qos_done_bio() needn't to be called for bio based driver 2) rq_qos_done_bio() needn't to be called for bio which isn't tracked, such as bios ended from error handling code. Especially in bio_endio(): 1) request queue is referred via bio->bi_bdev->bd_disk->queue, which may be gone since request queue refcount may not be held in above two cases 2) q->rq_qos may be freed in blk_cleanup_queue() when calling into __rq_qos_done_bio() Fix the potential kernel panic by not calling rq_qos_ops->done_bio if the bio isn't tracked. This way is safe because both ioc_rqos_done_bio() and blkcg_iolatency_done_bio() are nop if the bio isn't tracked. Reported-by: Yu Kuai <yukuai3@huawei.com> Cc: tj@kernel.org Signed-off-by: Ming Lei <ming.lei@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Tejun Heo <tj@kernel.org> Link: https://lore.kernel.org/r/20210924110704.1541818-1-ming.lei@redhat.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Conflict: block/bio.c Signed-off-by: Yu Kuai <yukuai3@huawei.com> Reviewed-by: Jason Yan <yanaijie@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- block/bio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/bio.c b/block/bio.c index da05350dfba28..d94243411ef30 100644 --- a/block/bio.c +++ b/block/bio.c @@ -1768,7 +1768,7 @@ void bio_endio(struct bio *bio) if (!bio_integrity_endio(bio)) return; - if (bio->bi_disk) + if (bio->bi_disk && bio_flagged(bio, BIO_TRACKED)) rq_qos_done_bio(bio->bi_disk->queue, bio); /* -- 2.25.1
participants (1)
-
Yang Yingliang