[PATCH v2 00/15] timers: Cleanup delay/sleep related mess

[PATCH v2 00/15] timers: Cleanup delay/sleep related mess

[Anna-Maria Behnsen]

Hi,

a question about which sleeping function should be used in acpi_os_sleep()
started a discussion and examination about the existing documentation and
implementation of functions which insert a sleep/delay.

The result of the discussion was, that the documentation is outdated and
the implemented fsleep() reflects the outdated documentation but doesn't
help to reflect reality which in turns leads to the queue which covers the
following things:

- Split out all timeout and sleep related functions from hrtimer.c and timer.c
  into a separate file

- Update function descriptions of sleep related functions

- Change fsleep() to reflect reality

- Rework all comments or users which obviously rely on the outdated
  documentation as they reference "Documentation/timers/timers-howto.rst"

- Last but not least (as there are no more references): Update the outdated
  documentation and move it into a file with a self explaining file name

The queue is available here and applies on top of tip/timers/core:

  git://git.kernel.org/pub/scm/linux/kernel/git/anna-maria/linux-devel.git timers/misc

Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
---
Changes in v2:
- change udelay() and ndelay() as suggested by Thomas
- Update some formatting in the new sleep_timeout.c file
- minor typo changes and other small review remarks

Thanks,

        Anna-Maria

---
Anna-Maria Behnsen (15):
      MAINTAINERS: Add missing file include/linux/delay.h
      timers: Move *sleep*() and timeout functions into a separate file
      timers: Update schedule_[hr]timeout*() related function descriptions
      timers: Rename usleep_idle_range() to usleep_range_idle()
      timers: Update function descriptions of sleep/delay related functions
      delay: Rework udelay and ndelay
      timers: Adjust flseep() to reflect reality
      mm/damon/core: Use generic upper bound recommondation for usleep_range()
      timers: Add a warning to usleep_range_state() for wrong order of arguments
      checkpatch: Remove broken sleep/delay related checks
      regulator: core: Use fsleep() to get best sleep mechanism
      iopoll/regmap/phy/snd: Fix comment referencing outdated timer documentation
      powerpc/rtas: Use fsleep() to minimize additional sleep duration
      media: anysee: Fix link to outdated sleep function documentation
      timers/Documentation: Cleanup delay/sleep documentation

 Documentation/dev-tools/checkpatch.rst         |   6 -
 Documentation/timers/delay_sleep_functions.rst | 122 ++++++++
 Documentation/timers/index.rst                 |   2 +-
 Documentation/timers/timers-howto.rst          | 115 --------
 MAINTAINERS                                    |   2 +
 arch/powerpc/kernel/rtas.c                     |  21 +-
 drivers/media/usb/dvb-usb-v2/anysee.c          |   6 +-
 drivers/regulator/core.c                       |  47 +---
 include/asm-generic/delay.h                    |  95 +++++--
 include/linux/delay.h                          |  79 ++++--
 include/linux/iopoll.h                         |  52 ++--
 include/linux/phy.h                            |   9 +-
 include/linux/regmap.h                         |  38 +--
 kernel/time/Makefile                           |   2 +-
 kernel/time/hrtimer.c                          | 120 --------
 kernel/time/sleep_timeout.c                    | 376 +++++++++++++++++++++++++
 kernel/time/timer.c                            | 192 -------------
 mm/damon/core.c                                |   5 +-
 scripts/checkpatch.pl                          |  38 ---
 sound/soc/sof/ops.h                            |   8 +-
 20 files changed, 701 insertions(+), 634 deletions(-)

[PATCH v2 13/15] powerpc/rtas: Use fsleep() to minimize additional sleep duration

[Anna-Maria Behnsen]

When commit 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
was introduced, documentation about proper usage of sleep realted functions
was outdated.

The commit message references the usage of a HZ=100 system. When using a
20ms sleep duration on such a system and therefore using msleep(), the
possible additional slack will be +10ms.

When the system is configured with HZ=100 the granularity of a jiffy and of
a bucket of the lowest timer wheel level is 10ms. To make sure a timer will
not expire early (when queueing of the timer races with an concurrent
update of jiffies), timers are always queued into the next bucket. This is
the reason for the maximal possible slack of 10ms.

