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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 13 Jul 2023 07:58:24 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Chengming Zhou <chengming.zhou@...ux.dev>
Cc:     Jens Axboe <axboe@...nel.dk>, hch@....de,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        ming.lei@...hat.com, zhouchengming@...edance.com
Subject: Re: [PATCH v5] blk-mq: fix start_time_ns and alloc_time_ns for
 pre-allocated rq

Hello,

On Thu, Jul 13, 2023 at 08:25:50PM +0800, Chengming Zhou wrote:
> Ok, this version will only get time stamp once for one request, it's actually
> not worse than the current code, which will get start time stamp once for each
> request even in the batch allocation.
> 
> But yes, maybe we can also set the start time stamp in the batch mode, and only
> update the time stamp in the block case, like you said, has better performance.
> 
> The first version [1] I posted actually just did this, in which use a nr_flush counter
> in plug to indicate that we blocked & flushed plug. Tejun and I think it seems fragile.
> So go to this way that only set time stamp once when the request actually used.
> 
> [1] https://lore.kernel.org/all/20230601053919.3639954-1-chengming.zhou@linux.dev/
> 
> Another way I can think of is to make rq_qos_throttle() return a bool to indicate
> if it blocked. Tejun and Jens, how do you think about this way?
> 
> Although it's better performance, in case of preemption, the time stamp maybe not accurate.

Trying to manually optimized timestamp reads seems like a bit of fool's
errand to me. I don't think anyone cares about nanosec accuracy, so there
are ample opportunities for generically caching timestamp so that we don't
have to contort code to optimzie timestamp calls.

It's a bit out of scope for this patchset but I think it might make sense to
build a timestamp caching infrastructure. The cached timestamp can be
invalidated on context switches (block layer already hooks into them) and
issue and other path boundaries (e.g. at the end of plug flush).

Thanks.

-- 
tejun

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