[PATCH] v6.6-rt-stable: printk/nbcon: Fix Illegal RCU Usage

[PATCH] v6.6-rt-stable: printk/nbcon: Fix Illegal RCU Usage

[Derek Barbosa]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

Hi!

On !PREEMPT_RT systems, printk-ing on a CPU that is coming online can produce a
nasty illegal RCU usage splat [0].

To fix this, use the correct RCU API in rcuwait_has_sleeper(), as seen in
Torvalds' tree at 76f258bf3f2aa.

I have appended a patch with a more detailed commit message.

Thanks,

Derek
[0] https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn/

[-- Attachment #2: 0001-PATCH-printk-nbcon-Fix-illegal-RCU-usage-on-thread-w.patch --]
[-- Type: text/plain, Size: 1769 bytes --]

From 1188b21c357dcb750a6851d7e5fd2d1f70ca9690 Mon Sep 17 00:00:00 2001
From: Derek Barbosa <debarbos@redhat.com>
Date: Mon, 21 Oct 2024 11:30:30 -0400
Subject: [PATCH 1/1] [PATCH] printk: nbcon: Fix illegal RCU usage on thread
 wakeup

In debug kernels, printk'ing during the SMP startup of a secondary CPU
results in a splat [0] 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.

[0] https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn/

Signed-off-by: Derek Barbosa <debarbos@redhat.com>
---
 kernel/printk/nbcon.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

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);
 }
 
 /**
-- 
2.46.0

Re: [PATCH] v6.6-rt-stable: printk/nbcon: Fix Illegal RCU Usage

[John Ogness]

On 2024-10-21, Derek Barbosa <debarbos@redhat.com> wrote:
> From 1188b21c357dcb750a6851d7e5fd2d1f70ca9690 Mon Sep 17 00:00:00 2001
> From: Derek Barbosa <debarbos@redhat.com>
> Date: Mon, 21 Oct 2024 11:30:30 -0400
> Subject: [PATCH 1/1] [PATCH] printk: nbcon: Fix illegal RCU usage on thread
>  wakeup
>
> In debug kernels, printk'ing during the SMP startup of a secondary CPU
> results in a splat [0] 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.
>
> [0] https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn/
>
> Signed-off-by: Derek Barbosa <debarbos@redhat.com>

Thanks!

Reviewed-by: John Ogness <john.ogness@linutronix.de>

Re: [PATCH] v6.6-rt-stable: printk/nbcon: Fix Illegal RCU Usage

[Clark Williams]

I'll queue this up for an upcoming v6.6-rt release

On Mon, Oct 21, 2024 at 8:56 PM John Ogness <john.ogness@linutronix.de> wrote:
>
> On 2024-10-21, Derek Barbosa <debarbos@redhat.com> wrote:
> > From 1188b21c357dcb750a6851d7e5fd2d1f70ca9690 Mon Sep 17 00:00:00 2001
> > From: Derek Barbosa <debarbos@redhat.com>
> > Date: Mon, 21 Oct 2024 11:30:30 -0400
> > Subject: [PATCH 1/1] [PATCH] printk: nbcon: Fix illegal RCU usage on thread
> >  wakeup
> >
> > In debug kernels, printk'ing during the SMP startup of a secondary CPU
> > results in a splat [0] 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.
> >
> > [0] https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn/
> >
> > Signed-off-by: Derek Barbosa <debarbos@redhat.com>
>
> Thanks!
>
> Reviewed-by: John Ogness <john.ogness@linutronix.de>
>

Re: [PATCH] v6.6-rt-stable: printk/nbcon: Fix Illegal RCU Usage

[Sebastian Andrzej Siewior]

On 2024-10-21 11:55:05 [-0400], Derek Barbosa wrote:
> From 1188b21c357dcb750a6851d7e5fd2d1f70ca9690 Mon Sep 17 00:00:00 2001
> From: Derek Barbosa <debarbos@redhat.com>
> Date: Mon, 21 Oct 2024 11:30:30 -0400
> Subject: [PATCH 1/1] [PATCH] printk: nbcon: Fix illegal RCU usage on thread
>  wakeup
> 
> In debug kernels, printk'ing during the SMP startup of a secondary CPU
> results in a splat [0] 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.
> 
> [0] https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn/

If you could replace the [0] notation with
 Reported-by: Andrew Halaney <ahalaney@redhat.com>
 Closes: https://lore.kernel.org/linux-rt-users/5x4nejpojrtny37k7l6loewqwuaituq77zc3tkkojvawcuvmml@thwg65fdb3vn

then it would make me happy.

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian