linux-kernel - Re: [PATCH 10/15] checkpatch: Remove broken sleep/delay related checks

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]

Message-ID: <877cbq5xw7.fsf@somnus>
Date: Thu, 05 Sep 2024 10:04:56 +0200
From: Anna-Maria Behnsen <anna-maria@...utronix.de>
To: Joe Perches <joe@...ches.com>, Frederic Weisbecker
 <frederic@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Jonathan
 Corbet <corbet@....net>
Cc: linux-kernel@...r.kernel.org, Len Brown <len.brown@...el.com>, "Rafael
 J. Wysocki" <rafael@...nel.org>, Andy Whitcroft <apw@...onical.com>,
 Dwaipayan Ray <dwaipayanray1@...il.com>
Subject: Re: [PATCH 10/15] checkpatch: Remove broken sleep/delay related checks

Joe Perches <joe@...ches.com> writes:

> On Wed, 2024-09-04 at 15:05 +0200, Anna-Maria Behnsen wrote:
>> 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().
> []
>> - Check: ($1 < 20)
>>   Message: "msleep < 20ms can sleep for up to 20ms;
>>             see Documentation/timers/timers-howto.rst\n"
>>   Why is the check broken: The message is simply wrong. msleep() will not
>>                            sleep (and never did it before)
>
> While it might have changed, the "never did it before" is
> not correct.
>
> https://lore.kernel.org/all/15327.1186166232@lwn.net/

Thanks for the fast reply!

Yes you are right. I wasn't able to read properly and had in my mind
that it says that msleep will sleep at least 20ms when I was writing
this "never did it before" - my fault.

The point I want to make here is that those 20ms cannot be hard coded,
as this is HZ dependent (same thing as it was before). I will rephrase
it to:

      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. Remove also no longer required checkpatch
>> documentation section about USLEEP_RANGE.
>
> It'd be useful to remove the appropriate now unnecessary
> verbose sections from Documentation/dev-tools/checkpatch.rst

Maybe my coffee does not do it's job yet - which part should I remove? I
already remove the USLEEP_RANGE part. For MSLEEP and LONG_UDELAY there
wasn't anything documented.

Or should I rephrase the commit message somehow?

Thanks,

	Anna-Maria