Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932491AbdIFOCK (ORCPT ); Wed, 6 Sep 2017 10:02:10 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:61727 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932308AbdIFOCF (ORCPT ); Wed, 6 Sep 2017 10:02:05 -0400 From: Paul Burton To: Thomas Gleixner CC: LKML , jeffy , Brian Norris , Marc Zyngier , , , Subject: Re: [2/2] genirq: Warn when IRQ_NOAUTOEN is used with shared interrupts Date: Wed, 6 Sep 2017 07:01:54 -0700 Message-ID: <2582391.E0BmZJ6Wil@np-p-burton> Organization: Imagination Technologies In-Reply-To: References: <20170531100212.210682135@linutronix.de> <45610923.Yru6qcift9@np-p-burton> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart250799053.iOmzJSugQd"; micalg=pgp-sha256; protocol="application/pgp-signature" X-Originating-IP: [10.20.78.200] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4422 Lines: 106 --nextPart250799053.iOmzJSugQd Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Hi Thomas, On Wednesday, 6 September 2017 01:16:48 PDT Thomas Gleixner wrote: > On Tue, 5 Sep 2017, Paul Burton wrote: > > I'm currently attempting to clean up a hack that we have in the MIPS GIC > > irqchip driver - we have some interrupts which are really per-CPU, but are > > currently used with the regular non-per-CPU IRQ APIs. Please search for > > usage of gic_all_vpes_local_irq_controller (or for the string "HACK") in > > drivers/ irqchip/irq-mips-gic.c if you wish to find what I'm talking > > about. The important details are that the interrupts in question are both > > per-CPU and on many systems are shared (between the CPU timer, > > performance counters & fast debug channel). > > > > I have been attempting to move towards using the per-CPU APIs instead in > > order to remove this hack - ie. using setup_percpu_irq() & > > enable_percpu_irq() in> > > place of plain old setup_irq(). Unfortunately what I've run into is this: > > - Per-CPU interrupts get the IRQ_NOAUTOEN flag set by default, in > > > > irq_set_percpu_devid_flags(). I can see why this makes sense in the > > general case, since the alternative is setup_percpu_irq() enabling the > > interrupt on the CPU that calls it & leaving it disabled on others, > > which > > feels a little unclean. > > > > - Your warning above triggers when a shared interrupt has the > > IRQ_NOAUTOEN > > > > flag set. I can see why your warning makes sense if another driver has > > already enabled the shared interrupt, which would make IRQ_NOAUTOEN > > ineffective. I'm not sure I follow your comment above the warning > > though - > > it sounds like you're trying to describe something else? > > > > > > + /* > > > + * Shared interrupts do not go well with disabling > > > + * auto enable. The sharing interrupt might request > > > + * it while it's still disabled and then wait for > > > + * interrupts forever. > > > + */ > > Assume the following: > > request_irq(X, handler1, NOAUTOEN|SHARED, dev1); > > now the second device does: > > request_irq(X, handler2, SHARED, dev2): > > which will see the first handler installed, so it wont run into the code > path which starts up the interrupt. That means as long as dev1 does not > explicitely enable the interrupt dev2 will wait for it forever. Ok, makes sense. > > For my interrupts which are both per-CPU & shared the combination of these > > 2> > > facts mean I end up triggering your warning. My current ideas include: > > - I could clear the IRQ_NOAUTOEN flag before calling setup_percpu_irq(). > > In > > > > my cases that should be fine - we call enable_percpu_irq() anyway, and > > would just enable the IRQ slightly earlier on the CPU which calls > > setup_percpu_irq() which wouldn't be a problem. It feels a bit yucky > > though. > > What's the problem with IRQ_NOAUTOEN and do > > setup_percpu_irq(); > enable_percpu_irq(); > > on the boot CPU and then later call it when the secondary CPUs come up in > cpu bringup code or a hotplug state callback? There's no problem with that at all, apart from that it triggers your warning when the boot CPU calls setup_percpu_irq(). Thanks, Paul --nextPart250799053.iOmzJSugQd Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEELIGR03D5+Fg+69wPgiDZ+mk8HGUFAlmv/9IACgkQgiDZ+mk8 HGXoBw/9HaQo0BaAMEAPjFkKS04U9l3usnMRG7Z8TfgHVCsuLu7PEyckWRq/sKiW N+ujDZZubNg9yUUlHD62FIg0Pna5eEdP7M1VkyjkQJuqSVzIr/LuE8z6ps4hAy+8 cPrMyN6IhOxstwLgDb381hBXefbFa4SgEV/c/ZHCHXA/1gwqLd5vIKYzcwtres+Q olztegt/FehwSPkPJS2wHWzxgTw58uje9m/7Iy7eRWT179f+s9SxCje7KZ5nQnrw R6YECUQ/KgSwZIyfeI3yGiEky5cdTnkQLBoWVpA55O0cs6ZXNXj2kSLrBLwdhY2D CS3vHyFtla/w9faCEUyYXzQW2DeKU7tp1VBRULGZj+kxbB90Za6lVJESFZrB24cR He2DsRqrD7Ke0oKSWhWs6wTWCh4Wx6lvK66E1UDJV1PGkSvj2YAVoZ95gOkV4hqy FH3dNCXkwKAa2cJZXbzhJCbIDU4LBAM+dK67tWM7UpkMPKIti19oodQmI83Y9dD0 y11EVShratmxT3k0DsDVGxnJgTts1pnyZk8JyBzYiyV+2RNExKcs3WE0aSbeOL3F yEJNQZiI3xmszYimze/tFPs/0Q/u/6GvuMRWnRroponmeZfc0KpZw6hyOAc5uRcG 92d7LdEW/G6BOo/6vnSINUXtZhjR5/MukEcMkfMwb+Lc5yGgOpI= =Mkj2 -----END PGP SIGNATURE----- --nextPart250799053.iOmzJSugQd--