fsleep() limits the maximal possible slack to 25% by making threshold
between usleep_range() and msleep() HZ dependent. As soon as the accuracy
of msleep() is sufficient, the less expensive timer list timer based
sleeping function is used instead of the more expensive hrtimer based
usleep_range() function. The udelay() will not be used in this specific
usecase as the lowest sleep length is larger than 1 microsecond.

Use fsleep() directly instead of using an own heuristic for the best
sleeping mechanism to use..

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
---
v2: fix typos
---
 arch/powerpc/kernel/rtas.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index f7e86e09c49f..d31c9799cab2 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1390,21 +1390,14 @@ bool __ref rtas_busy_delay(int status)
 		 */
 		ms = clamp(ms, 1U, 1000U);
 		/*
-		 * The delay hint is an order-of-magnitude suggestion, not
-		 * a minimum. It is fine, possibly even advantageous, for
-		 * us to pause for less time than hinted. For small values,
-		 * use usleep_range() to ensure we don't sleep much longer
-		 * than actually needed.
-		 *
-		 * See Documentation/timers/timers-howto.rst for
-		 * explanation of the threshold used here. In effect we use
-		 * usleep_range() for 9900 and 9901, msleep() for
-		 * 9902-9905.
+		 * The delay hint is an order-of-magnitude suggestion, not a
+		 * minimum. It is fine, possibly even advantageous, for us to
+		 * pause for less time than hinted. To make sure pause time will
+		 * not be way longer than requested independent of HZ
+		 * configuration, use fsleep(). See fsleep() for details of
+		 * used sleeping functions.
 		 */
-		if (ms <= 20)
-			usleep_range(ms * 100, ms * 1000);
-		else
-			msleep(ms);
+		fsleep(ms * 1000);
 		break;
 	case RTAS_BUSY:
 		ret = true;

-- 
2.39.2

Re: [PATCH v2 00/15] timers: Cleanup delay/sleep related mess

[Christophe JAILLET]

Le 11/09/2024 à 07:13, Anna-Maria Behnsen a écrit :
> Hi,
> 
> a question about which sleeping function should be used in acpi_os_sleep()
> started a discussion and examination about the existing documentation and
> implementation of functions which insert a sleep/delay.
> 
> The result of the discussion was, that the documentation is outdated and
> the implemented fsleep() reflects the outdated documentation but doesn't
> help to reflect reality which in turns leads to the queue which covers the
> following things:
> 
> - Split out all timeout and sleep related functions from hrtimer.c and timer.c
>    into a separate file
> 
> - Update function descriptions of sleep related functions
> 
> - Change fsleep() to reflect reality
> 
> - Rework all comments or users which obviously rely on the outdated
>    documentation as they reference "Documentation/timers/timers-howto.rst"
> 
> - Last but not least (as there are no more references): Update the outdated
>    documentation and move it into a file with a self explaining file name
> 
> The queue is available here and applies on top of tip/timers/core:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/anna-maria/linux-devel.git timers/misc
> 
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>

Hi,

not directly related to your serie, but some time ago I sent a patch to 
micro-optimize Optimize usleep_range(). (See [1])

The idea is that the 2 parameters of usleep_range() are usually 
constants and some code reordering could easily let the compiler compute 
a few things at compilation time.

There was consensus on the value of the change (see [2]), but as you are 
touching things here, maybe it makes sense now to save a few cycles at 
runtime and a few bytes of code?

CJ

[1]: 
https://lore.kernel.org/all/f0361b83a0a0b549f8ec5ab8134905001a6f2509.1659126514.git.christophe.jaillet@wanadoo.fr/

[2]: 
https://lore.kernel.org/all/03c2bbe795fe4ddcab66eb852bae3715@AcuMS.aculab.com/

Re: [PATCH v2 00/15] timers: Cleanup delay/sleep related mess

[Christophe JAILLET]

