On 1/12/23 14:13, Johan Hovold wrote:
On Mon, Jan 09, 2023 at 11:13:15AM +0100, Sebastian Andrzej Siewior wrote:
On 2022-12-27 15:56:47 [+0100], Johan Hovold wrote:
On Fri, Dec 16, 2022 at 12:53:38PM +0100, Marek Vasut wrote:
Requesting an interrupt with IRQF_ONESHOT will run the primary handler
in the hard-IRQ context even in the force-threaded mode. The
force-threaded mode is used by PREEMPT_RT in order to avoid acquiring
sleeping locks (spinlock_t) in hard-IRQ context. This combination
makes it impossible and leads to "sleeping while atomic" warnings.
Use one interrupt handler for both handlers (primary and secondary)
and drop the IRQF_ONESHOT flag which is not needed.
Fixes: e359b4411c283 ("serial: stm32: fix threaded interrupt handling")
I don't think a Fixes tag is warranted as this is only needed due to
this undocumented quirk of PREEMPT_RT.
It is not an undocumented quirk of PREEMPT_RT. The part that might not
be well documented is that IRQF_ONESHOT won't run the primary handler as
a threaded handler. This is also the case for IRQF_NO_THREAD and
IRQF_PERCPU but might be more obvious.
Yeah, that not being documented seems like an oversight as generally
drivers should not need be changed to continue working on PREEMPT_RT (or
with forced-threading generally).
Anyway, if the primary handler is not threaded then it runs in HARDIRQ
context and here you must not use a spinlock_t. This is documented in
Documentation/locking/locktypes.rst and there is also a LOCKDEP warning
for this on !RT which is off by default because it triggers with printk
(and this is worked on).
All interrupt handlers typically run in hard interrupt context unless
explicitly requested to be threaded on !RT and drivers still use
spinlock_t for that so not sure how that lockdep warning is supposed to
work. Or do you mean that you have a lockdep warning specifically for
IRQF_ONESHOT primary handlers?
And this should not be backported in any case.
Such things have been backported via -stable in the past. If you
disagree, please keep me in loop while is merged so I can poke people to
backport it as part of the RT patch for the relevant kernels.
The author did not seem to think this was stable material as there is no
cc-stable tag and it also seems fairly intrusive.
The author does not have enough experience with preempt-rt to make that
determination, hence deferred to Sebastian for better judgement.
But if the ST guys or whoever cares about this driver are fine with this
being backported, I don't really mind either.
[...]
