linux-kernel - Re: [PATCH v2 00/21] Converge on using secs_to

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] [day] [month] [year] [list]

Message-ID: <3113d4f4-4f8b-41d6-af89-f82cbc89da75@intel.com>
Date: Mon, 9 Dec 2024 16:03:31 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>, Easwar Hariharan
	<eahariha@...ux.microsoft.com>
CC: <netfilter-devel@...r.kernel.org>, <coreteam@...filter.org>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-s390@...r.kernel.org>,
	<dri-devel@...ts.freedesktop.org>, <intel-xe@...ts.freedesktop.org>,
	<linux-scsi@...r.kernel.org>, <xen-devel@...ts.xenproject.org>,
	<linux-block@...r.kernel.org>, <linux-wireless@...r.kernel.org>,
	<ath11k@...ts.infradead.org>, <linux-mm@...ck.org>,
	<linux-bluetooth@...r.kernel.org>, <linux-staging@...ts.linux.dev>,
	<ceph-devel@...r.kernel.org>, <live-patching@...r.kernel.org>,
	<linux-sound@...r.kernel.org>, <etnaviv@...ts.freedesktop.org>,
	<oss-drivers@...igine.com>, <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH v2 00/21] Converge on using secs_to_jiffies()

On 12/9/24 1:45 PM, Christophe Leroy wrote:
> 
> 
> Le 09/12/2024 à 13:01, Przemek Kitszel a écrit :
>> On 12/6/24 9:58 PM, Easwar Hariharan wrote:
>>> On 11/29/2024 4:57 AM, Przemek Kitszel wrote:
>>>>
>>>> [removed most non-list recipients, it's just too much]
>>>>
>>>> On 11/15/24 10:26 PM, Easwar Hariharan wrote:
>>> <snip>
>>
>>>>
>>>> Regarding code - you could also convert msecs_to_jiffies(const * HZ),
>>>> there are 10 that are greppable.

Thanks to Christope, I re-examined those ~10 cases, and that should be
refactored by just dropping msec_to_jiffies() part, not replacing
by sec_to_jiffies().

>>>>
>>>
>>> Those seem to be mistakes. const*HZ is a seconds-denominated timeout,
>>> being passed to msecs_to_jiffies() which will treat it as a
>>> millisecond-denominated timeout resulting in an excessively long
>>> timeout. I suppose that's better than a too-short timeout, and
>>> apparently it's been working fine all along since hardware responds
>>> before the too-long timeout expires. Half of them are in
>>> drivers/scsi/arcmsr/arcmsr_hba.c and the pattern has apparently been
>>> there since 2010.
>>
>> my point was that, the default value of HZ is 1000, and most of the code
>> that is just `$value*HZ` was meant as "$value seconds, in ms unit".
> 
> I can't follow you here. The default value of HZ is 250 as far as I can 
> see.

as default I understand "the value that is effective for those that
don't tweak", not necessarily "the fallback that will be used when not
set by any other means". On my RedHat and Fedora boxes it's 1000.

> 
> Regardless, HZ is the number of jiffies per second, nothing else.

That is true. But the name is wrong.

> 
>>
>> Same for HZ/const, HZ/2 being 500ms.
>>
>> HZ is awful in that it is not 1s but 1/s, but it was easy to abuse the
>> value in simple context.
> 
> Why is that awful ?

so, 1Hertz = 1/1s == once per second,
something happening twice per second, with freq of 2Hz, repeats
each HZ/2 jiffies
https://en.wikipedia.org/wiki/Hertz

the #define name should be really JHZ -> JIFFIES_PER_SECOND

> 
> HZ is a nice macro that gives you the number of ticks per second, so 
> that you are able to easily calculate the number of ticks for a given 
> duration, regardless of the configured number of ticks per second.

Again, technically true, but default being eq to number of msec in sec,
causes it to bite.