| 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: <Zw56n9P0Io1bMb0y@localhost.localdomain>
Date: Tue, 15 Oct 2024 16:22:23 +0200
From: Frederic Weisbecker <frederic@...nel.org>
To: Anna-Maria Behnsen <anna-maria@...utronix.de>
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
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?
Thanks.
> - }
> - }
> -
> # check for naked sscanf
> if ($perl_version_ok &&
> defined $stat &&
>
> --
> 2.39.5
>
Powered by blists - more mailing lists