On Wed, Jun 17, 2015 at 04:10:02PM +0200, Thomas Gleixner wrote: > On Tue, 16 Jun 2015, Russell King wrote: > > > Driver authors seem to get the ordering of irq_set_chained_handler() > > and irq_set_handler_data() wrong - ordering the former before the > > latter. This opens a race window where, if there is an interrupt > > pending, the handler will be called between these two calls, > > potentially resulting in an oops. > > > > Provide a single interface to set both of these together, especially > > as that's commonly what is required. > > > > Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx> > > --- > > It probably makes sense to convert everything over to this new > > registration function, and kill all users of irq_set_chained_handler() > > to prevent this problem recurring. Thoughts? > > Yes. Classic coccinelle problem. Here is the semantic patch: > > @@ > expression E1, E2, E3; > @@ > -irq_set_chained_handler(E1, E3); > ... > -irq_set_handler_data(E1, E2); > +irq_set_chained_handler_and_data(E1, E3, E2); > > This finds and corrects all instances which get it wrong: > > arch: > arm/common/locomo.c | 3 +-- > arm/common/sa1111.c | 3 +-- > arm/mach-gemini/gpio.c | 4 ++-- > avr32/mach-at32ap/extint.c | 3 +-- > m68k/mac/psc.c | 12 ++++-------- > mips/ath25/ar2315.c | 4 ++-- > mips/ath25/ar5312.c | 4 ++-- > mips/pci/pci-ar2315.c | 4 ++-- > mips/ralink/irq.c | 3 +-- > sh/intc/core.c | 5 +++-- > > drivers: > dma/ipu/ipu_irq.c | 6 ++---- > gpio/gpio-bcm-kona.c | 5 +++-- > gpio/gpio-davinci.c | 10 ++-------- > gpio/gpio-dwapb.c | 4 ++-- > gpio/gpio-msic.c | 3 +-- > gpio/gpio-mxc.c | 10 +++++----- > gpio/gpio-mxs.c | 4 ++-- > gpio/gpio-tegra.c | 4 ++-- > gpu/ipu-v3/ipu-common.c | 13 +++++-------- > irqchip/irq-keystone.c | 4 ++-- > irqchip/spear-shirq.c | 3 +-- > mfd/pm8921-core.c | 6 ++---- > mfd/t7l66xb.c | 3 +-- > mfd/tc6393xb.c | 3 +-- > pci/host/pci-keystone.c | 7 +++---- > pinctrl/mediatek/pinctrl-mtk-common.c | 3 +-- > pinctrl/pinctrl-adi2.c | 4 ++-- > pinctrl/pinctrl-st.c | 4 ++-- > pinctrl/samsung/pinctrl-exynos.c | 4 ++-- > pinctrl/samsung/pinctrl-s3c24xx.c | 3 +-- > pinctrl/samsung/pinctrl-s3c64xx.c | 8 ++++---- > pinctrl/sunxi/pinctrl-sunxi.c | 6 +++--- > spmi/spmi-pmic-arb.c | 6 ++---- > 33 files changed, 70 insertions(+), 98 deletions(-) > > If we revert the search pattern we get the ones which got it right: > > @@ > expression E1, E2, E3; > @@ > -irq_set_handler_data(E1, E2); > ... > -irq_set_chained_handler(E1, E3); > +irq_set_chained_handler_and_data(E1, E3, E2); > > That handles another bunch and leaves us with 135 instances of > irq_set_chained_handler() which do not set handler data. So its > trivial to change them to > > irq_set_chained_handler_and_data(irq, handler, NULL); > > and then remove irq_set_chained_handler() > > If thats ok for you, then i apply the lot you sent and run the cocci > scripts right at rc1. I have another set of transformations in that > area pending. Totally fine with that. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
