linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/9] irq: add irq_set_chained_handler_and_data()
Date: Wed, 17 Jun 2015 20:38:03 +0100	[thread overview]
Message-ID: <20150617193803.GY7557@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <alpine.DEB.2.11.1506171609390.4107@nanos>

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@arm.linux.org.uk>
> > ---
> > 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.

      reply	other threads:[~2015-06-17 19:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-16 22:06 [PATCH 1/9] irq: add irq_set_chained_handler_and_data() Russell King
2015-06-16 22:29 ` [PATCH] GPU: ipu: fix lockup caused by pending chained interrupts Russell King
2015-06-19 14:36   ` Philipp Zabel
2015-06-17 14:10 ` [PATCH 1/9] irq: add irq_set_chained_handler_and_data() Thomas Gleixner
2015-06-17 19:38   ` Russell King - ARM Linux [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150617193803.GY7557@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).