Le 16/09/2024 à 22:20, Christophe JAILLET a écrit :
> Le 11/09/2024 à 07:13, Anna-Maria Behnsen a écrit :
>> Hi,
>>
>> a question about which sleeping function should be used in 
>> acpi_os_sleep()
>> started a discussion and examination about the existing documentation and
>> implementation of functions which insert a sleep/delay.
>>
>> The result of the discussion was, that the documentation is outdated and
>> the implemented fsleep() reflects the outdated documentation but doesn't
>> help to reflect reality which in turns leads to the queue which covers 
>> the
>> following things:
>>
>> - Split out all timeout and sleep related functions from hrtimer.c and 
>> timer.c
>>    into a separate file
>>
>> - Update function descriptions of sleep related functions
>>
>> - Change fsleep() to reflect reality
>>
>> - Rework all comments or users which obviously rely on the outdated
>>    documentation as they reference "Documentation/timers/timers- 
>> howto.rst"
>>
>> - Last but not least (as there are no more references): Update the 
>> outdated
>>    documentation and move it into a file with a self explaining file name
>>
>> The queue is available here and applies on top of tip/timers/core:
>>
>>    git://git.kernel.org/pub/scm/linux/kernel/git/anna-maria/linux- 
>> devel.git timers/misc
>>
>> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> 
> Hi,
> 
> not directly related to your serie, but some time ago I sent a patch to 
> micro-optimize Optimize usleep_range(). (See [1])
> 
> The idea is that the 2 parameters of usleep_range() are usually 
> constants and some code reordering could easily let the compiler compute 
> a few things at compilation time.
> 
> There was consensus on the value of the change (see [2]), but as you are 

Typo: there was *no* consensus...

> touching things here, maybe it makes sense now to save a few cycles at 
> runtime and a few bytes of code?
> 
> CJ
> 
> [1]: https://lore.kernel.org/all/ 
> f0361b83a0a0b549f8ec5ab8134905001a6f2509.1659126514.git.christophe.jaillet@wanadoo.fr/
> 
> [2]: https://lore.kernel.org/ 
> all/03c2bbe795fe4ddcab66eb852bae3715@AcuMS.aculab.com/
> 
> 
> 
> 

Re: [PATCH v2 00/15] timers: Cleanup delay/sleep related mess

[Anna-Maria Behnsen]

Christophe JAILLET <christophe.jaillet@wanadoo.fr> writes:

> Le 16/09/2024 à 22:20, Christophe JAILLET a écrit :
>> Le 11/09/2024 à 07:13, Anna-Maria Behnsen a écrit :
>>> Hi,
>>>
>>> a question about which sleeping function should be used in 
>>> acpi_os_sleep()
>>> started a discussion and examination about the existing documentation and
>>> implementation of functions which insert a sleep/delay.
>>>
>>> The result of the discussion was, that the documentation is outdated and
>>> the implemented fsleep() reflects the outdated documentation but doesn't
>>> help to reflect reality which in turns leads to the queue which covers 
>>> the
>>> following things:
>>>
>>> - Split out all timeout and sleep related functions from hrtimer.c and 
>>> timer.c
>>>    into a separate file
>>>
>>> - Update function descriptions of sleep related functions
>>>
>>> - Change fsleep() to reflect reality
>>>
>>> - Rework all comments or users which obviously rely on the outdated
>>>    documentation as they reference "Documentation/timers/timers- 
>>> howto.rst"
>>>
>>> - Last but not least (as there are no more references): Update the 
>>> outdated
>>>    documentation and move it into a file with a self explaining file name
>>>
>>> The queue is available here and applies on top of tip/timers/core:
>>>
>>>    git://git.kernel.org/pub/scm/linux/kernel/git/anna-maria/linux- 
>>> devel.git timers/misc
>>>
>>> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
>> 
>> Hi,
>> 
>> not directly related to your serie, but some time ago I sent a patch to 
>> micro-optimize Optimize usleep_range(). (See [1])
>> 
>> The idea is that the 2 parameters of usleep_range() are usually 
>> constants and some code reordering could easily let the compiler compute 
>> a few things at compilation time.
>> 
>> There was consensus on the value of the change (see [2]), but as you are 
>
> Typo: there was *no* consensus...
>
>> touching things here, maybe it makes sense now to save a few cycles at 
>> runtime and a few bytes of code?
>> 

Sorry for the late reply. I'll check it and will come back to you.

Thanks,
	Anna-Maria

Re: [PATCH v2 00/15] timers: Cleanup delay/sleep related mess

[Thomas Gleixner]

On Mon, Sep 16 2024 at 22:20, Christophe JAILLET wrote:
> Le 11/09/2024 à 07:13, Anna-Maria Behnsen a écrit :
>
> not directly related to your serie, but some time ago I sent a patch to 
> micro-optimize Optimize usleep_range(). (See [1])
>
> The idea is that the 2 parameters of usleep_range() are usually 
> constants and some code reordering could easily let the compiler compute 
> a few things at compilation time.
>
> There was consensus on the value of the change (see [2]), but as you are 
> touching things here, maybe it makes sense now to save a few cycles at 
> runtime and a few bytes of code?

For the price of yet another ugly interface and pushing the
multiplication into the non-constant call sites.

Seriously usleep() is not a hotpath operation and the multiplication is
not even measurable except in micro benchmarks.

Thanks,

        tglx

Re: [PATCH v2 13/15] powerpc/rtas: Use fsleep() to minimize additional sleep duration

[Frederic Weisbecker]

Le Wed, Sep 11, 2024 at 07:13:39AM +0200, Anna-Maria Behnsen a écrit :
> When commit 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
> was introduced, documentation about proper usage of sleep realted functions

related*

> was outdated.
> 
> The commit message references the usage of a HZ=100 system. When using a
> 20ms sleep duration on such a system and therefore using msleep(), the
> possible additional slack will be +10ms.
> 
> When the system is configured with HZ=100 the granularity of a jiffy and of
> a bucket of the lowest timer wheel level is 10ms. To make sure a timer will
> not expire early (when queueing of the timer races with an concurrent
> update of jiffies), timers are always queued into the next bucket. This is
> the reason for the maximal possible slack of 10ms.
> 
> fsleep() limits the maximal possible slack to 25% by making threshold
> between usleep_range() and msleep() HZ dependent. As soon as the accuracy
> of msleep() is sufficient, the less expensive timer list timer based
> sleeping function is used instead of the more expensive hrtimer based
> usleep_range() function. The udelay() will not be used in this specific
> usecase as the lowest sleep length is larger than 1 microsecond.

Isn't udelay() for everything below 10us ?

> 
> Use fsleep() directly instead of using an own heuristic for the best
> sleeping mechanism to use..
> 
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

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

Re: [PATCH v2 13/15] powerpc/rtas: Use fsleep() to minimize additional sleep duration

[Anna-Maria Behnsen]

Frederic Weisbecker <frederic@kernel.org> writes:

> Le Wed, Sep 11, 2024 at 07:13:39AM +0200, Anna-Maria Behnsen a écrit :
>> When commit 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
>> was introduced, documentation about proper usage of sleep realted functions
>
> related*
>
>> was outdated.
>> 
>> The commit message references the usage of a HZ=100 system. When using a
>> 20ms sleep duration on such a system and therefore using msleep(), the
>> possible additional slack will be +10ms.
>> 
>> When the system is configured with HZ=100 the granularity of a jiffy and of
>> a bucket of the lowest timer wheel level is 10ms. To make sure a timer will
>> not expire early (when queueing of the timer races with an concurrent
>> update of jiffies), timers are always queued into the next bucket. This is
>> the reason for the maximal possible slack of 10ms.
>> 
>> fsleep() limits the maximal possible slack to 25% by making threshold
>> between usleep_range() and msleep() HZ dependent. As soon as the accuracy
>> of msleep() is sufficient, the less expensive timer list timer based
>> sleeping function is used instead of the more expensive hrtimer based
>> usleep_range() function. The udelay() will not be used in this specific
>> usecase as the lowest sleep length is larger than 1 microsecond.
>
> Isn't udelay() for everything below 10us ?

It's larger than 1 millisecond...

>
>> 
>> Use fsleep() directly instead of using an own heuristic for the best
>> sleeping mechanism to use..
>> 
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Nathan Lynch <nathanl@linux.ibm.com>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
>> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
>
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>