Re: [PATCH v2 07/15] timers: Adjust flseep() to reflect reality

[Frederic Weisbecker]

Le Wed, Sep 11, 2024 at 07:13:33AM +0200, Anna-Maria Behnsen a écrit :
> fsleep() simply implements the recommondations of the outdated

recommendations*

> documentation in "Documentation/timers/timers-howto.rst". This should be a
> user friendly interface to choose always the best timeout function
> approach:
> 
> - udelay() for very short sleep durations shorter than 10 microseconds
> - usleep_range() for sleep durations until 20 milliseconds
> - msleep() for the others
> 
> The actual implementation has several problems:
> 
> - It does not take into account that HZ resolution also has an impact on
>   granularity of jiffies and has also an impact on the granularity of the
>   buckets of timer wheel levels. This means that accuracy for the timeout
>   does not have an upper limit. When executing fsleep(20000) on a HZ=100
>   system, the possible additional slack will be 50% as the granularity of
>   the buckets in the lowest level is 10 milliseconds.
> 
> - The upper limit of usleep_range() is twice the requested timeout. When no
>   other interrupts occur in this range, the maximum value is used. This
>   means that the requested sleep length has then an additional delay of
>   100%.
> 
> Change the thresholds for the decisions in fsleep() to make sure the
> maximum slack which is added to the sleep duration is 25%.
> 
> Note: Outdated documentation will be updated in a followup patch.
> 
> Cc: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> Cc: David S. Miller <davem@xxxxxxxxxxxxx>
> Signed-off-by: Anna-Maria Behnsen <anna-maria@xxxxxxxxxxxxx>
> ---
>  include/linux/delay.h | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/delay.h b/include/linux/delay.h
> index 23623fa79768..b49a63c85c43 100644
> --- a/include/linux/delay.h
> +++ b/include/linux/delay.h
> @@ -11,6 +11,7 @@
>  
>  #include <linux/math.h>
>  #include <linux/sched.h>
> +#include <linux/jiffies.h>
>  
>  extern unsigned long loops_per_jiffy;
>  
> @@ -102,15 +103,35 @@ static inline void ssleep(unsigned int seconds)
>  	msleep(seconds * 1000);
>  }
>  
> -/* see Documentation/timers/timers-howto.rst for the thresholds */
> +static const unsigned int max_slack_shift = 2;

Should it be a define? (such const variables are usually for arrays).

Anyway:

Reviewed-by: Frederic Weisbecker <frederic@xxxxxxxxxx>