archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <>
To: Rasmus Villemoes <>,
	Song Hui <>,
	Bartosz Golaszewski <>
	LKML <>,
	Linus Walleij <>,
	Vladimir Oltean <>,
	"Steven Rostedt \(VMware\)" <>,
	Esben Haabendal <>
Subject: Re: commit 3d5bfbd97163 versus -rt
Date: Fri, 18 Jun 2021 22:04:07 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Jun 15 2021 at 14:35, Rasmus Villemoes wrote:
>  [<8016fb80>] (generic_handle_irq) from [<8043e98c>]
> (mpc8xxx_gpio_irq_cascade+0xac/0xd0)
>  [<8043e8e0>] (mpc8xxx_gpio_irq_cascade) from [<80171c80>]
> (irq_forced_thread_fn+0x38/0x8c)
>   r5:80e282c0 r4:80deda00
>  [<80171c48>] (irq_forced_thread_fn) from [<80171eb4>]
> (irq_thread+0x11c/0x238)
> Reverting commit 3d5bfbd9716318b1ca5c38488aa69f64d38a9aa5 (gpio:
> mpc8xxx: change the gpio interrupt flags.) makes it go away, as does
> disabling CONFIG_PREEMPT_RT or simply booting a vanilla v5.10.42 (where
> that option exists but cannot be selected).
> This seems to be the kind of thing where an -rt expert can immediately
> see what's wrong and how to fix it. Ideas anyone?

Let me explain. The force threaded interrupt code in mainline made the
wrong assumption that disabling softirqs is sufficient to provide the
semantics of non-threaded handler invocation. This turned out to be
wrong and caused people to do fancy workarounds.

See 81e2073c175b ("genirq: Disable interrupts for force threaded
handlers") for details.

So people started to remove IRQF_NO_THREAD or local_irq_save/restore
pairs in drivers.

But 3d5bfbd97163 ("gpio: mpc8xxx: change the gpio interrupt flags.") has
nothing to do with that:

    "Delete the interrupt IRQF_NO_THREAD flags in order to gpio interrupts
     can be threaded to allow high-priority processes to preempt."

This changelog is blatantly wrong. In mainline forced irq threads have
always been invoked with softirqs disabled, which obviously makes them
non-preemptible. And on RT this would have exploded exactly in the way
you observed.

The commit predates 81e2073c175b and therefore was obviously never
tested neither on mainline nor on RT.

Bartosz, please revert this ASAP.

I'll add a lockdep assert into generic_handle_irq() to make it obvious
where the problem is. That won't help with completely untested garbage
of course.

81e2073c175b is obviously a problem for demultiplexing drivers which
make weird assumptions when RT comes into play and I'm not surprised
that there is some fallout which we need to look at. Especially when
people start to zap IRQF_NO_THREAD or irq_save/restore pairs blindly.



  parent reply	other threads:[~2021-06-18 20:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 12:35 commit 3d5bfbd97163 versus -rt Rasmus Villemoes
2021-06-15 15:33 ` Steven Rostedt
2021-06-15 15:57   ` Rasmus Villemoes
2021-06-15 16:24     ` Rasmus Villemoes
2021-06-15 17:09       ` Steven Rostedt
2021-06-16  8:00         ` Oleksandr Natalenko
2021-06-18 20:04 ` Thomas Gleixner [this message]
2021-06-21  7:54   ` Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).