mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>,
	Frederic Weisbecker <frederic@kernel.org>,
	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 V7 04/21] posix-cpu-timers: Cleanup the firing logic
Date: Tue,  5 Nov 2024 09:14:33 +0100 (CET)	[thread overview]
Message-ID: <20241105064213.172848618@linutronix.de> (raw)
In-Reply-To: <20241105063544.565410398@linutronix.de>

The firing flag of a posix CPU timer is tristate:

  0: when the timer is not about to deliver a signal

  1: when the timer has expired, but the signal has not been delivered yet

 -1: when the timer was queued for signal delivery and a rearm operation
     raced against it and supressed the signal delivery.

This is a pointless exercise as this can be simply expressed with a
boolean. Only if set, the signal is delivered. This makes delete and rearm
consistent with the rest of the posix timers.

Convert firing to bool and fixup the usage sites accordingly and add
comments why the timer cannot be dequeued right away.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
---
V6: New patch after detecting the tristate mismatch vs. bool from the
    patch which introduced the nanosleep flag.
---
 include/linux/posix-timers.h   |    2 +-
 kernel/time/posix-cpu-timers.c |   34 ++++++++++++++++++++++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -49,7 +49,7 @@ struct cpu_timer {
 	struct timerqueue_head		*head;
 	struct pid			*pid;
 	struct list_head		elist;
-	int				firing;
+	bool				firing;
 	struct task_struct __rcu	*handling;
 };
 
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -493,10 +493,18 @@ static int posix_cpu_timer_del(struct k_
 		 */
 		WARN_ON_ONCE(ctmr->head || timerqueue_node_queued(&ctmr->node));
 	} else {
-		if (timer->it.cpu.firing)
+		if (timer->it.cpu.firing) {
+			/*
+			 * Prevent signal delivery. The timer cannot be dequeued
+			 * because it is on the firing list which is not protected
+			 * by sighand->lock. The delivery path is waiting for
+			 * the timer lock. So go back, unlock and retry.
+			 */
+			timer->it.cpu.firing = false;
 			ret = TIMER_RETRY;
-		else
+		} else {
 			disarm_timer(timer, p);
+		}
 		unlock_task_sighand(p, &flags);
 	}
 
@@ -668,7 +676,13 @@ static int posix_cpu_timer_set(struct k_
 	old_expires = cpu_timer_getexpires(ctmr);
 
 	if (unlikely(timer->it.cpu.firing)) {
-		timer->it.cpu.firing = -1;
+		/*
+		 * Prevent signal delivery. The timer cannot be dequeued
+		 * because it is on the firing list which is not protected
+		 * by sighand->lock. The delivery path is waiting for
+		 * the timer lock. So go back, unlock and retry.
+		 */
+		timer->it.cpu.firing = false;
 		ret = TIMER_RETRY;
 	} else {
 		cpu_timer_dequeue(ctmr);
@@ -809,7 +823,7 @@ static u64 collect_timerqueue(struct tim
 		if (++i == MAX_COLLECTED || now < expires)
 			return expires;
 
-		ctmr->firing = 1;
+		ctmr->firing = true;
 		/* See posix_cpu_timer_wait_running() */
 		rcu_assign_pointer(ctmr->handling, current);
 		cpu_timer_dequeue(ctmr);
@@ -1364,7 +1378,7 @@ static void handle_posix_cpu_timers(stru
 	 * timer call will interfere.
 	 */
 	list_for_each_entry_safe(timer, next, &firing, it.cpu.elist) {
-		int cpu_firing;
+		bool cpu_firing;
 
 		/*
 		 * spin_lock() is sufficient here even independent of the
@@ -1376,13 +1390,13 @@ static void handle_posix_cpu_timers(stru
 		spin_lock(&timer->it_lock);
 		list_del_init(&timer->it.cpu.elist);
 		cpu_firing = timer->it.cpu.firing;
-		timer->it.cpu.firing = 0;
+		timer->it.cpu.firing = false;
 		/*
-		 * The firing flag is -1 if we collided with a reset
-		 * of the timer, which already reported this
-		 * almost-firing as an overrun.  So don't generate an event.
+		 * If the firing flag is cleared then this raced with a
+		 * timer rearm/delete operation. So don't generate an
+		 * event.
 		 */
-		if (likely(cpu_firing >= 0))
+		if (likely(cpu_firing))
 			cpu_timer_fire(timer);
 		/* See posix_cpu_timer_wait_running() */
 		rcu_assign_pointer(timer->it.cpu.handling, NULL);


  parent reply	other threads:[~2024-11-05  8:14 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-05  8:14 [patch V7 00/21] posix-timers: Cure the SIG_IGN mess Thomas Gleixner
2024-11-05  8:14 ` [patch V7 01/21] posix-cpu-timers: Correctly update timer status in posix_cpu_timer_del() Thomas Gleixner
2024-11-05 12:02   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 02/21] posix-timers: Make signal delivery consistent Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 03/21] posix-timers: Make signal overrun accounting sensible Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` Thomas Gleixner [this message]
2024-11-07  1:31   ` [tip: timers/core] posix-cpu-timers: Cleanup the firing logic tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 05/21] posix-cpu-timers: Use dedicated flag for CPU timer nanosleep Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 06/21] posix-timers: Add a refcount to struct k_itimer Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 07/21] signal: Split up __sigqueue_alloc() Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 08/21] signal: Provide posixtimer_sigqueue_init() Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 09/21] posix-timers: Store PID type in the timer Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 10/21] signal: Refactor send_sigqueue() Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 11/21] signal: Replace resched_timer logic Thomas Gleixner
2024-11-05 12:08   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 12/21] posix-timers: Embed sigqueue in struct k_itimer Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 13/21] signal: Cleanup unused posix-timer leftovers Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 14/21] posix-timers: Move sequence logic into struct k_itimer Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 15/21] signal: Provide ignored_posix_timers list Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 16/21] posix-timers: Handle ignored list on delete and exit Thomas Gleixner
2024-11-05 13:08   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 17/21] signal: Handle ignored signals in do_sigaction(action != SIG_IGN) Thomas Gleixner
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 18/21] signal: Queue ignored posixtimers on ignore list Thomas Gleixner
2024-11-05 14:02   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 19/21] posix-timers: Cleanup SIG_IGN workaround leftovers Thomas Gleixner
2024-11-05 14:26   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 20/21] alarmtimers: Remove the throttle mechanism from alarm_forward_now() Thomas Gleixner
2024-11-05 14:29   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2024-11-05  8:14 ` [patch V7 21/21] alarmtimers: Remove return value from alarm functions Thomas Gleixner
2024-11-05 14:34   ` Frederic Weisbecker
2024-11-07  1:31   ` [tip: timers/core] " tip-bot2 for 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=20241105064213.172848618@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®