Re: "BUG: Invalid wait context" in ls_extirq_set_type

[Rasmus Villemoes <linux@rasmusvillemoes.dk>]

On 25/08/2021 15.54, Vladimir Oltean wrote:
> Hi,
> 
> Apologies for my novice level of understanding. I see a stack trace on
> my system and would like to understand what is the correct way to get
> rid of it.
> 
> I have a consumer of the drivers/irqchip/irq-ls-extirq.c driver which
> calls request_threaded_irq.
> 
> struct irq_desc has a lock which is a raw spinlock.
> The __setup_irq function takes this desc->lock raw spinlock, then calls
> __irq_set_trigger. Finally this calls chip->irq_set_type which is
> implemented by ls_extirq_set_type.
> 
> The problem is that ls_extirq_set_type uses regmap_update_bits, which
> ends up taking a non-raw spin lock, the kind that becomes sleepable on RT
> (this system is not RT, but still).
> So that's kind of bad, and this is what the stack trace below is saying:
> 

So, we've been using the irq-ls-extirq driver for years, on an RT
kernel, without seeing something like that. Does it require certain
debug knobs in .config? Or perhaps the sanity checks have been added
later, we've mostly been using it on 4.14.y and 4.19.y.

I don't know what the right fix is. Am I right when a say that for !RT,
spinlock==raw_spinlock? If so, switching regmap's spinlock to
raw_spinlock would be nop for !RT and fix this issue, but would of
course have quite far-reaching effects on RT kernels.

Perhaps it's silly for the driver to use syscon's regmap to access that
single register, maybe it should just iomap it itself, and protect
access with a raw_spinlock of its own. While syscon's regmap would cover
that intpcr register, nobody else would ever access it, so that should
work fine.

Then there's your suggestion. While there's nothing wrong with adding
raw_spinlock lock support in regmap, the fact that nobody has needed
that till now suggests that one should at least pause and think about
other options. And you point out the weaknesses of basing the
.use_raw_spinlock on a .compatible string yourself.

> What to do?

TL;DR: I don't know.

Rasmus