linux-kernel - Re: commit 3d5bfbd97163 versus -rt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <89f1be10-853b-1ee5-d4ef-48068c591af5@prevas.dk>
Date:   Mon, 21 Jun 2021 09:54:42 +0200
From:   Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Song Hui <hui.song_1@....com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Cc:     linux-rt-users@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Vladimir Oltean <vladimir.oltean@....com>,
        "Steven Rostedt (VMware)" <rostedt@...dmis.org>,
        Esben Haabendal <esben@...nix.com>
Subject: Re: commit 3d5bfbd97163 versus -rt

On 18/06/2021 22.04, Thomas Gleixner wrote:
> 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.

Thanks. Commit 81e2073c175b was included in 5.10.y per 5.10.26, and I
can confirm that booting a vanilla 5.10.25 with threadirqs does indeed
trigger the same splat I see on 5.10.41-rt42.

> Bartosz, please revert this ASAP.

Thanks for confirming that reverting 3d5bfbd97163 is the right thing to do.

Rasmus