Re: [PATCH 10/15] checkpatch: Remove broken sleep/delay related checks
From: Anna-Maria Behnsen
Date: Thu Sep 05 2024 - 04:05:06 EST
Joe Perches <joe@xxxxxxxxxxx> 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@xxxxxxx/
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