With PREEMPT_RT enabled, a spin_lock_irqsave() becomes a possibly sleeping
spin_lock(), without preempt_disable() or irq_disable().
This allows a task T1 to get preempted or interrupted while holding the
port->lock. If the preempting task T2 need the lock, spin_lock() code
will schedule T1 back until it finishes using the lock, and then go back to
T2.
There is an issue if a T1 holding port->lock is interrupted by an
IRQ, and this IRQ handler needs to get port->lock for writting (printk):
spin_lock() code will try to reschedule the interrupt handler, which is in
atomic context, causing a BUG() for trying to reschedule/sleep in atomic
context.
So for the case (PREEMPT_RT && in_atomic()) try to get the lock, and if it
fails proceed anyway, just like it's done in oops_in_progress case.
Signed-off-by: Leonardo Bras <leobras@redhat.com>
---
drivers/tty/serial/8250/8250_port.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 8ca061d3bbb92..8480832846319 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3397,7 +3397,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
touch_nmi_watchdog();
- if (oops_in_progress)
+ if (oops_in_progress || (IS_ENABLED(CONFIG_PREEMPT_RT) && in_atomic())
locked = uart_port_trylock_irqsave(port, &flags);
else
uart_port_lock_irqsave(port, &flags);
--
2.43.0On Tue, Jan 16 2024 at 04:37, Leonardo Bras wrote:
> With PREEMPT_RT enabled, a spin_lock_irqsave() becomes a possibly sleeping
> spin_lock(), without preempt_disable() or irq_disable().
>
> This allows a task T1 to get preempted or interrupted while holding the
> port->lock. If the preempting task T2 need the lock, spin_lock() code
> will schedule T1 back until it finishes using the lock, and then go back to
> T2.
>
> There is an issue if a T1 holding port->lock is interrupted by an
> IRQ, and this IRQ handler needs to get port->lock for writting (printk):
> spin_lock() code will try to reschedule the interrupt handler, which is in
> atomic context, causing a BUG() for trying to reschedule/sleep in atomic
> context.
>
> So for the case (PREEMPT_RT && in_atomic()) try to get the lock, and if it
> fails proceed anyway, just like it's done in oops_in_progress case.
That's just blantantly wrong. The locks are really only to be ignored
for the oops case, but not for regular printk.
I assume that this is not against the latest RT kernel as that should
not have that problem at all.
Thanks,
tglx
On Wed, Jan 17, 2024 at 11:44:55PM +0100, Thomas Gleixner wrote: > On Tue, Jan 16 2024 at 04:37, Leonardo Bras wrote: > > With PREEMPT_RT enabled, a spin_lock_irqsave() becomes a possibly sleeping > > spin_lock(), without preempt_disable() or irq_disable(). > > > > This allows a task T1 to get preempted or interrupted while holding the > > port->lock. If the preempting task T2 need the lock, spin_lock() code > > will schedule T1 back until it finishes using the lock, and then go back to > > T2. > > > > There is an issue if a T1 holding port->lock is interrupted by an > > IRQ, and this IRQ handler needs to get port->lock for writting (printk): > > spin_lock() code will try to reschedule the interrupt handler, which is in > > atomic context, causing a BUG() for trying to reschedule/sleep in atomic > > context. > > > > So for the case (PREEMPT_RT && in_atomic()) try to get the lock, and if it > > fails proceed anyway, just like it's done in oops_in_progress case. > > That's just blantantly wrong. The locks are really only to be ignored > for the oops case, but not for regular printk. I agree, but the alternative was to have a BUG() due to scheduling in atomic context. This would only ignore the lock if it was already taken anyway. That being said, I agree it is not the best solution for the issue, and just sent this in the RFC in order to get feedback on what could be done. > > I assume that this is not against the latest RT kernel as that should > not have that problem at all. I am based on torvalds/linux at master branch, so maybe I am missing some RT-specific patches. Which tree do you recommend me testing? > > Thanks, > > tglx > Thank you! Leo
On Tue, 16 Jan 2024, Leonardo Bras wrote: > With PREEMPT_RT enabled, a spin_lock_irqsave() becomes a possibly sleeping > spin_lock(), without preempt_disable() or irq_disable(). > > This allows a task T1 to get preempted or interrupted while holding the > port->lock. If the preempting task T2 need the lock, spin_lock() code > will schedule T1 back until it finishes using the lock, and then go back to > T2. > > There is an issue if a T1 holding port->lock is interrupted by an > IRQ, and this IRQ handler needs to get port->lock for writting (printk): > spin_lock() code will try to reschedule the interrupt handler, which is in > atomic context, causing a BUG() for trying to reschedule/sleep in atomic > context. I thought that the printk side was supposed to be become aware when it's not safe to write to serial side so the printing can be deferred... Has that plan changed? -- i. > So for the case (PREEMPT_RT && in_atomic()) try to get the lock, and if it > fails proceed anyway, just like it's done in oops_in_progress case. > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > --- > drivers/tty/serial/8250/8250_port.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index 8ca061d3bbb92..8480832846319 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -3397,7 +3397,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, > > touch_nmi_watchdog(); > > - if (oops_in_progress) > + if (oops_in_progress || (IS_ENABLED(CONFIG_PREEMPT_RT) && in_atomic()) > locked = uart_port_trylock_irqsave(port, &flags); > else > uart_port_lock_irqsave(port, &flags); >
On Tue, Jan 16, 2024 at 10:48:44AM +0200, Ilpo Järvinen wrote: > On Tue, 16 Jan 2024, Leonardo Bras wrote: > > > With PREEMPT_RT enabled, a spin_lock_irqsave() becomes a possibly sleeping > > spin_lock(), without preempt_disable() or irq_disable(). > > > > This allows a task T1 to get preempted or interrupted while holding the > > port->lock. If the preempting task T2 need the lock, spin_lock() code > > will schedule T1 back until it finishes using the lock, and then go back to > > T2. > > > > There is an issue if a T1 holding port->lock is interrupted by an > > IRQ, and this IRQ handler needs to get port->lock for writting (printk): > > spin_lock() code will try to reschedule the interrupt handler, which is in > > atomic context, causing a BUG() for trying to reschedule/sleep in atomic > > context. Hello Ilpo, thanks for replying! > > I thought that the printk side was supposed to be become aware when it's > not safe to write to serial side so the printing can be deferred... Has > that plan changed? > > -- > i. I was not aware of this plan. Well, at least in an PREEMPT_RT=y kernel I have found this same bug reproducing several times, and through the debugging that I went through I saw no mechanism for preventing it. This is one example of the bug: While writing to serial with serial8250_tx_chars in a irq_thread handler there is an interruption, and __report_bad_irq() tries to printk stuff to the console, and when printk goes down to serial8250_console_write() and tried to get the port->lock, which causes the "BUG: scheduling while atomic": [ 42.485878] irq 4: nobody cared (try booting with the "irqpoll" option) [ 42.485886] BUG: scheduling while atomic: irq/4-ttyS0/751/0x00010002 [ 42.485890] Modules linked in: [ 42.485892] Preemption disabled at: [ 42.485893] [<ffffffff8118ac80>] irq_enter_rcu+0x10/0x80 [ 42.485919] CPU: 0 PID: 751 Comm: irq/4-ttyS0 Not tainted 6.7.0-rc6+ #6 [ 42.485927] Hardware name: Red Hat KVM/RHEL, BIOS 1.16.3-1.el9 04/01/2014 [ 42.485929] Call Trace: [ 42.485940] <IRQ> [ 42.485944] dump_stack_lvl+0x33/0x50 [ 42.485976] __schedule_bug+0x89/0xa0 [ 42.485991] schedule_debug.constprop.0+0xd1/0x120 [ 42.485996] __schedule+0x50/0x690 [ 42.486026] schedule_rtlock+0x1e/0x40 [ 42.486029] rtlock_slowlock_locked+0xe7/0x2b0 [ 42.486047] rt_spin_lock+0x41/0x60 [ 42.486051] serial8250_console_write+0x1be/0x460 [ 42.486094] console_flush_all+0x18d/0x3c0 [ 42.486111] console_unlock+0x6c/0xd0 [ 42.486117] vprintk_emit+0x1d6/0x290 [ 42.486122] _printk+0x58/0x80 [ 42.486139] __report_bad_irq+0x26/0xc0 [ 42.486147] note_interrupt+0x2a1/0x2f0 [ 42.486155] handle_irq_event+0x84/0x90 [ 42.486161] handle_edge_irq+0x9f/0x260 [ 42.486168] __common_interrupt+0x68/0x100 [ 42.486178] common_interrupt+0x9f/0xc0 [ 42.486184] </IRQ> Thanks! Leo > > > So for the case (PREEMPT_RT && in_atomic()) try to get the lock, and if it > > fails proceed anyway, just like it's done in oops_in_progress case. > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > > --- > > drivers/tty/serial/8250/8250_port.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > > index 8ca061d3bbb92..8480832846319 100644 > > --- a/drivers/tty/serial/8250/8250_port.c > > +++ b/drivers/tty/serial/8250/8250_port.c > > @@ -3397,7 +3397,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, > > > > touch_nmi_watchdog(); > > > > - if (oops_in_progress) > > + if (oops_in_progress || (IS_ENABLED(CONFIG_PREEMPT_RT) && in_atomic()) > > locked = uart_port_trylock_irqsave(port, &flags); > > else > > uart_port_lock_irqsave(port, &flags); > > >
On 2024-01-16, Leonardo Bras <leobras@redhat.com> wrote: > Well, at least in an PREEMPT_RT=y kernel I have found this same bug > reproducing several times, and through the debugging that I went through I > saw no mechanism for preventing it. > > This is one example of the bug: > While writing to serial with serial8250_tx_chars in a irq_thread handler > there is an interruption, and __report_bad_irq() tries to printk > stuff to the console, and when printk goes down to > serial8250_console_write() and tried to get the port->lock, which causes > the "BUG: scheduling while atomic": > > [ 42.485878] irq 4: nobody cared (try booting with the "irqpoll" option) > [ 42.485886] BUG: scheduling while atomic: irq/4-ttyS0/751/0x00010002 > [ 42.485890] Modules linked in: > [ 42.485892] Preemption disabled at: > [ 42.485893] [<ffffffff8118ac80>] irq_enter_rcu+0x10/0x80 > [ 42.485919] CPU: 0 PID: 751 Comm: irq/4-ttyS0 Not tainted 6.7.0-rc6+ #6 This is 6.7.0-rc6+. How are you setting PREEMPT_RT? > [ 42.485927] Hardware name: Red Hat KVM/RHEL, BIOS 1.16.3-1.el9 04/01/2014 > [ 42.485929] Call Trace: > [ 42.485940] <IRQ> > [ 42.485944] dump_stack_lvl+0x33/0x50 > [ 42.485976] __schedule_bug+0x89/0xa0 > [ 42.485991] schedule_debug.constprop.0+0xd1/0x120 > [ 42.485996] __schedule+0x50/0x690 > [ 42.486026] schedule_rtlock+0x1e/0x40 > [ 42.486029] rtlock_slowlock_locked+0xe7/0x2b0 > [ 42.486047] rt_spin_lock+0x41/0x60 > [ 42.486051] serial8250_console_write+0x1be/0x460 On PREEMPT_RT-patched kernel, serial8250_console_write() is not compiled. So obviously you are not running a PREEMPT_RT-patched kernel. > [ 42.486094] console_flush_all+0x18d/0x3c0 > [ 42.486111] console_unlock+0x6c/0xd0 Flushing on console_unlock() is the legacy method. I assume you are using a mainline kernel with forced threading of irqs. Mainline has many known problems with console printing, including calling printk when the port->lock is held. This has been discussed before [0]. > [ 42.486117] vprintk_emit+0x1d6/0x290 > [ 42.486122] _printk+0x58/0x80 > [ 42.486139] __report_bad_irq+0x26/0xc0 > [ 42.486147] note_interrupt+0x2a1/0x2f0 > [ 42.486155] handle_irq_event+0x84/0x90 > [ 42.486161] handle_edge_irq+0x9f/0x260 > [ 42.486168] __common_interrupt+0x68/0x100 > [ 42.486178] common_interrupt+0x9f/0xc0 > [ 42.486184] </IRQ> If you want to fix any threaded irq problems relating to printk and console drivers, please use the latest PREEMPT_RT patch series with CONFIG_PREEMPT_RT enabled. This is the current work that is being reviewed on LKML for mainline inclusion. Thanks! John Ogness [0] https://lore.kernel.org/lkml/87il5o32w9.fsf@jogness.linutronix.de
On Thu, Jan 18, 2024 at 10:07:45AM +0106, John Ogness wrote:
> On 2024-01-16, Leonardo Bras <leobras@redhat.com> wrote:
> > Well, at least in an PREEMPT_RT=y kernel I have found this same bug
> > reproducing several times, and through the debugging that I went through I
> > saw no mechanism for preventing it.
> >
> > This is one example of the bug:
> > While writing to serial with serial8250_tx_chars in a irq_thread handler
> > there is an interruption, and __report_bad_irq() tries to printk
> > stuff to the console, and when printk goes down to
> > serial8250_console_write() and tried to get the port->lock, which causes
> > the "BUG: scheduling while atomic":
> >
> > [ 42.485878] irq 4: nobody cared (try booting with the "irqpoll" option)
> > [ 42.485886] BUG: scheduling while atomic: irq/4-ttyS0/751/0x00010002
> > [ 42.485890] Modules linked in:
> > [ 42.485892] Preemption disabled at:
> > [ 42.485893] [<ffffffff8118ac80>] irq_enter_rcu+0x10/0x80
> > [ 42.485919] CPU: 0 PID: 751 Comm: irq/4-ttyS0 Not tainted 6.7.0-rc6+ #6
>
> This is 6.7.0-rc6+. How are you setting PREEMPT_RT?
By setting ARCH_SUPPORTS_RT=y
>
> > [ 42.485927] Hardware name: Red Hat KVM/RHEL, BIOS 1.16.3-1.el9 04/01/2014
> > [ 42.485929] Call Trace:
> > [ 42.485940] <IRQ>
> > [ 42.485944] dump_stack_lvl+0x33/0x50
> > [ 42.485976] __schedule_bug+0x89/0xa0
> > [ 42.485991] schedule_debug.constprop.0+0xd1/0x120
> > [ 42.485996] __schedule+0x50/0x690
> > [ 42.486026] schedule_rtlock+0x1e/0x40
> > [ 42.486029] rtlock_slowlock_locked+0xe7/0x2b0
> > [ 42.486047] rt_spin_lock+0x41/0x60
> > [ 42.486051] serial8250_console_write+0x1be/0x460
>
> On PREEMPT_RT-patched kernel, serial8250_console_write() is not
> compiled. So obviously you are not running a PREEMPT_RT-patched kernel.
Yes, as mentioned to Thomas before, I am on Vanilla torvalds/linux.
I was not aware of any extra patches for PREEMPT_RT, could you please point
the repo, or the patchset for that PREEMPT_RT-patched version?
>
> > [ 42.486094] console_flush_all+0x18d/0x3c0
> > [ 42.486111] console_unlock+0x6c/0xd0
>
> Flushing on console_unlock() is the legacy method.
Great! so this part is solved :)
>
> I assume you are using a mainline kernel with forced threading of
> irqs. Mainline has many known problems with console printing, including
> calling printk when the port->lock is held.
I am using mainline (torvalds/linux) kernel, forcing ARCH_SUPPORTS_RT:
diff --git a/arch/Kconfig b/arch/Kconfig
index 5ca66aad0d08..879c34398cb7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1195,7 +1195,7 @@ config ARCH_NO_PREEMPT
bool
config ARCH_SUPPORTS_RT
- bool
+ def_bool y
Since I was not aware of a PREEMPT_RT-patched tree, I did this so I could
compile a PREEMPT_RT kernel.
>
> This has been discussed before [0].
>
> > [ 42.486117] vprintk_emit+0x1d6/0x290
> > [ 42.486122] _printk+0x58/0x80
> > [ 42.486139] __report_bad_irq+0x26/0xc0
> > [ 42.486147] note_interrupt+0x2a1/0x2f0
> > [ 42.486155] handle_irq_event+0x84/0x90
> > [ 42.486161] handle_edge_irq+0x9f/0x260
> > [ 42.486168] __common_interrupt+0x68/0x100
> > [ 42.486178] common_interrupt+0x9f/0xc0
> > [ 42.486184] </IRQ>
>
> If you want to fix any threaded irq problems relating to printk and
> console drivers, please use the latest PREEMPT_RT patch series with
> CONFIG_PREEMPT_RT enabled. This is the current work that is being
> reviewed on LKML for mainline inclusion. Thanks!
>
Sure, please let me know of where can I find the latest PREEMPT_RT patch
series so I can re-test my bug. By what you comment, it's higly probable
that patch 2/2 will not be necessary.
On the other hand, unless some extra work was done in preventing the
scenario in patch 1/2, I think that can still be discussed.
> John Ogness
>
> [0] https://lore.kernel.org/lkml/87il5o32w9.fsf@jogness.linutronix.de
>
Thanks!
Leo
On 18. 01. 24, 10:36, Leonardo Bras wrote: > I am using mainline (torvalds/linux) kernel, forcing ARCH_SUPPORTS_RT: > > diff --git a/arch/Kconfig b/arch/Kconfig > index 5ca66aad0d08..879c34398cb7 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -1195,7 +1195,7 @@ config ARCH_NO_PREEMPT > bool > > config ARCH_SUPPORTS_RT > - bool > + def_bool y > > Since I was not aware of a PREEMPT_RT-patched tree, I did this so I could > compile a PREEMPT_RT kernel. Huh, when exactly did you intend to mention this? -- js suse labs
On Thu, Jan 18, 2024 at 11:33:08AM +0100, Jiri Slaby wrote: > On 18. 01. 24, 10:36, Leonardo Bras wrote: > > I am using mainline (torvalds/linux) kernel, forcing ARCH_SUPPORTS_RT: > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 5ca66aad0d08..879c34398cb7 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -1195,7 +1195,7 @@ config ARCH_NO_PREEMPT > > bool > > config ARCH_SUPPORTS_RT > > - bool > > + def_bool y > > > > Since I was not aware of a PREEMPT_RT-patched tree, I did this so I could > > compile a PREEMPT_RT kernel. > > Huh, when exactly did you intend to mention this? Since I was not aware of an PREEMPT_RT-patched tree, I thought that this was the vanilla way to get a PREEMPT_RT kernel running. TBH I did not even though that there were an external repo for PREEMPT_RT. I mean, I knew about non-mainline patches in the past, but I thought everything got already merged upstream, and any other patches would be WIP. I understand this was a mistake on my part, and I feel sorry if this brought any pain to reviewers. For the future, I will be basing my RT work in this RT-devel tree shared by John. Thanks! Leo > > -- > js > suse labs >
On 2024-01-18, Leonardo Bras <leobras@redhat.com> wrote: > Sure, please let me know of where can I find the latest PREEMPT_RT > patch series so I can re-test my bug. By what you comment, it's higly > probable that patch 2/2 will not be necessary. Some links for you: The Real-Time Wiki at the Linux Foundation: https://wiki.linuxfoundation.org/realtime/ The latest development RT patch series for 6.7: https://cdn.kernel.org/pub/linux/kernel/projects/rt/6.7/patches-6.7-rt6.tar.xz RT git (branch linux-6.7.y-rt-rebase is probably what you want): https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git > On the other hand, unless some extra work was done in preventing the > scenario in patch 1/2, I think that can still be discussed. I agree. Thanks for looking into this. John
On Thu, Jan 18, 2024 at 11:33:04AM +0106, John Ogness wrote: > On 2024-01-18, Leonardo Bras <leobras@redhat.com> wrote: > > Sure, please let me know of where can I find the latest PREEMPT_RT > > patch series so I can re-test my bug. By what you comment, it's higly > > probable that patch 2/2 will not be necessary. > > Some links for you: > > > The Real-Time Wiki at the Linux Foundation: > > https://wiki.linuxfoundation.org/realtime/ > > > The latest development RT patch series for 6.7: > > https://cdn.kernel.org/pub/linux/kernel/projects/rt/6.7/patches-6.7-rt6.tar.xz > > > RT git (branch linux-6.7.y-rt-rebase is probably what you want): > > https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git > Hello John, thank you for sharing the links! > > > On the other hand, unless some extra work was done in preventing the > > scenario in patch 1/2, I think that can still be discussed. > > I agree. Thanks for looking into this. > > John > Thank you! Leo
© 2016 - 2025 Red Hat, Inc.