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

[PATCH 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:

- Minor changes (naming and typo fixes)

- 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

Cc: linux-kernel@vger.kernel.org
Cc: Len Brown <len.brown@intel.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
To: Frederic Weisbecker <frederic@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
To: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>

Thanks,

Anna-Maria

---
Anna-Maria Behnsen (15):
      timers: Rename next_expiry_recalc() to be unique
      cpu: Use already existing usleep_range()
      Comments: Fix wrong singular form of jiffies
      timers: Move *sleep*() and timeout functions into a separate file
      timers: Rename sleep_idle_range() to sleep_range_idle()
      timers: Update function descriptions of sleep/delay related functions
      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/admin-guide/media/vivid.rst          |   2 +-
 Documentation/dev-tools/checkpatch.rst             |   6 -
 Documentation/timers/delay_sleep_functions.rst     | 122 +++++++
 Documentation/timers/index.rst                     |   2 +-
 Documentation/timers/timers-howto.rst              | 115 -------
 .../sp_SP/scheduler/sched-design-CFS.rst           |   2 +-
 MAINTAINERS                                        |   2 +
 arch/arm/mach-versatile/spc.c                      |   2 +-
 arch/m68k/q40/q40ints.c                            |   2 +-
 arch/powerpc/kernel/rtas.c                         |  21 +-
 arch/x86/kernel/cpu/mce/dev-mcelog.c               |   2 +-
 drivers/char/ipmi/ipmi_ssif.c                      |   2 +-
 drivers/dma-buf/st-dma-fence.c                     |   2 +-
 drivers/gpu/drm/i915/gem/i915_gem_wait.c           |   2 +-
 drivers/gpu/drm/i915/gt/selftest_execlists.c       |   4 +-
 drivers/gpu/drm/i915/i915_utils.c                  |   2 +-
 drivers/gpu/drm/v3d/v3d_bo.c                       |   2 +-
 drivers/isdn/mISDN/dsp_cmx.c                       |   2 +-
 drivers/media/usb/dvb-usb-v2/anysee.c              |   6 +-
 drivers/net/ethernet/marvell/mvmdio.c              |   2 +-
 drivers/regulator/core.c                           |  33 +-
 fs/xfs/xfs_buf.h                                   |   2 +-
 include/asm-generic/delay.h                        |  46 ++-
 include/linux/delay.h                              |  79 ++++-
 include/linux/iopoll.h                             |  24 +-
 include/linux/jiffies.h                            |   2 +-
 include/linux/phy.h                                |   7 +-
 include/linux/regmap.h                             |  18 +-
 include/linux/timekeeper_internal.h                |   2 +-
 kernel/cpu.c                                       |   2 +-
 kernel/time/Makefile                               |   2 +-
 kernel/time/alarmtimer.c                           |   2 +-
 kernel/time/clockevents.c                          |   2 +-
 kernel/time/hrtimer.c                              | 122 +------
 kernel/time/posix-timers.c                         |   4 +-
 kernel/time/sleep_timeout.c                        | 363 +++++++++++++++++++++
 kernel/time/timer.c                                | 210 +-----------
 lib/Kconfig.debug                                  |   2 +-
 mm/damon/core.c                                    |   5 +-
 net/batman-adv/types.h                             |   2 +-
 scripts/checkpatch.pl                              |  38 ---
 sound/soc/sof/ops.h                                |   6 +-
 42 files changed, 668 insertions(+), 607 deletions(-)

[PATCH 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>
---
 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..0794ca28e51e 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 a way longer than requested independent of HZ
+		 * configuration, use fsleep(). See fsleep() for detailes 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 00/15] timers: Cleanup delay/sleep related mess

[Rafael J. Wysocki]

On Wed, Sep 4, 2024 at 3:05 PM Anna-Maria Behnsen
<anna-maria@linutronix.de> wrote:
>
> 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:
>
> - Minor changes (naming and typo fixes)
>
> - 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
>
> Cc: linux-kernel@vger.kernel.org
> Cc: Len Brown <len.brown@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> To: Frederic Weisbecker <frederic@kernel.org>
> To: Thomas Gleixner <tglx@linutronix.de>
> To: Jonathan Corbet <corbet@lwn.net>
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
>
> Thanks,
>
> Anna-Maria
>
> ---
> Anna-Maria Behnsen (15):
>       timers: Rename next_expiry_recalc() to be unique
>       cpu: Use already existing usleep_range()
>       Comments: Fix wrong singular form of jiffies
>       timers: Move *sleep*() and timeout functions into a separate file
>       timers: Rename sleep_idle_range() to sleep_range_idle()
>       timers: Update function descriptions of sleep/delay related functions
>       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

I like the changes, so

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

for the series.

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

[Michael Ellerman]

Anna-Maria Behnsen <anna-maria@linutronix.de> writes:
> When commit 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
> was introduced, documentation about proper usage of sleep realted functions
> was outdated.
>
...
> Use fsleep() directly instead of using an own heuristic for the best
> sleeping mechanism to use..

Thanks for tidying this up. I only learnt about fsleep() in the last ~year.

Two minor nits ...

> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index f7e86e09c49f..0794ca28e51e 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 a way longer than requested independent of HZ
 
"not be way longer" reads better I think?

> +		 * configuration, use fsleep(). See fsleep() for detailes of
                                                                 ^
                                                                 details
> +		 * used sleeping functions.
>  		 */
> -		if (ms <= 20)
> -			usleep_range(ms * 100, ms * 1000);
> -		else
> -			msleep(ms);
> +		fsleep(ms * 1000);
>  		break;
>  	case RTAS_BUSY:
>  		ret = true;

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)


cheers

Re: (subset) [PATCH 00/15] timers: Cleanup delay/sleep related mess

[Mark Brown]

On Wed, 04 Sep 2024 15:04:50 +0200, Anna-Maria Behnsen wrote:
> 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:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[11/15] regulator: core: Use fsleep() to get best sleep mechanism
        commit: f20669fbcf99d0e15e94fb50929bb1c41618e197

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark