lkml.org 
[lkml]   [2022]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH for-6.2/block V3 2/2] block: Change the granularity of io ticks from ms to ns
On Wed, Dec 21, 2022 at 04:05:06AM +0000, Gulam Mohamed wrote:
> +u64 blk_get_iostat_ticks(struct request_queue *q)
> +{
> + return (blk_queue_precise_io_stat(q) ? ktime_get_ns() : jiffies);
> +}
> +EXPORT_SYMBOL_GPL(blk_get_iostat_ticks);
> +
> void update_io_ticks(struct block_device *part, u64 now, bool end)
> {
> u64 stamp;
> @@ -968,20 +980,26 @@ EXPORT_SYMBOL(bdev_start_io_acct);
> u64 bio_start_io_acct(struct bio *bio)
> {
> return bdev_start_io_acct(bio->bi_bdev, bio_sectors(bio),
> - bio_op(bio), jiffies);
> + bio_op(bio),
> + blk_get_iostat_ticks(bio->bi_bdev->bd_queue));
> }
> EXPORT_SYMBOL_GPL(bio_start_io_acct);
>
> void bdev_end_io_acct(struct block_device *bdev, enum req_op op,
> u64 start_time)
> {
> + u64 now;
> + u64 duration;
> + struct request_queue *q = bdev_get_queue(bdev);
> const int sgrp = op_stat_group(op);
> - u64 now = READ_ONCE(jiffies);
> - u64 duration = (unsigned long)now -(unsigned long) start_time;
> + now = blk_get_iostat_ticks(q);;

I don't think you can rely on the blk_queue_precise_io_stat() flag in
the completion side. The user can toggle this with data in flight, which
means the completion may use different tick units than the start. I
think you'll need to add a flag to the request in the start accounting
side to know which method to use for the completion.

> @@ -951,6 +951,7 @@ ssize_t part_stat_show(struct device *dev,
> struct request_queue *q = bdev_get_queue(bdev);
> struct disk_stats stat;
> unsigned int inflight;
> + u64 stat_ioticks;
>
> if (queue_is_mq(q))
> inflight = blk_mq_in_flight(q, bdev);
> @@ -959,10 +960,13 @@ ssize_t part_stat_show(struct device *dev,
>
> if (inflight) {
> part_stat_lock();
> - update_io_ticks(bdev, jiffies, true);
> + update_io_ticks(bdev, blk_get_iostat_ticks(q), true);
> part_stat_unlock();
> }
> part_stat_read_all(bdev, &stat);
> + stat_ioticks = (blk_queue_precise_io_stat(q) ?
> + div_u64(stat.io_ticks, NSEC_PER_MSEC) :
> + jiffies_to_msecs(stat.io_ticks));


With the user able to change the precision at will, I think these
io_ticks need to have a consistent unit size. Mixing jiffies and nsecs
is going to create garbage stats output. Could existing io_ticks using
jiffies be converted to jiffies_to_nsecs() so that you only have one
unit to consider?

\
 
 \ /
  Last update: 2023-03-26 23:17    [W:0.163 / U:1.952 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site