linux-kernel - Re: [PATCH] serial: imx: drop workaround for forced irq threading

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YFn9KenzUQl4KPRt@hovoldconsulting.com>
Date:   Tue, 23 Mar 2021 15:37:29 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
        Sam Nobs <samuel.nobs@...tradio.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] serial: imx: drop workaround for forced irq threading

On Mon, Mar 22, 2021 at 02:40:32PM +0100, Uwe Kleine-König wrote:
> Hello Johan,
> 
> On Mon, Mar 22, 2021 at 02:20:57PM +0100, Johan Hovold wrote:
> > On Mon, Mar 22, 2021 at 12:55:36PM +0100, Uwe Kleine-König wrote:
> > > On Mon, Mar 22, 2021 at 12:39:18PM +0100, Sebastian Andrzej Siewior wrote:
> > > > On 2021-03-22 12:34:02 [+0100], Uwe Kleine-König wrote:
> > > > > On Mon, Mar 22, 2021 at 12:10:36PM +0100, Johan Hovold wrote:
> > > > > > Force-threaded interrupt handlers used to run with interrupts enabled,
> > > > > > something which could lead to deadlocks in case a threaded handler
> > > > > > shared a lock with code running in hard interrupt context (e.g. timer
> > > > > > callbacks) and did not explicitly disable interrupts.
> > > > > > 
> > > > > > This was specifically the case for serial drivers that take the port
> > > > > > lock in their console write path as printk can be called from hard
> > > > > > interrupt context also with forced threading ("threadirqs").
> > > > > > 
> > > > > > Since commit 81e2073c175b ("genirq: Disable interrupts for force
> > > > > > threaded handlers") interrupt handlers always run with interrupts
> > > > > > disabled on non-RT so that drivers no longer need to do handle this.
> > > > > 
> > > > > So we're breaking RT knowingly here? If this is the case I'm not happy
> > > > > with your change. (And if RT is not affected a different wording would
> > > > > be good.)
> > > > 
> > > > Which wording, could you be more specific? It looks good from here and
> > > > no, RT is not affected.
> > > 
> > > The commit log says essentially: "The change is fine on non-RT" which
> > > suggests there is a problem on RT.
> > 
> > I don't think you can read that into the commit message.
> 
> From a strictly logically point of view you indeed cannot. But if you go
> to the street and say to people there that they can park their car in
> this street free of charge between Monday and Friday, I expect that most
> of them will assume that they have to pay for parking on weekends.

That analogy would almost seem to suggest bad intent on my side.

To say that this workaround is no longer needed on !RT does not imply
that it is needed on RT. If anything it suggests I have considered RT,
I'd say.

> So when you said that on on-RT the reason why it used to need a
> workaround is gone made me wonder what that implies for RT.

Fair enough. I thought it was obvious from the commit message and the
commits referred to, and that RT wouldn't depend on patching random
mainline drivers like this without a clear marking such as using raw
spin locks.

Greg's already picked this one up and this is hopefully the last one
we'll see of these.

Johan

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)