Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1485727 > unrolled thread

[GIT pull] irq fixes for 4.8

Back to article view | Back to linux.kernel

Contents

  [GIT pull] irq fixes for 4.8 Thomas Gleixner <tglx@linutronix.de> - 2016-09-18 10:10 +0200
    Re: [GIT pull] irq fixes for 4.8 Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-18 20:20 +0200
      Re: [GIT pull] irq fixes for 4.8 Thomas Gleixner <tglx@linutronix.de> - 2016-09-19 09:20 +0200

#1485727 — [GIT pull] irq fixes for 4.8

Linus,

please pull the latest irq-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-urgent-for-linus

Two patches from Boris which address a potential deadlock in the atmel irq
chip driver.

Thanks,

	tglx

------------------>
Boris Brezillon (2):
      genirq: Provide irq_gc_{lock_irqsave,unlock_irqrestore}() helpers
      irqchip/atmel-aic: Fix potential deadlock in ->xlate()


 drivers/irqchip/irq-atmel-aic.c  |  5 +++--
 drivers/irqchip/irq-atmel-aic5.c |  5 +++--
 include/linux/irq.h              | 10 ++++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 112e17c2768b..37f952dd9fc9 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -176,6 +176,7 @@ static int aic_irq_domain_xlate(struct irq_domain *d,
 {
 	struct irq_domain_chip_generic *dgc = d->gc;
 	struct irq_chip_generic *gc;
+	unsigned long flags;
 	unsigned smr;
 	int idx;
 	int ret;
@@ -194,11 +195,11 @@ static int aic_irq_domain_xlate(struct irq_domain *d,
 
 	gc = dgc->gc[idx];
 
-	irq_gc_lock(gc);
+	irq_gc_lock_irqsave(gc, flags);
 	smr = irq_reg_readl(gc, AT91_AIC_SMR(*out_hwirq));
 	aic_common_set_priority(intspec[2], &smr);
 	irq_reg_writel(gc, smr, AT91_AIC_SMR(*out_hwirq));
-	irq_gc_unlock(gc);
+	irq_gc_unlock_irqrestore(gc, flags);
 
 	return ret;
 }
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 4f0d068e1abe..2a624d87a035 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -258,6 +258,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 				 unsigned int *out_type)
 {
 	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(d, 0);
+	unsigned long flags;
 	unsigned smr;
 	int ret;
 
@@ -269,12 +270,12 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
-	irq_gc_lock(bgc);
+	irq_gc_lock_irqsave(bgc, flags);
 	irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
 	smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
 	aic_common_set_priority(intspec[2], &smr);
 	irq_reg_writel(bgc, smr, AT91_AIC5_SMR);
-	irq_gc_unlock(bgc);
+	irq_gc_unlock_irqrestore(bgc, flags);
 
 	return ret;
 }
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b52424eaa0ed..0ac26c892fe2 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -945,6 +945,16 @@ static inline void irq_gc_lock(struct irq_chip_generic *gc) { }
 static inline void irq_gc_unlock(struct irq_chip_generic *gc) { }
 #endif
 
+/*
+ * The irqsave variants are for usage in non interrupt code. Do not use
+ * them in irq_chip callbacks. Use irq_gc_lock() instead.
+ */
+#define irq_gc_lock_irqsave(gc, flags)	\
+	raw_spin_lock_irqsave(&(gc)->lock, flags)
+
+#define irq_gc_unlock_irqrestore(gc, flags)	\
+	raw_spin_unlock_irqrestore(&(gc)->lock, flags)
+
 static inline void irq_reg_writel(struct irq_chip_generic *gc,
 				  u32 val, int reg_offset)
 {

[toc] | [next] | [standalone]

#1485895

On Sun, Sep 18, 2016 at 1:05 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Boris Brezillon (2):
>       genirq: Provide irq_gc_{lock_irqsave,unlock_irqrestore}() helpers

This seems somewhat questionable.

The non-irqsafe versions of these functions have specialized non-SMP
versions for them. The new ones don't.

I pulled it, because I think it's actually the old versions that are
garbage (the locking should become a no-op on UP regardless - except
for the preemption issue which I don't see why it wouldn't be real),
but I think this is inconsistent and should be fixed (or a comment
added about why it is ok).

              Linus

[toc] | [prev] | [next] | [standalone]

#1486190

On Sun, 18 Sep 2016, Linus Torvalds wrote:
> On Sun, Sep 18, 2016 at 1:05 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Boris Brezillon (2):
> >       genirq: Provide irq_gc_{lock_irqsave,unlock_irqrestore}() helpers
> 
> This seems somewhat questionable.
> 
> The non-irqsafe versions of these functions have specialized non-SMP
> versions for them. The new ones don't.
> 
> I pulled it, because I think it's actually the old versions that are
> garbage (the locking should become a no-op on UP regardless - except
> for the preemption issue which I don't see why it wouldn't be real),
> but I think this is inconsistent and should be fixed (or a comment
> added about why it is ok).

Yes, it lacks a comment. The reason why the non save versions are empty is
that these primitives are solely used in the irq chip hotpath functions. UP
folks wanted to avoid the preempt disable/enable in these anyway interrupt
disabled contexts.

Thanks,

	tglx

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web