On Tue, Jan 25, 2022 at 08:19:06AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 25, 2022 at 07:16:34AM +0100, Christoph Hellwig wrote:
> > So why not key off accouning off "rq->bio && rq->bio->bi_bdev"
> > and remove the need for the flag and the second half of the assignment
> > above? That is much less error probe and removes code size.
>
> Something like this, lightly tested:
>
> ---
> From 5499d013341b492899d1fecde7680ff8ebd232e9 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@xxxxxx>
> Date: Tue, 25 Jan 2022 07:29:06 +0100
> Subject: block: remove the part field from struct request
>
> All file system I/O and most userspace passthrough bios have bi_bdev set.
> Switch I/O accounting to directly use the bio and stop copying it into a
> separate struct request field.
>
> This changes behavior in that e.g. /dev/sgX requests are not accounted
> to the gendisk for the SCSI disk any more, which is the correct thing to
> do as they never went through that gendisk, and fixes a potential race
> when the disk driver is unbound while /dev/sgX I/O is in progress.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> block/blk-merge.c | 12 ++++++------
> block/blk-mq.c | 32 +++++++++++++-------------------
> block/blk.h | 6 +++---
> include/linux/blk-mq.h | 1 -
> 4 files changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 4de34a332c9fd..43e46ea2f0152 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -739,11 +739,11 @@ void blk_rq_set_mixed_merge(struct request *rq)
>
> static void blk_account_io_merge_request(struct request *req)
> {
> - if (blk_do_io_stat(req)) {
> - part_stat_lock();
> - part_stat_inc(req->part, merges[op_stat_group(req_op(req))]);
> - part_stat_unlock();
> - }
> + if (!blk_do_io_stat(req))
> + return;
> + part_stat_lock();
> + part_stat_inc(req->bio->bi_bdev, merges[op_stat_group(req_op(req))]);
> + part_stat_unlock();
> }
>
> static enum elv_merge blk_try_req_merge(struct request *req,
> @@ -947,7 +947,7 @@ static void blk_account_io_merge_bio(struct request *req)
> return;
>
> part_stat_lock();
> - part_stat_inc(req->part, merges[op_stat_group(req_op(req))]);
> + part_stat_inc(req->bio->bi_bdev, merges[op_stat_group(req_op(req))]);
> part_stat_unlock();
> }
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index f3bf3358a3bb2..01b3862347965 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -132,10 +132,12 @@ static bool blk_mq_check_inflight(struct request *rq, void *priv,
> {
> struct mq_inflight *mi = priv;
>
> - if ((!mi->part->bd_partno || rq->part == mi->part) &&
> - blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
> - mi->inflight[rq_data_dir(rq)]++;
> + if (blk_mq_rq_state(rq) != MQ_RQ_IN_FLIGHT)
> + return true;
> + if (mi->part->bd_partno && rq->bio && rq->bio->bi_bdev != mi->part)
> + return true;
>
> + mi->inflight[rq_data_dir(rq)]++;
> return true;
> }
>
> @@ -331,7 +333,6 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
> rq->tag = BLK_MQ_NO_TAG;
> rq->internal_tag = BLK_MQ_NO_TAG;
> rq->start_time_ns = ktime_get_ns();
> - rq->part = NULL;
> blk_crypto_rq_set_defaults(rq);
> }
> EXPORT_SYMBOL(blk_rq_init);
> @@ -368,7 +369,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
> rq->start_time_ns = ktime_get_ns();
> else
> rq->start_time_ns = 0;
> - rq->part = NULL;
> #ifdef CONFIG_BLK_RQ_ALLOC_TIME
> rq->alloc_time_ns = alloc_time_ns;
> #endif
> @@ -687,11 +687,11 @@ static void req_bio_endio(struct request *rq, struct bio *bio,
>
> static void blk_account_io_completion(struct request *req, unsigned int bytes)
> {
> - if (req->part && blk_do_io_stat(req)) {
> + if (blk_do_io_stat(req)) {
> const int sgrp = op_stat_group(req_op(req));
>
> part_stat_lock();
> - part_stat_add(req->part, sectors[sgrp], bytes >> 9);
> + part_stat_add(req->bio->bi_bdev, sectors[sgrp], bytes >> 9);
> part_stat_unlock();
> }
> }
> @@ -859,11 +859,12 @@ EXPORT_SYMBOL_GPL(blk_update_request);
> static void __blk_account_io_done(struct request *req, u64 now)
> {
> const int sgrp = op_stat_group(req_op(req));
> + struct block_device *bdev = req->bio->bi_bdev;
Then you need to move account_io_done before calling blk_update_request().
thanks,
Ming
