| 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
| ||
|
Message-ID: <87y12ocqyo.fsf@somnus>
Date: Wed, 16 Oct 2024 11:55:11 +0200
From: Anna-Maria Behnsen <anna-maria@...utronix.de>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, Jonathan Corbet <corbet@....net>,
linux-kernel@...r.kernel.org, Len Brown <len.brown@...el.com>, "Rafael J.
Wysocki" <rafael@...nel.org>, rust-for-linux@...r.kernel.org, Alice Ryhl
<aliceryhl@...gle.com>, FUJITA Tomonori <fujita.tomonori@...il.com>,
Andrew Lunn <andrew@...n.ch>, Miguel Ojeda <ojeda@...nel.org>, Andy
Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>, Dwaipayan
Ray <dwaipayanray1@...il.com>
Subject: Re: [PATCH v3 16/16] checkpatch: Remove broken sleep/delay related
checks
Frederic Weisbecker <frederic@...nel.org> writes:
> Le Mon, Oct 14, 2024 at 10:22:33AM +0200, Anna-Maria Behnsen a écrit :
>> checkpatch.pl checks for several things related to sleep and delay
>> functions. In all warnings the outdated documentation is referenced. All
>> broken parts are listed one by one in the following with an explanation why
>> this check is broken. For a basic background of those functions please also
>> refere to the updated function descriptions of udelay(), nsleep_range() and
>> msleep().
>>
>> Be aware: The change is done with a perl knowledge of the level "I'm able
>> to spell perl".
>>
>> The following checks are broken:
>>
>> - Check: (! ($delay < 10) )
>> Message: "usleep_range is preferred over udelay;
>> see Documentation/timers/timers-howto.rst\n"
>> Why is the check broken: When it is an atomic context, udelay() is
>> mandatory.
>>
>> - Check: ($min eq $max)
>> Message: "usleep_range should not use min == max args;
>> see Documentation/timers/timers-howto.rst\n"
>> Why is the check broken: When the requested accuracy for the sleep
>> duration requires it, it is also valid to use
>> min == max.
>>
>> - Check: ($delay > 2000)
>> Message: "long udelay - prefer mdelay;
>> see arch/arm/include/asm/delay.h\n"
>> Why is the check broken: The threshold when to start using mdelay() to
>> prevent an overflow depends on
>> MAX_UDELAY_MS. This value is architecture
>> dependent. The used value for the check and
>> reference is arm specific. Generic would be 5ms,
>> but this would "break" arm, loongarch and mips
>> and also the arm value might "break" mips and
>> loongarch in some configurations.
>>
>> - Check: ($1 < 20)
>> Message: "msleep < 20ms can sleep for up to 20ms;
>> see Documentation/timers/timers-howto.rst\n"
>> Why is the check broken: msleep(1) might sleep up to 20ms but only on a
>> HZ=100 system. On a HZ=1000 system this will be
>> 2ms. This means, the threshold cannot be hard
>> coded as it depends on HZ (jiffy granularity and
>> timer wheel bucket/level granularity) and also
>> on the required accuracy of the callsite. See
>> msleep() and also the USLEEP_RANGE_UPPER_BOUND
>> value.
>>
>> Remove all broken checks. Update checkpatch documentation accordingly.
>>
>> Cc: Andy Whitcroft <apw@...onical.com>
>> Cc: Joe Perches <joe@...ches.com>
>> Cc: Dwaipayan Ray <dwaipayanray1@...il.com>
>> Signed-off-by: Anna-Maria Behnsen <anna-maria@...utronix.de>
>> ---
>> v3: Move it to the end of the queue and adapt it to the new patch which
>> removes the link to the outdated documentation before.
>> v2: Rephrase commit message
>> ---
>> Documentation/dev-tools/checkpatch.rst | 4 ----
>> scripts/checkpatch.pl | 38 ----------------------------------
>> 2 files changed, 42 deletions(-)
>>
>> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
>> index abb3ff682076..f5c27be9e673 100644
>> --- a/Documentation/dev-tools/checkpatch.rst
>> +++ b/Documentation/dev-tools/checkpatch.rst
>> @@ -466,10 +466,6 @@ API usage
>> **UAPI_INCLUDE**
>> No #include statements in include/uapi should use a uapi/ path.
>>
>> - **USLEEP_RANGE**
>> - usleep_range() should be preferred over udelay(). The proper way of
>> - using usleep_range() is mentioned in the kernel docs.
>> -
>>
>> Comments
>> --------
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 98790fe5115d..34d4b5beda29 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -6591,28 +6591,6 @@ sub process {
>> }
>> }
>>
>> -# prefer usleep_range over udelay
>> - if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) {
>> - my $delay = $1;
>> - # ignore udelay's < 10, however
>> - if (! ($delay < 10) ) {
>> - CHK("USLEEP_RANGE",
>> - "usleep_range is preferred over udelay; see function description of usleep_range() and udelay().\n" . $herecurr);
>> - }
>> - if ($delay > 2000) {
>> - WARN("LONG_UDELAY",
>> - "long udelay - prefer mdelay; see function description of mdelay().\n" . $herecurr);
>> - }
>> - }
>> -
>> -# warn about unexpectedly long msleep's
>> - if ($line =~ /\bmsleep\s*\((\d+)\);/) {
>> - if ($1 < 20) {
>> - WARN("MSLEEP",
>> - "msleep < 20ms can sleep for up to 20ms; see function description of msleep().\n" . $herecurr);
>> - }
>> - }
>> -
>> # check for comparisons of jiffies
>> if ($line =~ /\bjiffies\s*$Compare|$Compare\s*jiffies\b/) {
>> WARN("JIFFIES_COMPARISON",
>> @@ -7069,22 +7047,6 @@ sub process {
>> }
>> }
>>
>> -# check usleep_range arguments
>> - if ($perl_version_ok &&
>> - defined $stat &&
>> - $stat =~ /^\+(?:.*?)\busleep_range\s*\(\s*($FuncArg)\s*,\s*($FuncArg)\s*\)/) {
>> - my $min = $1;
>> - my $max = $7;
>> - if ($min eq $max) {
>> - WARN("USLEEP_RANGE",
>> - "usleep_range should not use min == max args; see function description of usleep_range().\n" . "$here\n$stat\n");
>> - } elsif ($min =~ /^\d+$/ && $max =~ /^\d+$/ &&
>> - $min > $max) {
>> - WARN("USLEEP_RANGE",
>> - "usleep_range args reversed, use min then max; see function description of usleep_range().\n" . "$here\n$stat\n");
>
> Why not keep the min > max static check?
I removed it accidentially... It was my plan to keep the min > max
static check.
I'll send a v4 for this patch.
Thanks,
Anna-Maria
Powered by blists - more mailing lists