[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ed6src28.ffs@tglx>
Date: Tue, 13 Aug 2024 11:45:19 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Felix Moessbauer <felix.moessbauer@...mens.com>,
linux-kernel@...r.kernel.org
Cc: Frederic Weisbecker <frederic@...nel.org>, Anna-Maria Behnsen
<anna-maria@...utronix.de>, jan.kiszka@...mens.com, Sebastian Andrzej
Siewior <bigeasy@...utronix.de>, qyousef@...alina.io, Felix Moessbauer
<felix.moessbauer@...mens.com>
Subject: Re: [PATCH v3 1/1] hrtimer: use and report correct timerslack
values for realtime tasks
On Tue, Aug 13 2024 at 09:29, Felix Moessbauer wrote:
> @@ -2569,10 +2569,11 @@ static ssize_t timerslack_ns_write(struct file *file, const char __user *buf,
> }
>
> task_lock(p);
> - if (slack_ns == 0)
> - p->timer_slack_ns = p->default_timer_slack_ns;
> - else
> - p->timer_slack_ns = slack_ns;
> + if (task_is_realtime(p))
> + slack_ns = 0;
This should respect the user supplied value, i.e.
if (!task_is_realtime(p) && !slack_ns)
slack_ns = p->default_timer_slack_ns;
> + else if (slack_ns == 0)
> + slack_ns = p->default_timer_slack_ns;
> + p->timer_slack_ns = slack_ns;
> task_unlock(p);
>
> out:
> diff --git a/fs/select.c b/fs/select.c
> index 9515c3fa1a03..153124ed50fd 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -77,19 +77,13 @@ u64 select_estimate_accuracy(struct timespec64 *tv)
> {
> u64 ret;
> struct timespec64 now;
> -
> - /*
> - * Realtime tasks get a slack of 0 for obvious reasons.
> - */
> -
> - if (rt_task(current))
> - return 0;
> + u64 slack = current->timer_slack_ns;
>
> ktime_get_ts64(&now);
> now = timespec64_sub(*tv, now);
> ret = __estimate_accuracy(&now);
> - if (ret < current->timer_slack_ns)
> - return current->timer_slack_ns;
> + if (ret < slack || slack == 0)
> + return slack;
Seriously? Do all the calculations first and then discard them when
slack is 0?
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index ae1b42775ef9..195d2f2834a9 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -406,6 +406,14 @@ static void __setscheduler_params(struct task_struct *p,
> else if (fair_policy(policy))
> p->static_prio = NICE_TO_PRIO(attr->sched_nice);
>
> + /* rt-policy tasks do not have a timerslack */
> + if (task_is_realtime(p)) {
> + p->timer_slack_ns = 0;
> + } else if (p->timer_slack_ns == 0) {
> + /* when switching back to non-rt policy, restore timerslack */
> + p->timer_slack_ns = p->default_timer_slack_ns;
> + }
> +
> /*
> * __sched_setscheduler() ensures attr->sched_priority == 0 when
> * !rt_policy. Always setting this ensures that things like
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 3a2df1bd9f64..e3c4cffb520c 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2557,6 +2557,8 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> error = current->timer_slack_ns;
> break;
> case PR_SET_TIMERSLACK:
> + if (task_is_realtime(current))
> + break;
Why are you declaring that a RT task has to have 0 slack if we are
lifting the hard coded slack zeroing in the hrtimer functions?
Thanks,
tglx
Powered by blists - more mailing lists