mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Nicholas Piggin <npiggin@gmail.com>,
	linux-arch@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>
Subject: [RFC PATCH 1/3] hrtimer: balance irq save/restore
Date: Tue,  9 May 2023 21:07:37 +1000	[thread overview]
Message-ID: <20230509110739.241735-2-npiggin@gmail.com> (raw)
In-Reply-To: <20230509110739.241735-1-npiggin@gmail.com>

hrtimers uses a trick where irq flags are saved initially and then
passed to callers to restore irqs when dropping the lock, which then
re-lock with raw_spin_lock_irq.

This is not actually a bug, but it is one of the few places in the
kernel that may disable irqs when they are already disabled. It would
be nice to have a debug check for that and enforce irq calls are
balanced. It's one thing for core code to be clever, but something like
this in a crufty driver could be a red flag.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 kernel/time/hrtimer.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index e8c08292defc..8baa36b091f0 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1364,13 +1364,13 @@ static void hrtimer_cpu_base_unlock_expiry(struct hrtimer_cpu_base *base)
  * allows the waiter to acquire the lock and make progress.
  */
 static void hrtimer_sync_wait_running(struct hrtimer_cpu_base *cpu_base,
-				      unsigned long flags)
+				      unsigned long *flags)
 {
 	if (atomic_read(&cpu_base->timer_waiters)) {
-		raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
+		raw_spin_unlock_irqrestore(&cpu_base->lock, *flags);
 		spin_unlock(&cpu_base->softirq_expiry_lock);
 		spin_lock(&cpu_base->softirq_expiry_lock);
-		raw_spin_lock_irq(&cpu_base->lock);
+		raw_spin_lock_irqsave(&cpu_base->lock, *flags);
 	}
 }
 
@@ -1424,7 +1424,7 @@ hrtimer_cpu_base_lock_expiry(struct hrtimer_cpu_base *base) { }
 static inline void
 hrtimer_cpu_base_unlock_expiry(struct hrtimer_cpu_base *base) { }
 static inline void hrtimer_sync_wait_running(struct hrtimer_cpu_base *base,
-					     unsigned long flags) { }
+					     unsigned long *flags) { }
 #endif
 
 /**
@@ -1642,7 +1642,7 @@ EXPORT_SYMBOL_GPL(hrtimer_active);
 static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 			  struct hrtimer_clock_base *base,
 			  struct hrtimer *timer, ktime_t *now,
-			  unsigned long flags) __must_hold(&cpu_base->lock)
+			  unsigned long *flags) __must_hold(&cpu_base->lock)
 {
 	enum hrtimer_restart (*fn)(struct hrtimer *);
 	bool expires_in_hardirq;
@@ -1678,7 +1678,7 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 	 * protected against migration to a different CPU even if the lock
 	 * is dropped.
 	 */
-	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
+	raw_spin_unlock_irqrestore(&cpu_base->lock, *flags);
 	trace_hrtimer_expire_entry(timer, now);
 	expires_in_hardirq = lockdep_hrtimer_enter(timer);
 
@@ -1686,7 +1686,7 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 
 	lockdep_hrtimer_exit(expires_in_hardirq);
 	trace_hrtimer_expire_exit(timer);
-	raw_spin_lock_irq(&cpu_base->lock);
+	raw_spin_lock_irqsave(&cpu_base->lock, *flags);
 
 	/*
 	 * Note: We clear the running state after enqueue_hrtimer and
@@ -1715,7 +1715,7 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 }
 
 static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now,
-				 unsigned long flags, unsigned int active_mask)
+				 unsigned long *flags, unsigned int active_mask)
 {
 	struct hrtimer_clock_base *base;
 	unsigned int active = cpu_base->active_bases & active_mask;
@@ -1763,7 +1763,7 @@ static __latent_entropy void hrtimer_run_softirq(struct softirq_action *h)
 	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 
 	now = hrtimer_update_base(cpu_base);
-	__hrtimer_run_queues(cpu_base, now, flags, HRTIMER_ACTIVE_SOFT);
+	__hrtimer_run_queues(cpu_base, now, &flags, HRTIMER_ACTIVE_SOFT);
 
 	cpu_base->softirq_activated = 0;
 	hrtimer_update_softirq_timer(cpu_base, true);
@@ -1808,7 +1808,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
 		raise_softirq_irqoff(HRTIMER_SOFTIRQ);
 	}
 
-	__hrtimer_run_queues(cpu_base, now, flags, HRTIMER_ACTIVE_HARD);
+	__hrtimer_run_queues(cpu_base, now, &flags, HRTIMER_ACTIVE_HARD);
 
 	/* Reevaluate the clock bases for the [soft] next expiry */
 	expires_next = hrtimer_update_next_event(cpu_base);
@@ -1921,7 +1921,7 @@ void hrtimer_run_queues(void)
 		raise_softirq_irqoff(HRTIMER_SOFTIRQ);
 	}
 
-	__hrtimer_run_queues(cpu_base, now, flags, HRTIMER_ACTIVE_HARD);
+	__hrtimer_run_queues(cpu_base, now, &flags, HRTIMER_ACTIVE_HARD);
 	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
 }
 
-- 
2.40.1


  reply	other threads:[~2023-05-09 11:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-09 11:07 [RFC PATCH 0/3] Prohibit irq disable when disabled or enable when enabled Nicholas Piggin
2023-05-09 11:07 ` Nicholas Piggin [this message]
2023-05-09 11:07 ` [RFC PATCH 2/3] init: Require archs call start_kernel with arch irqs disabled Nicholas Piggin
2023-05-09 11:07 ` [RFC PATCH 3/3] irqflags: Warn on irq disable when disabled and enable when enabled Nicholas Piggin
2023-05-22  8:25   ` kernel test robot

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=20230509110739.241735-2-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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®