| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 23 Dec 2022 14:47:06 +0000
From: Gulam Mohamed <gulam.mohamed@...cle.com>
To: Keith Busch <kbusch@...nel.org>
CC: "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
"axboe@...nel.dk" <axboe@...nel.dk>,
"philipp.reisner@...bit.com" <philipp.reisner@...bit.com>,
"lars.ellenberg@...bit.com" <lars.ellenberg@...bit.com>,
"christoph.boehmwalder@...bit.com" <christoph.boehmwalder@...bit.com>,
"minchan@...nel.org" <minchan@...nel.org>,
"ngupta@...are.org" <ngupta@...are.org>,
"senozhatsky@...omium.org" <senozhatsky@...omium.org>,
"colyli@...e.de" <colyli@...e.de>,
"kent.overstreet@...il.com" <kent.overstreet@...il.com>,
"agk@...hat.com" <agk@...hat.com>,
"snitzer@...nel.org" <snitzer@...nel.org>,
"dm-devel@...hat.com" <dm-devel@...hat.com>,
"song@...nel.org" <song@...nel.org>,
"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
"vishal.l.verma@...el.com" <vishal.l.verma@...el.com>,
"dave.jiang@...el.com" <dave.jiang@...el.com>,
"ira.weiny@...el.com" <ira.weiny@...el.com>,
Junxiao Bi <junxiao.bi@...cle.com>,
Martin Petersen <martin.petersen@...cle.com>,
"kch@...dia.com" <kch@...dia.com>,
"drbd-dev@...ts.linbit.com" <drbd-dev@...ts.linbit.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-bcache@...r.kernel.org" <linux-bcache@...r.kernel.org>,
"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
"nvdimm@...ts.linux.dev" <nvdimm@...ts.linux.dev>,
Konrad Wilk <konrad.wilk@...cle.com>,
Joe Jin <joe.jin@...cle.com>,
Rajesh Sivaramasubramaniom
<rajesh.sivaramasubramaniom@...cle.com>,
Shminderjit Singh <shminderjit.singh@...cle.com>
Subject: RE: [PATCH for-6.2/block V3 2/2] block: Change the granularity of io
ticks from ms to ns
Hi Keith,
Thanks for reviewing this request. Can you please see my inline comments?
Regards,
Gulam Mohamed.
-----Original Message-----
From: Keith Busch <kbusch@...nel.org>
Sent: Wednesday, December 21, 2022 10:39 PM
To: Gulam Mohamed <gulam.mohamed@...cle.com>
Cc: linux-block@...r.kernel.org; axboe@...nel.dk; philipp.reisner@...bit.com; lars.ellenberg@...bit.com; christoph.boehmwalder@...bit.com; minchan@...nel.org; ngupta@...are.org; senozhatsky@...omium.org; colyli@...e.de; kent.overstreet@...il.com; agk@...hat.com; snitzer@...nel.org; dm-devel@...hat.com; song@...nel.org; dan.j.williams@...el.com; vishal.l.verma@...el.com; dave.jiang@...el.com; ira.weiny@...el.com; Junxiao Bi <junxiao.bi@...cle.com>; Martin Petersen <martin.petersen@...cle.com>; kch@...dia.com; drbd-dev@...ts.linbit.com; linux-kernel@...r.kernel.org; linux-bcache@...r.kernel.org; linux-raid@...r.kernel.org; nvdimm@...ts.linux.dev; Konrad Wilk <konrad.wilk@...cle.com>; Joe Jin <joe.jin@...cle.com>; Rajesh Sivaramasubramaniom <rajesh.sivaramasubramaniom@...cle.com>; Shminderjit Singh <shminderjit.singh@...cle.com>
Subject: Re: [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.
[GULAM]: As per my understanding, this may work for a single request_queue implemetation. But this request based accounting, as per my understanding, may be an issue with the Multi-QUEUE as there is a separate queue for each CPU and the time-stamp being recorded for the block device is a global one. Also, the issue you mentioned about the start and end accounting may update the ticks in different
units for the inflight IOs, may be just for a while. So, even if it works for MQ, I am trying to understand how much is it feasible to do this request-based change for an issue which may be there for just a moment?
So, can you please correct me if I am wrong and explore more on your suggestion so that I can understand properly?
> @@ -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?
[GULAM]: I am not sure if this will work as we just multiply with 1000000 to convert jiffies to nano-seconds.
Powered by blists - more mailing lists