From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: linux-kernel@vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Valentin Schneider <valentin.schneider@arm.com>,
Ingo Molnar <mingo@kernel.org>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Subject: [PATCH] notifier: Make atomic_notifiers use raw_spinlock
Date: Fri, 6 Aug 2021 16:07:18 +0200 [thread overview]
Message-ID: <20210806140718.mxss3cbqijfebdo5@linutronix.de> (raw)
From: Valentin Schneider <valentin.schneider@arm.com>
Date: Sun, 22 Nov 2020 20:19:04 +0000
Booting a recent PREEMPT_RT kernel (v5.10-rc3-rt7-rebase) on my arm64 Juno
leads to the idle task blocking on an RT sleeping spinlock down some
notifier path:
| BUG: scheduling while atomic: swapper/5/0/0x00000002
…
| atomic_notifier_call_chain_robust (kernel/notifier.c:71 kernel/notifier.c:118 kernel/notifier.c:186)
| cpu_pm_enter (kernel/cpu_pm.c:39 kernel/cpu_pm.c:93)
| psci_enter_idle_state (drivers/cpuidle/cpuidle-psci.c:52 drivers/cpuidle/cpuidle-psci.c:129)
| cpuidle_enter_state (drivers/cpuidle/cpuidle.c:238)
| cpuidle_enter (drivers/cpuidle/cpuidle.c:353)
| do_idle (kernel/sched/idle.c:132 kernel/sched/idle.c:213 kernel/sched/idle.c:273)
| cpu_startup_entry (kernel/sched/idle.c:368 (discriminator 1))
| secondary_start_kernel (arch/arm64/kernel/smp.c:273)
Two points worth noting:
1) That this is conceptually the same issue as pointed out in:
313c8c16ee62 ("PM / CPU: replace raw_notifier with atomic_notifier")
2) Only the _robust() variant of atomic_notifier callchains suffer from
this
AFAICT only the cpu_pm_notifier_chain really needs to be changed, but
singling it out would mean introducing a new (truly) non-blocking API. At
the same time, callers that are fine with any blocking within the call
chain should use blocking notifiers, so patching up all atomic_notifier's
doesn't seem *too* crazy to me.
Fixes: 70d932985757 ("notifier: Fix broken error handling pattern")
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Link: https://lkml.kernel.org/r/20201122201904.30940-1-valentin.schneider@arm.com
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
What do we do with this?
Do we merge this as-is, add another "robust atomic notifier" using only
raw_spinlock_t for registration and notification (for only
cpu_pm_notifier_chain) instead of switching to raw_spinlock_t for all
atomic notifier in -tree?
include/linux/notifier.h | 6 +++---
kernel/notifier.c | 12 ++++++------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 2fb373a5c1ede..723bc2df63882 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -58,7 +58,7 @@ struct notifier_block {
};
struct atomic_notifier_head {
- spinlock_t lock;
+ raw_spinlock_t lock;
struct notifier_block __rcu *head;
};
@@ -78,7 +78,7 @@ struct srcu_notifier_head {
};
#define ATOMIC_INIT_NOTIFIER_HEAD(name) do { \
- spin_lock_init(&(name)->lock); \
+ raw_spin_lock_init(&(name)->lock); \
(name)->head = NULL; \
} while (0)
#define BLOCKING_INIT_NOTIFIER_HEAD(name) do { \
@@ -95,7 +95,7 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
cleanup_srcu_struct(&(name)->srcu);
#define ATOMIC_NOTIFIER_INIT(name) { \
- .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \
.head = NULL }
#define BLOCKING_NOTIFIER_INIT(name) { \
.rwsem = __RWSEM_INITIALIZER((name).rwsem), \
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 1b019cbca594a..c20782f076432 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -142,9 +142,9 @@ int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
unsigned long flags;
int ret;
- spin_lock_irqsave(&nh->lock, flags);
+ raw_spin_lock_irqsave(&nh->lock, flags);
ret = notifier_chain_register(&nh->head, n);
- spin_unlock_irqrestore(&nh->lock, flags);
+ raw_spin_unlock_irqrestore(&nh->lock, flags);
return ret;
}
EXPORT_SYMBOL_GPL(atomic_notifier_chain_register);
@@ -164,9 +164,9 @@ int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
unsigned long flags;
int ret;
- spin_lock_irqsave(&nh->lock, flags);
+ raw_spin_lock_irqsave(&nh->lock, flags);
ret = notifier_chain_unregister(&nh->head, n);
- spin_unlock_irqrestore(&nh->lock, flags);
+ raw_spin_unlock_irqrestore(&nh->lock, flags);
synchronize_rcu();
return ret;
}
@@ -182,9 +182,9 @@ int atomic_notifier_call_chain_robust(struct atomic_notifier_head *nh,
* Musn't use RCU; because then the notifier list can
* change between the up and down traversal.
*/
- spin_lock_irqsave(&nh->lock, flags);
+ raw_spin_lock_irqsave(&nh->lock, flags);
ret = notifier_call_chain_robust(&nh->head, val_up, val_down, v);
- spin_unlock_irqrestore(&nh->lock, flags);
+ raw_spin_unlock_irqrestore(&nh->lock, flags);
return ret;
}
--
2.32.0
next reply other threads:[~2021-08-06 14:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-06 14:07 Sebastian Andrzej Siewior [this message]
2021-08-06 18:02 ` Peter Zijlstra
2021-08-06 18:06 ` Sebastian Andrzej Siewior
2021-08-06 18:20 ` Peter Zijlstra
-- strict thread matches above, loose matches on Subject: below --
2020-11-22 20:19 Valentin Schneider
2020-11-23 14:14 ` Peter Zijlstra
2020-11-23 14:52 ` Valentin Schneider
2020-11-30 10:09 ` Valentin Schneider
2020-11-30 13:51 ` Sebastian Andrzej Siewior
2020-11-30 13:55 ` Daniel Bristot de Oliveira
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=20210806140718.mxss3cbqijfebdo5@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=bristot@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rafael.j.wysocki@intel.com \
--cc=tglx@linutronix.de \
--cc=valentin.schneider@arm.com \
/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®