Re: [PATCH v2 05/15] timers: Update function descriptions of sleep/delay related functions

[Frederic Weisbecker <frederic@kernel.org>]

Le Fri, Oct 11, 2024 at 12:20:20PM +0200, Anna-Maria Behnsen a écrit :
> Frederic Weisbecker <frederic@kernel.org> writes:
> > However this doesn't need a second to apply. It only takes crossing levels above
> > 0. Or am I missing something?
> 
> s/the second/level 1/
> 
> more clear? Then it's the same number as used in the timer wheel
> documentation.

Oh right! I was confused with second (s) and 2nd. Yes much better.

> 
> >>  *
> >>  * The slack of timers which will end up in the first level depends on:
> >>  *
> 
> Same here: s/the first level/level 0/

Much better !

> 
> >>  * * sleep duration (msecs)
> >>  * * HZ configuration
> >>  *
> >>  * To make sure the sleep duration with the slack is accurate enough, a slack
> >>  * value is required (because of the design of the timer wheel it is not
> >
> > But where is it required?
> 
> The callsite has to decide which accuracy/slack is required for their
> use case (this was also part of the discussion which leads to this
> queue).
> 
> >>  * possible to define a value smaller than 12.5%). The following check makes
> >>  * clear, whether the sleep duration with the defined slack and with the HZ
> >>  * configuration will meet the constraints:
> >>  *
> >>  *  ``msecs >= (MSECS_PER_TICK / slack)``
> >>  *
> >>  * Examples:
> >>  *
> >>  * * ``HZ=1000`` with ``slack=25%``: ``MSECS_PER_TICK / slack = 1 / (1/4) = 4``:
> >>  *   all sleep durations greater or equal 4ms will meet the constraints.
> >>  * * ``HZ=1000`` with ``slack=12.5%``: ``MSECS_PER_TICK / slack = 1 / (1/8) = 8``:
> >>  *   all sleep durations greater or equal 8ms will meet the constraints.
> >>  * * ``HZ=250`` with ``slack=25%``: ``MSECS_PER_TICK / slack = 4 / (1/4) = 16``:
> >>  *   all sleep durations greater or equal 16ms will meet the constraints.
> >>  * * ``HZ=250`` with ``slack=12.5%``: ``MSECS_PER_TICK / slack = 4 / (1/8) = 32``:
> >>  *   all sleep durations greater or equal 32ms will meet the constraints.
> >
> > But who defines those slacks and where? I'm even more confused now...
> 
> I think I know where the confusion comes from. I rephrase it once more
> and turned around the calculation:
> 
> /**
>  * msleep - sleep safely even with waitqueue interruptions
>  * @msecs:	Requested sleep duration in milliseconds
>  *
>  * msleep() uses jiffy based timeouts for the sleep duration. Because of the
>  * design of the timer wheel, the maximum additional percentage delay (slack) is
>  * 12.5%. This is only valid for timers which will end up in level 1 or a
>  * higher level of the timer wheel. For explanation of those 12.5% please check
>  * the detailed description about the basics of the timer wheel.
>  *
>  * The slack of timers which will end up in level 0 depends on sleep
>  * duration (msecs) and HZ configuration and can be calculated in the
>  * following way (with the timer wheel design restriction that the slack
>  * is not less than 12.5%):
>  *
>  *   ``slack = MSECS_PER_TICK / msecs``
>  *
>  * When the allowed slack of the callsite is known, the calculation
>  * could be turned around to find the minimal allowed sleep duration to meet
>  * the constraints. For example:
>  *
>  * * ``HZ=1000`` with ``slack=25%``: ``MSECS_PER_TICK / slack = 1 / (1/4) = 4``:
>  *   all sleep durations greater or equal 4ms will meet the constraints.
>  * * ``HZ=1000`` with ``slack=12.5%``: ``MSECS_PER_TICK / slack = 1 / (1/8) = 8``:
>  *   all sleep durations greater or equal 8ms will meet the constraints.
>  * * ``HZ=250`` with ``slack=25%``: ``MSECS_PER_TICK / slack = 4 / (1/4) = 16``:
>  *   all sleep durations greater or equal 16ms will meet the constraints.
>  * * ``HZ=250`` with ``slack=12.5%``: ``MSECS_PER_TICK / slack = 4 / (1/8) = 32``:
>  *   all sleep durations greater or equal 32ms will meet the constraints.
>  *
>  * See also the signal aware variant msleep_interruptible().
>  */
> 
> Hopefully this attempt clarifies the confusion?
> 

Yes now it's perfectly clear!

Please add: Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

Thanks.