[PATCH 0/2] hrtimer: More fixes for handling of timer slack of rt tasks

[PATCH 0/2] hrtimer: More fixes for handling of timer slack of rt tasks

[Felix Moessbauer]

This series fixes the (hopefully) last location of an incorrectly
handled timer slack on rt tasks in hrtimer_start_range_ns(), which was
uncovered by a userland change in glibc 2.33. Along that, the patch documents,
that currently all tasks with rt priority should ignore the timer slack.

For me, it is unclear though, if this behavior is actually intended, as
also PI boosted tasks that create or reprogram timers ignore the timer slack.
While I guess this does not have an effect in most cases, it is at least
a lost performance optimization chance. Instead, ignoring the timer
slack could be limited to tasks with RT/DL scheduling policies.

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (2):
  hrtimer: Document, that PI boosted tasks have no timer slack
  hrtimer: Ignore slack time for RT tasks in hrtimer_start_range_ns()

 kernel/time/hrtimer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.39.2

[PATCH 1/2] hrtimer: Document, that PI boosted tasks have no timer slack

[Felix Moessbauer]

The documentation of schedule_hrtimeout_range already states, that RT
and DL tasks do not have a timer slack. However, no information about PI
boosted tasks is given. The current implementation consistently ignores
the timer slack also for PI boosted tasks (all tasks with a rt priority
at time of programming the timer).

This patch improves the documentation by stating that the timer slack is
also ignored for PI boosted tasks. It does not include any functional
change.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 kernel/time/hrtimer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index b8ee320208d4..2b1469f61d9c 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -2311,7 +2311,7 @@ EXPORT_SYMBOL_GPL(schedule_hrtimeout_range_clock);
  *
  * The @delta argument gives the kernel the freedom to schedule the
  * actual wakeup to a time that is both power and performance friendly
- * for regular (non RT/DL) tasks.
+ * for regular (non RT/DL or PI boosted) tasks.
  * The kernel give the normal best effort behavior for "@expires+@delta",
  * but may decide to fire the timer earlier, but no earlier than @expires.
  *
-- 
2.39.2

Re: [PATCH 1/2] hrtimer: Document, that PI boosted tasks have no timer slack

[Thomas Gleixner]

On Mon, Aug 05 2024 at 14:41, Felix Moessbauer wrote:
> The documentation of schedule_hrtimeout_range already states, that RT
> and DL tasks do not have a timer slack. However, no information about PI
> boosted tasks is given. The current implementation consistently ignores
> the timer slack also for PI boosted tasks (all tasks with a rt priority
> at time of programming the timer).

Which is wrong. This condition should not use rt_task() it should use
task_is_realtime() instead.

> This patch improves the documentation by stating that the timer slack
> is

git grep "This patch" Documentation/process/

Thanks,

        tglx

Re: [PATCH 1/2] hrtimer: Document, that PI boosted tasks have no timer slack

[Sebastian Andrzej Siewior]

On 2024-08-05 15:02:00 [+0200], Thomas Gleixner wrote:
> On Mon, Aug 05 2024 at 14:41, Felix Moessbauer wrote:
> > The documentation of schedule_hrtimeout_range already states, that RT
> > and DL tasks do not have a timer slack. However, no information about PI
> > boosted tasks is given. The current implementation consistently ignores
> > the timer slack also for PI boosted tasks (all tasks with a rt priority
> > at time of programming the timer).
> 
> Which is wrong. This condition should not use rt_task() it should use
> task_is_realtime() instead.

There is also the series 20240610192018.1567075-1-qyousef@layalina.io

> Thanks,
> 
>         tglx

Sebastian

[PATCH 2/2] hrtimer: Ignore slack time for RT tasks in hrtimer_start_range_ns()

[Felix Moessbauer]

RT tasks do not have any timerslack, as this induces jitter. By
that, the timer slack is already ignored in the nanosleep family and
schedule_hrtimeout_range() (fixed in 0c52310f2600).

The hrtimer_start_range_ns function is indirectly used by glibc-2.33+
for timed waits on condition variables. These are sometimes used in
RT applications for realtime queue processing. At least on the
combination of kernel 5.10 and glibc-2.31, the timed wait on condition
variables in rt tasks was precise (no slack), however glibc-2.33
changed the internal wait implementation, exposing the kernel bug.

This patch makes the timer slack consistent across all hrtimer
programming code, by ignoring the timerslack for rt tasks also in the
last remaining location in hrtimer_start_range_ns().

Similar to 0c52310f2600, this fix should be backported as well.

Cc: stable@vger.kernel.org
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 kernel/time/hrtimer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 2b1469f61d9c..1b26e095114d 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1274,7 +1274,7 @@ static int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
  * hrtimer_start_range_ns - (re)start an hrtimer
  * @timer:	the timer to be added
  * @tim:	expiry time
- * @delta_ns:	"slack" range for the timer
+ * @delta_ns:	"slack" range for the timer for SCHED_OTHER tasks
  * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
  *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED);
  *		softirq based mode is considered for debug purpose only!
@@ -1299,6 +1299,10 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 
 	base = lock_hrtimer_base(timer, &flags);
 
+	/* rt-tasks do not have a timer slack for obvious reasons */
+	if (rt_task(current))
+		delta_ns = 0;
+
 	if (__hrtimer_start_range_ns(timer, tim, delta_ns, mode, base))
 		hrtimer_reprogram(timer, true);
 
-- 
2.39.2

Re: [PATCH 2/2] hrtimer: Ignore slack time for RT tasks in hrtimer_start_range_ns()

[Thomas Gleixner]

On Mon, Aug 05 2024 at 14:41, Felix Moessbauer wrote:
> RT tasks do not have any timerslack, as this induces jitter. By
> that, the timer slack is already ignored in the nanosleep family and
> schedule_hrtimeout_range() (fixed in 0c52310f2600).
>
> The hrtimer_start_range_ns function is indirectly used by glibc-2.33+
> for timed waits on condition variables. These are sometimes used in
> RT applications for realtime queue processing. At least on the
> combination of kernel 5.10 and glibc-2.31, the timed wait on condition
> variables in rt tasks was precise (no slack), however glibc-2.33
> changed the internal wait implementation, exposing the kernel bug.

That's hardly a bug. It's an oversight.

> This patch makes the timer slack consistent across all hrtimer

"This patch" ....

> programming code, by ignoring the timerslack for rt tasks also in the
> last remaining location in hrtimer_start_range_ns().
>
> Similar to 0c52310f2600, this fix should be backported as well.

This is not part of the change log.

> Cc: stable@vger.kernel.org
> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> ---
>  kernel/time/hrtimer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
> index 2b1469f61d9c..1b26e095114d 100644
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -1274,7 +1274,7 @@ static int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
>   * hrtimer_start_range_ns - (re)start an hrtimer
>   * @timer:	the timer to be added
>   * @tim:	expiry time
> - * @delta_ns:	"slack" range for the timer
> + * @delta_ns:	"slack" range for the timer for SCHED_OTHER tasks
>   * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
>   *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED);
>   *		softirq based mode is considered for debug purpose only!
> @@ -1299,6 +1299,10 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
>  
>  	base = lock_hrtimer_base(timer, &flags);
>  
> +	/* rt-tasks do not have a timer slack for obvious reasons */
> +	if (rt_task(current))
> +		delta_ns = 0;

        task_is_realtime()

please

Thanks,

        tglx