diff mbox series

[v3] printk: nbcon: Fix illegal RCU usage on thread wakeup

Message ID ZyjaTu8ktE119iwW@debarbos-thinkpadt14sgen2i.remote.csb
State Superseded
Headers show
Series [v3] printk: nbcon: Fix illegal RCU usage on thread wakeup | expand

Commit Message

Derek Barbosa Nov. 4, 2024, 2:29 p.m. UTC 
In debug kernels, printk'ing during the SMP startup of a secondary CPU
results in a splat on boot that details illegal RCU usage from
offline CPUs.

This patch aligns rcuwait_has_sleeper() with what currently exists in
Torvalds' tree, at commit 76f258bf3f2aa, and will avoid the
aforementioned splat.

Furthermore, this patch makes it so that we avoid the use of hacky
rcu_derefrencing through the waiter's task struct,and instead leverages
the correct RCU API with the rcuwait_active() function.

Reported-by: Andrew Halaney <ahalaney@redhat.com>
Closes: https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: Derek Barbosa <debarbos@redhat.com>

---
 kernel/printk/nbcon.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

--

Comments

Sebastian Andrzej Siewior Nov. 4, 2024, 2:40 p.m. UTC | #1 
On 2024-11-04 09:29:34 [-0500], Derek Barbosa wrote:
> In debug kernels, printk'ing during the SMP startup of a secondary CPU
> results in a splat on boot that details illegal RCU usage from
> offline CPUs.
> 
> This patch aligns rcuwait_has_sleeper() with what currently exists in
> Torvalds' tree, at commit 76f258bf3f2aa, and will avoid the
> aforementioned splat.
> 
> Furthermore, this patch makes it so that we avoid the use of hacky
> rcu_derefrencing through the waiter's task struct,and instead leverages
> the correct RCU API with the rcuwait_active() function.
> 
> Reported-by: Andrew Halaney <ahalaney@redhat.com>
> Closes: https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Reviewed-by: John Ogness <john.ogness@linutronix.de>
> Signed-off-by: Derek Barbosa <debarbos@redhat.com>

  \o/

Sebastian
Andrew Halaney Nov. 4, 2024, 2:50 p.m. UTC | #2 
On Mon, Nov 04, 2024 at 09:29:34AM -0500, Derek Barbosa wrote:
> In debug kernels, printk'ing during the SMP startup of a secondary CPU
> results in a splat on boot that details illegal RCU usage from
> offline CPUs.
> 
> This patch aligns rcuwait_has_sleeper() with what currently exists in
> Torvalds' tree, at commit 76f258bf3f2aa, and will avoid the
> aforementioned splat.
> 
> Furthermore, this patch makes it so that we avoid the use of hacky
> rcu_derefrencing through the waiter's task struct,and instead leverages
> the correct RCU API with the rcuwait_active() function.
> 
> Reported-by: Andrew Halaney <ahalaney@redhat.com>
> Closes: https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Reviewed-by: John Ogness <john.ogness@linutronix.de>
> Signed-off-by: Derek Barbosa <debarbos@redhat.com>

Thanks for pushing this!

Reviewed-by: Andrew Halaney <ajhalaney@gmail.com>
diff mbox series

Patch

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index b941039ee7d2..579f71fffff4 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1101,9 +1101,6 @@  static void nbcon_irq_work(struct irq_work *irq_work)

 static inline bool rcuwait_has_sleeper(struct rcuwait *w)
 {
-       bool has_sleeper;
-
-       rcu_read_lock();
        /*
         * Guarantee any new records can be seen by tasks preparing to wait
         * before this context checks if the rcuwait is empty.
@@ -1116,10 +1113,7 @@  static inline bool rcuwait_has_sleeper(struct rcuwait *w)
         * This pairs with nbcon_kthread_func:A.
         */
        smp_mb(); /* LMM(rcuwait_has_sleeper:A) */
-       has_sleeper = !!rcu_dereference(w->task);
-       rcu_read_unlock();
-
-       return has_sleeper;
+       return rcuwait_active(w);
 }

 /**