From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>,
Anna-Maria Behnsen <anna-maria@linutronix.de>,
John Stultz <jstultz@google.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>, Stephen Boyd <sboyd@kernel.org>,
Eric Biederman <ebiederm@xmission.com>,
Oleg Nesterov <oleg@redhat.com>
Subject: [patch 09/45] posix-cpu-timers: Fix posix_cpu_timer_get() behaviour
Date: Tue, 6 Jun 2023 16:37:33 +0200 (CEST) [thread overview]
Message-ID: <20230606142031.532247561@linutronix.de> (raw)
In-Reply-To: <20230606132949.068951363@linutronix.de>
timer_gettime() must return the remaining time to the next expiry of a
timer or 0 if the timer is not armed and no signal pending.
This has to be correct also for interval timers even if the signal is
pending or the timer has been created with SIGEV_NONE.
The posix CPU timer implementation fails for both cases as it does not
forward the timer in posix_cpu_timer_get() before calculating the expiry
time.
It neither clears the expiry value when a oneshot SIGEV_NONE timer expired
and returns 1nsec instead, which is only correct for timers with signals
when the signal delivery did not happen yet.
Aside of that posix_cpu_timer_set() pointlessly arms SIGEV_NONE timers
which are later disarmed when the initial expiry happens. That's bogus and
just keeping the process wide timer active for nothing.
Cure this by:
1) Avoiding to arm SIGEV_NONE timers
2) Forwarding interval timers in posix_cpu_timer_get()
3) Taking SIGEV_NONE into account when a oneshot timer has expired
Make the update logic a separate function so it can be reused to simplify
posix_cpu_timer_set().
Fixes: ae1a78eecc45 ("cpu-timers: Return correct previous timer reload value")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/time/posix-cpu-timers.c | 96 +++++++++++++++++++++++------------------
1 file changed, 54 insertions(+), 42 deletions(-)
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -584,12 +584,7 @@ static void cpu_timer_fire(struct k_itim
{
struct cpu_timer *ctmr = &timer->it.cpu;
- if ((timer->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE) {
- /*
- * User don't want any signal.
- */
- cpu_timer_setexpires(ctmr, 0);
- } else if (unlikely(timer->sigq == NULL)) {
+ if (unlikely(timer->sigq == NULL)) {
/*
* This a special case for clock_nanosleep,
* not a normal timer from sys_timer_create.
@@ -623,6 +618,7 @@ static void cpu_timer_fire(struct k_itim
static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
struct itimerspec64 *new, struct itimerspec64 *old)
{
+ bool sigev_none = timer->it_sigev_notify == SIGEV_NONE;
clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
u64 old_expires, new_expires, old_incr, val;
struct cpu_timer *ctmr = &timer->it.cpu;
@@ -686,7 +682,7 @@ static int posix_cpu_timer_set(struct k_
if (CPUCLOCK_PERTHREAD(timer->it_clock))
val = cpu_clock_sample(clkid, p);
else
- val = cpu_clock_sample_group(clkid, p, true);
+ val = cpu_clock_sample_group(clkid, p, !sigev_none);
if (old) {
if (old_expires == 0) {
@@ -723,19 +719,20 @@ static int posix_cpu_timer_set(struct k_
goto out;
}
- if (new_expires != 0 && !(timer_flags & TIMER_ABSTIME)) {
+ /* Convert relative expiry time to absolute */
+ if (new_expires && !(timer_flags & TIMER_ABSTIME))
new_expires += val;
- }
+
+ /* Set the new expiry time (might be 0) */
+ cpu_timer_setexpires(ctmr, new_expires);
/*
- * Install the new expiry time (or zero).
- * For a timer with no notification action, we don't actually
- * arm the timer (we'll just fake it for timer_gettime).
+ * Arm the timer if it is not disabled, the new expiry value has
+ * not yet expired and the timer requires signal delivery.
+ * SIGEV_NONE timers are never armed.
*/
- cpu_timer_setexpires(ctmr, new_expires);
- if (new_expires != 0 && val < new_expires) {
+ if (!sigev_none && new_expires && val < new_expires)
arm_timer(timer, p);
- }
unlock_task_sighand(p, &flags);
/*
@@ -754,7 +751,7 @@ static int posix_cpu_timer_set(struct k_
timer->it_overrun_last = 0;
timer->it_overrun = -1;
- if (val >= new_expires) {
+ if (!sigev_none && val >= new_expires) {
if (new_expires != 0) {
/*
* The designated time already passed, so we notify
@@ -785,45 +782,60 @@ static int posix_cpu_timer_set(struct k_
return ret;
}
-static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp)
+static void __posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp, u64 now)
{
- clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
- struct cpu_timer *ctmr = &timer->it.cpu;
- u64 now, expires = cpu_timer_getexpires(ctmr);
- struct task_struct *p;
-
- rcu_read_lock();
- p = cpu_timer_task_rcu(timer);
- if (!p)
- goto out;
+ bool sigev_none = timer->it_sigev_notify == SIGEV_NONE;
+ u64 expires;
/*
- * Easy part: convert the reload time.
+ * Make sure that interval timers are moved forward for the
+ * following cases:
+ * - SIGEV_NONE timers which are never armed
+ * - Timers which expired, but the signal has not yet been
+ * delivered
*/
- itp->it_interval = ktime_to_timespec64(timer->it_interval);
-
- if (!expires)
- goto out;
+ expires = bump_cpu_timer(timer, now);
/*
- * Sample the clock to take the difference with the expiry time.
+ * Interval timers cannot have a remaining time <= 0 because the
+ * forwarding guarantees to move them forward so that the next
+ * timer expiry is > @now.
*/
- if (CPUCLOCK_PERTHREAD(timer->it_clock))
- now = cpu_clock_sample(clkid, p);
- else
- now = cpu_clock_sample_group(clkid, p, false);
-
if (now < expires) {
itp->it_value = ns_to_timespec64(expires - now);
} else {
/*
- * The timer should have expired already, but the firing
- * hasn't taken place yet. Say it's just about to expire.
+ * A single shot SIGEV_NONE timer must return 0, when it is
+ * expired! Timers which have a real signal delivery mode
+ * must return a remaining time greater than 0 because the
+ * signal has not yet been delivered.
*/
- itp->it_value.tv_nsec = 1;
- itp->it_value.tv_sec = 0;
+ if (!sigev_none)
+ itp->it_value.tv_nsec = 1;
+ }
+}
+
+static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp)
+{
+ clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
+ struct cpu_timer *ctmr = &timer->it.cpu;
+ struct task_struct *p;
+ u64 now;
+
+ rcu_read_lock();
+ p = cpu_timer_task_rcu(timer);
+ if (p) {
+ itp->it_interval = ktime_to_timespec64(timer->it_interval);
+
+ if (cpu_timer_getexpires(ctmr)) {
+ if (CPUCLOCK_PERTHREAD(timer->it_clock))
+ now = cpu_clock_sample(clkid, p);
+ else
+ now = cpu_clock_sample_group(clkid, p, false);
+
+ __posix_cpu_timer_get(timer, itp, now);
+ }
}
-out:
rcu_read_unlock();
}
next prev parent reply other threads:[~2023-06-06 14:38 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-06 14:37 [patch 00/45] posix-timers: Cure inconsistencies and the SIG_IGN mess Thomas Gleixner
2023-06-06 14:37 ` [patch 01/45] selftests/timers/posix_timers: Make signal distribution test less fragile Thomas Gleixner
2023-06-06 14:37 ` [patch 02/45] selftests/timers/posix_timers: Use TAP reporting format Thomas Gleixner
2023-06-06 14:37 ` [patch 03/45] selftests/timers/posix_timers: Add SIG_IGN test Thomas Gleixner
2023-06-06 14:37 ` [patch 04/45] selftests/timers/posix_timers: Validate signal rules Thomas Gleixner
2023-06-06 14:37 ` [patch 05/45] selftests/timers/posix-timers: Validate SIGEV_NONE Thomas Gleixner
2023-06-06 14:37 ` [patch 06/45] selftests/timers/posix-timers: Validate timer_gettime() Thomas Gleixner
2023-06-06 14:37 ` [patch 07/45] selftests/timers/posix-timers: Validate overrun after unblock Thomas Gleixner
2023-06-06 14:37 ` [patch 08/45] posix-timers: Convert timer list to hlist Thomas Gleixner
2023-06-22 21:18 ` Frederic Weisbecker
2023-06-06 14:37 ` Thomas Gleixner [this message]
2023-06-26 22:46 ` [patch 09/45] posix-cpu-timers: Fix posix_cpu_timer_get() behaviour Frederic Weisbecker
2023-06-29 18:14 ` Thomas Gleixner
2023-06-06 14:37 ` [patch 10/45] posix-cpu-timers: Use @now instead of @val for clarity Thomas Gleixner
2023-06-27 9:53 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 11/45] posix-cpu-timers: Remove incorrect comment in posix_cpu_timer_set() Thomas Gleixner
2023-06-27 10:30 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 12/45] posix-cpu-timers: Simplify posix_cpu_timer_set() Thomas Gleixner
2023-06-27 10:51 ` Frederic Weisbecker
2023-06-29 18:43 ` Thomas Gleixner
2023-06-06 14:37 ` [patch 13/45] posix-cpu-timers: Replace old expiry retrieval in posix_cpu_timer_set() Thomas Gleixner
2023-06-27 11:32 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 14/45] posix-timers: Consolidate interval retrieval Thomas Gleixner
2023-06-28 13:08 ` Frederic Weisbecker
2023-06-29 18:47 ` Thomas Gleixner
2023-06-30 11:25 ` Frederic Weisbecker
2023-06-30 13:07 ` Thomas Gleixner
2023-06-30 14:04 ` Frederic Weisbecker
2023-07-01 18:01 ` Thomas Gleixner
2023-06-06 14:37 ` [patch 15/45] posix-timers: Clear overrun in common_timer_set() Thomas Gleixner
2023-06-30 21:40 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 16/45] posix-timers: Consolidate timer setup Thomas Gleixner
2023-07-03 21:12 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 17/45] posix-cpu-timers: Make k_itimer::it_active consistent Thomas Gleixner
2023-07-03 22:30 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 18/45] posix-timers: Consolidate signal queueing Thomas Gleixner
2023-07-03 22:51 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 19/45] signal: Remove task argument from dequeue_signal() Thomas Gleixner
2023-07-04 10:02 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 20/45] signal: Replace BUG_ON()s Thomas Gleixner
2023-07-04 10:24 ` Frederic Weisbecker
2023-06-06 14:37 ` [patch 21/45] signal: Confine POSIX_TIMERS properly Thomas Gleixner
2023-06-06 14:37 ` [patch 22/45] signal: Get rid of resched_timer logic Thomas Gleixner
2023-06-06 14:37 ` [patch 23/45] posix-timers: Cure si_sys_private race Thomas Gleixner
2023-06-06 14:37 ` [patch 24/45] signal: Allow POSIX timer signals to be dropped Thomas Gleixner
2023-06-06 14:37 ` [patch 25/45] posix-timers: Drop signal if timer has been deleted or reprogrammed Thomas Gleixner
2023-06-06 14:38 ` [patch 26/45] posix-timers: Rename k_itimer::it_requeue_pending Thomas Gleixner
2023-06-06 14:38 ` [patch 27/45] posix-timers: Add proper state tracking Thomas Gleixner
2023-06-06 14:38 ` [patch 28/45] posix-timers: Make signal delivery consistent Thomas Gleixner
2023-06-06 14:38 ` [patch 29/45] posix-timers: Make signal overrun accounting sensible Thomas Gleixner
2023-06-06 14:38 ` [patch 30/45] posix-cpu-timers: Use dedicated flag for CPU timer nanosleep Thomas Gleixner
2023-06-06 14:38 ` [patch 31/45] posix-timers: Add a refcount to struct k_itimer Thomas Gleixner
2023-06-06 14:38 ` [patch 32/45] signal: Split up __sigqueue_alloc() Thomas Gleixner
2023-06-06 14:38 ` [patch 33/45] signal: Provide posixtimer_sigqueue_init() Thomas Gleixner
2023-06-06 14:38 ` [patch 34/45] signal: Add sys_private_ptr to siginfo::_sifields::_timer Thomas Gleixner
2023-06-06 14:38 ` [patch 35/45] signal: Refactor send_sigqueue() Thomas Gleixner
2023-06-06 14:38 ` [patch 36/45] posix-timers: Embed sigqueue in struct k_itimer Thomas Gleixner
2023-06-06 14:38 ` [patch 37/45] signal: Cleanup unused posix-timer leftovers Thomas Gleixner
2023-06-06 14:38 ` [patch 38/45] signal: Add task argument to flush_sigqueue_mask() Thomas Gleixner
2023-06-06 14:38 ` [patch 39/45] signal: Provide ignored_posix_timers list Thomas Gleixner
2023-06-06 14:38 ` [patch 40/45] posix-timers: Handle ignored list on delete and exit Thomas Gleixner
2023-06-06 14:38 ` [patch 41/45] signal: Handle ignored signals in do_sigaction(action != SIG_IGN) Thomas Gleixner
2023-06-06 14:38 ` [patch 42/45] signal: Queue ignored posixtimers on ignore list Thomas Gleixner
2023-06-06 14:38 ` [patch 43/45] posix-timers: Cleanup SIG_IGN workaround leftovers Thomas Gleixner
2023-06-06 14:38 ` [patch 44/45] alarmtimers: Remove the throttle mechanism from alarm_forward_now() Thomas Gleixner
2023-06-06 14:38 ` [patch 45/45] alarmtimers: Remove return value from alarm functions Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230606142031.532247561@linutronix.de \
--to=tglx@linutronix.de \
--cc=anna-maria@linutronix.de \
--cc=ebiederm@xmission.com \
--cc=frederic@kernel.org \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®