Instead of the jiffies, we should update the io_ticks counter
with the passed in parameter 'now'.
Signed-off-by: Wen Yang <[email protected]>
Reviewed-by: Joseph Qi <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Mikulas Patocka <[email protected]>
Cc: Mike Snitzer <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
v2->v1: Use the same clock source for io_ticks and other statistics in the diskstats
block/blk-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 379f6f5..da7de9f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1365,7 +1365,7 @@ void blk_account_io_done(struct request *req, u64 now)
part_stat_lock();
part = req->part;
- update_io_ticks(part, jiffies);
+ update_io_ticks(part, nsecs_to_jiffies(now));
part_stat_inc(part, ios[sgrp]);
part_stat_add(part, nsecs[sgrp], now - req->start_time_ns);
part_stat_add(part, time_in_queue, nsecs_to_jiffies64(now - req->start_time_ns));
@@ -1407,7 +1407,7 @@ void blk_account_io_start(struct request *rq, bool new_io)
rq->part = part;
}
- update_io_ticks(part, jiffies);
+ update_io_ticks(part, nsecs_to_jiffies(ktime_get_ns()));
part_stat_unlock();
}
--
1.8.3.1
On 12/25/19 8:10 PM, Wen Yang wrote:
> Instead of the jiffies, we should update the io_ticks counter
> with the passed in parameter 'now'.
I'm still missing some justification for this. What exactly is this
patch trying to solve or improve? Your commit message says "we should",
but why?
--
Jens Axboe
On 2019/12/26 11:39 上午, Jens Axboe wrote:
> On 12/25/19 8:10 PM, Wen Yang wrote:
>> Instead of the jiffies, we should update the io_ticks counter
>> with the passed in parameter 'now'.
>
> I'm still missing some justification for this. What exactly is this
> patch trying to solve or improve? Your commit message says "we should",
> but why?
>
Hi Jens,
Thank you for your comments.
We observed in the document that:
io_ticks
========
This value counts the number of milliseconds during which the device has
had I/O requests queued.
And the iostat command uses io_ticks count to calculate %util:
https://github.com/sysstat/sysstat/blob/master/rd_stats.c#L372
eg：
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
So we need to unify the time windows of these statistics（io_ticks,
rd_tick, time_in_queue, etc）.
However, the current code uses jiffies to count io_ticks.
Jiffies is different from the passed in parameter 'now',
so these statistics will be inconsistent：
void blk_account_io_done(struct request *req, u64 now)
{
…
update_io_ticks(part, jiffies);
part_stat_inc(part, ios[sgrp]);
part_stat_add(part, nsecs[sgrp], now - req->start_time_ns);
part_stat_add(part, time_in_queue, nsecs_to_jiffies64(now -
req->start_time_ns));
…
}
In addition, we also found another issue:
the update_io_tick() function only adds one to io_ticks at a time,
which will result in the calculated %util lower than the real one.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/bio.c#n1713
We will try our best to improve it.
please kindly help with some suggestions.
Thanks.
--
Best Regards,
Wen