block: count 'ios' and 'sectors' when io is done for bio-based device

BugLink: https://bugs.launchpad.net/bugs/2016876
[ Upstream commit 5f27571382ca42daa3e3d40d1b252bf18c2b61d2 ]

While using iostat for raid, I observed very strange 'await'
occasionally, and turns out it's due to that 'ios' and 'sectors' is
counted in bdev_start_io_acct(), while 'nsecs' is counted in
bdev_end_io_acct(). I'm not sure why they are ccounted like that
but I think this behaviour is obviously wrong because user will get
wrong disk stats.

Fix the problem by counting 'ios' and 'sectors' when io is done, like
what rq-based device does.

Fixes: 394ffa503bc4 ("blk: introduce generic io stat accounting help function")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20230223091226.1135678-1-yukuai1@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>

diff --git a/block/blk-core.c b/block/blk-core.c
index 5a0049215ee7279fde0e54cbc7f77f4ea069bc8b..597293151cd1198606edf44d720cd282ed266351 100644 (file)
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -946,16 +946,11 @@ again:
        }
 }
 
-unsigned long bdev_start_io_acct(struct block_device *bdev,
-                                unsigned int sectors, enum req_op op,
+unsigned long bdev_start_io_acct(struct block_device *bdev, enum req_op op,
                                 unsigned long start_time)
 {
-       const int sgrp = op_stat_group(op);
-
        part_stat_lock();
        update_io_ticks(bdev, start_time, false);
-       part_stat_inc(bdev, ios[sgrp]);
-       part_stat_add(bdev, sectors[sgrp], sectors);
        part_stat_local_inc(bdev, in_flight[op_is_write(op)]);
        part_stat_unlock();
 
@@ -971,13 +966,12 @@ EXPORT_SYMBOL(bdev_start_io_acct);
  */
 unsigned long bio_start_io_acct(struct bio *bio)
 {
-       return bdev_start_io_acct(bio->bi_bdev, bio_sectors(bio),
-                                 bio_op(bio), jiffies);
+       return bdev_start_io_acct(bio->bi_bdev, bio_op(bio), jiffies);
 }
 EXPORT_SYMBOL_GPL(bio_start_io_acct);
 
 void bdev_end_io_acct(struct block_device *bdev, enum req_op op,
-                     unsigned long start_time)
+                     unsigned int sectors, unsigned long start_time)
 {
        const int sgrp = op_stat_group(op);
        unsigned long now = READ_ONCE(jiffies);
@@ -985,6 +979,8 @@ void bdev_end_io_acct(struct block_device *bdev, enum req_op op,
 
        part_stat_lock();
        update_io_ticks(bdev, now, true);
+       part_stat_inc(bdev, ios[sgrp]);
+       part_stat_add(bdev, sectors[sgrp], sectors);
        part_stat_add(bdev, nsecs[sgrp], jiffies_to_nsecs(duration));
        part_stat_local_dec(bdev, in_flight[op_is_write(op)]);
        part_stat_unlock();
@@ -994,7 +990,7 @@ EXPORT_SYMBOL(bdev_end_io_acct);
 void bio_end_io_acct_remapped(struct bio *bio, unsigned long start_time,
                              struct block_device *orig_bdev)
 {
-       bdev_end_io_acct(orig_bdev, bio_op(bio), start_time);
+       bdev_end_io_acct(orig_bdev, bio_op(bio), bio_sectors(bio), start_time);
 }
 EXPORT_SYMBOL_GPL(bio_end_io_acct_remapped);
 

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index e290d6d970474eeba24d913a168242990a10c5f7..03ef03e10618da74ff757f13b2065e0feb7e78ce 100644 (file)
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2108,9 +2108,9 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
        bv.bv_offset = 0;
 
        start_time = bdev_start_io_acct(bdev->bd_disk->part0,
-                       SECTORS_PER_PAGE, op, jiffies);
+                       op, jiffies);
        ret = zram_bvec_rw(zram, &bv, index, offset, op, NULL);
-       bdev_end_io_acct(bdev->bd_disk->part0, op, start_time);
+       bdev_end_io_acct(bdev->bd_disk->part0, op, SECTORS_PER_PAGE, start_time);
 out:
        /*
         * If I/O fails, just return error(ie, non-zero) without

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 605662935ce9102ef28c64bdfc3d0c8597d12a1a..fdcf42554e2a9d422ed3a15f1c08d11fc7054c47 100644 (file)
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -510,10 +510,10 @@ static void dm_io_acct(struct dm_io *io, bool end)
                sectors = io->sectors;
 
        if (!end)
-               bdev_start_io_acct(bio->bi_bdev, sectors, bio_op(bio),
-                                  start_time);
+               bdev_start_io_acct(bio->bi_bdev, bio_op(bio), start_time);
        else
-               bdev_end_io_acct(bio->bi_bdev, bio_op(bio), start_time);
+               bdev_end_io_acct(bio->bi_bdev, bio_op(bio), sectors,
+                                start_time);
 
        if (static_branch_unlikely(&stats_enabled) &&
            unlikely(dm_stats_used(&md->stats))) {

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index fc39d01e7b63be8a4f9e794f915715cbc2c8609b..9171452e2f6d4e2eed95d0c6ef9ec0cbf05bc7ef 100644 (file)
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -123,9 +123,8 @@ void nvme_mpath_start_request(struct request *rq)
                return;
 
        nvme_req(rq)->flags |= NVME_MPATH_IO_STATS;
-       nvme_req(rq)->start_time = bdev_start_io_acct(disk->part0,
-                                       blk_rq_bytes(rq) >> SECTOR_SHIFT,
-                                       req_op(rq), jiffies);
+       nvme_req(rq)->start_time = bdev_start_io_acct(disk->part0, req_op(rq),
+                                                     jiffies);
 }
 EXPORT_SYMBOL_GPL(nvme_mpath_start_request);
 
@@ -136,7 +135,8 @@ void nvme_mpath_end_request(struct request *rq)
        if (!(nvme_req(rq)->flags & NVME_MPATH_IO_STATS))
                return;
        bdev_end_io_acct(ns->head->disk->part0, req_op(rq),
-               nvme_req(rq)->start_time);
+                        blk_rq_bytes(rq) >> SECTOR_SHIFT,
+                        nvme_req(rq)->start_time);
 }
 
 void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 43d4e073b1115e4628a001081fbf08b296d342df..c3e066242941d71e9d5a0e72e00b2391591e7c21 100644 (file)
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1434,11 +1434,10 @@ static inline void blk_wake_io_task(struct task_struct *waiter)
                wake_up_process(waiter);
 }
 
-unsigned long bdev_start_io_acct(struct block_device *bdev,
-                                unsigned int sectors, enum req_op op,
+unsigned long bdev_start_io_acct(struct block_device *bdev, enum req_op op,
                                 unsigned long start_time);
 void bdev_end_io_acct(struct block_device *bdev, enum req_op op,
-               unsigned long start_time);
+                     unsigned int sectors, unsigned long start_time);
 
 unsigned long bio_start_io_acct(struct bio *bio);
 void bio_end_io_acct_remapped(struct bio *bio, unsigned long start_time,