Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: "BUG: Invalid wait context" in ls_extirq_set_type
To:     Vladimir Oltean <olteanv@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Lee Jones <lee.jones@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>, linux-kernel@vger.kernel.org
References: <20210825135438.ubcuxm5vctt6ne2q@skbuf>
Cc:     Esben Haabendal <esben@geanix.com>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
Message-ID: <26de7b85-e466-e9af-077a-9d1dc087e061@rasmusvillemoes.dk>
Date:   Thu, 26 Aug 2021 11:01:31 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <20210825135438.ubcuxm5vctt6ne2q@skbuf>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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