linux-kernel - Re: [PATCH] irqchip/jcore: fix lost per-cpu interrupts

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]

Date:   Wed, 12 Oct 2016 10:18:02 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Rich Felker <dalias@...c.org>
cc:     linux-kernel@...r.kernel.org, linux-sh@...r.kernel.org,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH] irqchip/jcore: fix lost per-cpu interrupts

On Tue, 11 Oct 2016, Rich Felker wrote:
> On Sun, Oct 09, 2016 at 09:23:58PM +0200, Thomas Gleixner wrote:
> > On Sun, 9 Oct 2016, Rich Felker wrote:
> > > On Sun, Oct 09, 2016 at 01:03:10PM +0200, Thomas Gleixner wrote:
> > > My preference would just be to keep the branch, but with your improved
> > > version that doesn't need a function call:
> > > 
> > > 	irqd_is_per_cpu(irq_desc_get_irq_data(desc))
> > >
> > > While there is some overhead testing this condition every time, I can
> > > probably come up with several better places to look for a ~10 cycle
> > > improvement in the irq code path without imposing new requirements on
> > > the DT bindings.
> > 
> > Fair enough. Your call.
> >  
> > > As noted in my followup to the clocksource stall thread, there's also
> > > a possibility that it might make sense to consider the current
> > > behavior of having non-percpu irqs bound to a particular cpu as part
> > > of what's required by the compatible tag, in which case
> > > handle_percpu_irq or something similar/equivalent might be suitable
> > > for both the percpu and non-percpu cases. I don't understand the irq
> > > subsystem well enough to insist on that but I think it's worth
> > > consideration since it looks like it would improve performance of
> > > non-percpu interrupts a bit.
> > 
> > Well, you can use handle_percpu_irq() for your device interrupts if you
> > guarantee at the hardware level that there is no reentrancy. Once you make
> > the hardware capable of delivering them on either core the picture changes.
> 
> One more concern here -- I see that handle_simple_irq is handling the
> soft-disable / IRQS_PENDING flag behavior, and irq_check_poll stuff
> that's perhaps important too. Since soft-disable is all we have
> (there's no hard-disable of interrupts), is this a problem? In other
> words, can drivers have an expectation of not receiving interrupts
> when the irq is disabled? I would think anything compatible with irq
> sharing can't have such an expectation, but perhaps the kernel needs
> disabling internally for synchronization at module-unload time or
> similar cases?

Sure. A driver would be surprised getting an interrupt when it is disabled,
but with your exceptionally well thought out interrupt controller a pending
(level) interrupt which is not handled will be reraised forever and just
hard lock the machine.

> If you think any of these things are problems I'll switch back to the
> conditional version rather than using handle_percpu_irq for
> everything.

It might be the approach of least surprise, but it won't make a difference
for the situation described above.

Thanks,

	tglx