On Fri, Mar 19, 2021 at 06:36:39AM +0000, Song Bao Hua (Barry Song) wrote:

quoted

-----Original Message-----
From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
Sent: Tuesday, March 16, 2021 10:41 PM
To: Johan Hovold <johan@kernel.org>; Finn Thain <redacted>;
Song Bao Hua (Barry Song) [off-list ref]
Cc: Krzysztof Kozlowski <redacted>; Greg
Kroah-Hartman [off-list ref]; Jiri Slaby [off-list ref];
linux-arm Mailing List [off-list ref]; Linux Samsung
SOC [off-list ref]; open list:SERIAL DRIVERS
[off-list ref]; Linux Kernel Mailing List
[off-list ref]; Hector Martin [off-list ref]; Arnd
Bergmann [off-list ref]
Subject: Re: [PATCH] tty: serial: samsung_tty: remove spinlock flags in
interrupt handlers

On Tue, Mar 16, 2021 at 11:02 AM Johan Hovold [off-list ref] wrote:

quoted

On Mon, Mar 15, 2021 at 07:12:12PM +0100, Krzysztof Kozlowski wrote:

quoted

Since interrupt handler is called with disabled local interrupts, there
is no need to use the spinlock primitives disabling interrupts as well.

This isn't generally true due to "threadirqs" and that can lead to
deadlocks if the console code is called from hard irq context.

Now, this is *not* the case for this particular driver since it doesn't
even bother to take the port lock in console_write(). That should
probably be fixed instead.

See https://lore.kernel.org/r/X7kviiRwuxvPxC8O@localhost (local).

Finn, Barry, something to check I think?

My understanding is that spin_lock_irqsave can't protect the context
the console_write() is called in hardirq for threaded_irq case mainly
for preempt-rt scenarios as spin_lock_irqsave doesn't disable irq in
that case at all.

Forced threaded interrupts have so far run with interrupts enabled and
spin_lock_irqsave() would suffice on non-RT. This is about to change
though so that drivers don't need to worry about "threadirqs":

	https://lore.kernel.org/r/20210317143859.513307808@linutronix.de (local)

See:
https://www.kernel.org/doc/html/latest/locking/locktypes.html
spinlock_t and PREEMPT_RT
On a PREEMPT_RT kernel spinlock_t is mapped to a separate implementation
based on rt_mutex which changes the semantics:
Preemption is not disabled.
The hard interrupt related suffixes for spin_lock / spin_unlock operations
(_irq, _irqsave / _irqrestore) do not affect the CPU’s interrupt disabled
state.

So if console_write() can interrupt our code in hardirq, we should
move to raw_spin_lock_irqsave for this driver.

No, no. RT handles this by deferring console writes apparently.

I think it is almost always wrong to call spin_lock_irqsave in hardirq.

Again, no. It's even been a requirement due to "threadirqs" in some
cases (e.g. hrtimers) up until now (or rather until the above patch is
in mainline).

Johan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel