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

Groups > linux.kernel > #1340450 > unrolled thread

[PATCH 0/5] ARC IPI related fix

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] ARC IPI related fix Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2016-02-23 10:20 +0100
    [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver didn't ack prev Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2016-02-23 10:20 +0100
      Re: [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver  didn't ack prev Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2016-02-26 12:10 +0100
    [PATCH 1/5] ARCv2: SMP: Emulate IPI to self using software triggered interrupt Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2016-02-23 10:20 +0100

#1340450 — [PATCH 0/5] ARC IPI related fix

Hi,

This cam eout of debugging a csd deadlock on ARC.
The fix itself is 1/5, rest of patches are additional improvements,
optimizations while we are at it.

Thx,
-Vineet

Vineet Gupta (5):
  ARCv2: SMP: Emulate IPI to self using software triggered interrupt
  ARC: [intc-compact] Remove IPI setup from ARCompact port
  ARCv2: SMP: Push IPI_IRQ into IPI provider
  ARCv2: Elide sending new cross core intr if receiver didn't ack prev
  ARC: SMP: No need for CONFIG_ARC_IPI_DBG

 arch/arc/Kconfig                      |  8 ------
 arch/arc/include/asm/irq.h            |  2 --
 arch/arc/include/asm/irqflags-arcv2.h | 11 ++++++++
 arch/arc/kernel/intc-compact.c        |  3 --
 arch/arc/kernel/mcip.c                | 52 ++++++++++++++++++-----------------
 arch/arc/kernel/smp.c                 |  3 --
 6 files changed, 38 insertions(+), 41 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]

#1340451 — [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver didn't ack prev

ARConnect/MCIP IPI sending has a retry-wait loop in case caller had
not seen a previous such interrupt. Turns out that it is not needed at
all. Linux cross core calling allows coalescing multiple IPIs to same
receiver - it is fine as long as there is one.

This logic is built into upper layer already, at a higher level of
abstraction. ipi_send_msg_one() sets the actual msg payload, but it only
calls MCIP IPI sending if msg holder was empty (using
atomic-set-new-and-get-old construct). Thus it is unlikely that the
retry-wait looping was ever getting exercised at all.

Cc: Chuck Jordan <cjordan@synopsys.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/kernel/mcip.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index e30d5d428330..7afc3c703ed1 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -40,26 +40,19 @@ static void mcip_ipi_send(int cpu)
 		return;
 	}
 
+	raw_spin_lock_irqsave(&mcip_lock, flags);
+
 	/*
-	 * NOTE: We must spin here if the other cpu hasn't yet
-	 * serviced a previous message. This can burn lots
-	 * of time, but we MUST follows this protocol or
-	 * ipi messages can be lost!!!
-	 * Also, we must release the lock in this loop because
-	 * the other side may get to this same loop and not
-	 * be able to ack -- thus causing deadlock.
+	 * If receiver already has a pending interrupt, elide sending this one.
+	 * Linux cross core calling works well with concurrent IPIs
+	 * coalesced into one
+	 * see arch/arc/kernel/smp.c: ipi_send_msg_one()
 	 */
+	__mcip_cmd(CMD_INTRPT_READ_STATUS, cpu);
+	ipi_was_pending = read_aux_reg(ARC_REG_MCIP_READBACK);
+	if (!ipi_was_pending)
+		__mcip_cmd(CMD_INTRPT_GENERATE_IRQ, cpu);
 
-	do {
-		raw_spin_lock_irqsave(&mcip_lock, flags);
-		__mcip_cmd(CMD_INTRPT_READ_STATUS, cpu);
-		ipi_was_pending = read_aux_reg(ARC_REG_MCIP_READBACK);
-		if (ipi_was_pending == 0)
-			break; /* break out but keep lock */
-		raw_spin_unlock_irqrestore(&mcip_lock, flags);
-	} while (1);
-
-	__mcip_cmd(CMD_INTRPT_GENERATE_IRQ, cpu);
 	raw_spin_unlock_irqrestore(&mcip_lock, flags);
 
 #ifdef CONFIG_ARC_IPI_DBG
-- 
2.5.0

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

#1344132 — Re: [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver didn't ack prev

On Tuesday 23 February 2016 02:41 PM, Vineet Gupta wrote:
> ARConnect/MCIP IPI sending has a retry-wait loop in case caller had
> not seen a previous such interrupt. Turns out that it is not needed at
> all. Linux cross core calling allows coalescing multiple IPIs to same
> receiver - it is fine as long as there is one.
> 
> This logic is built into upper layer already, at a higher level of
> abstraction. ipi_send_msg_one() sets the actual msg payload, but it only
> calls MCIP IPI sending if msg holder was empty (using
> atomic-set-new-and-get-old construct). Thus it is unlikely that the
> retry-wait looping was ever getting exercised at all.

Turns out that this patch was needed for more serious reasons.
For experiment sake I reverted the IPI eliding optimization and immediately ran
into a deadlock, with LTP:trace_sched !

@@ -241,7 +241,7 @@ static void ipi_send_msg_one(int cpu, enum ipi_msg_type msg)
         * IPI handler, because !@old means it has not yet dequeued the msg(s)
         * so @new msg can be a free-loader
         */
-       if (plat_smp_ops.ipi_send && !old)
+       if (plat_smp_ops.ipi_send)

-Vineet

> 
> Cc: Chuck Jordan <cjordan@synopsys.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
>  arch/arc/kernel/mcip.c | 27 ++++++++++-----------------
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index e30d5d428330..7afc3c703ed1 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -40,26 +40,19 @@ static void mcip_ipi_send(int cpu)
>  		return;
>  	}
>  
> +	raw_spin_lock_irqsave(&mcip_lock, flags);
> +
>  	/*
> -	 * NOTE: We must spin here if the other cpu hasn't yet
> -	 * serviced a previous message. This can burn lots
> -	 * of time, but we MUST follows this protocol or
> -	 * ipi messages can be lost!!!
> -	 * Also, we must release the lock in this loop because
> -	 * the other side may get to this same loop and not
> -	 * be able to ack -- thus causing deadlock.
> +	 * If receiver already has a pending interrupt, elide sending this one.
> +	 * Linux cross core calling works well with concurrent IPIs
> +	 * coalesced into one
> +	 * see arch/arc/kernel/smp.c: ipi_send_msg_one()
>  	 */
> +	__mcip_cmd(CMD_INTRPT_READ_STATUS, cpu);
> +	ipi_was_pending = read_aux_reg(ARC_REG_MCIP_READBACK);
> +	if (!ipi_was_pending)
> +		__mcip_cmd(CMD_INTRPT_GENERATE_IRQ, cpu);
>  
> -	do {
> -		raw_spin_lock_irqsave(&mcip_lock, flags);
> -		__mcip_cmd(CMD_INTRPT_READ_STATUS, cpu);
> -		ipi_was_pending = read_aux_reg(ARC_REG_MCIP_READBACK);
> -		if (ipi_was_pending == 0)
> -			break; /* break out but keep lock */
> -		raw_spin_unlock_irqrestore(&mcip_lock, flags);
> -	} while (1);
> -
> -	__mcip_cmd(CMD_INTRPT_GENERATE_IRQ, cpu);
>  	raw_spin_unlock_irqrestore(&mcip_lock, flags);
>  
>  #ifdef CONFIG_ARC_IPI_DBG
> 

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

#1340452 — [PATCH 1/5] ARCv2: SMP: Emulate IPI to self using software triggered interrupt

ARConnect/MCIP Inter-Core-Interrupt module can't send interrupt to
local core. So use core intc capability to trigger software
interrupt to self, using an unsued IRQ #21.

This showed up as csd deadlock with LTP trace_sched on a dual core
system. This test acts as scheduler fuzzer, triggering all sorts of
schedulting activity. Trouble starts with IPI to self, which doesn't get
delivered (effectively lost due to H/w capability), but the msg intended
to be sent remain enqueued in per-cpu @ipi_data.

All subsequent IPIs to this core from other cores get elided due to the
IPI coalescing optimization in ipi_send_msg_one() where a pending msg
implies an IPI already sent and assumes other core is yet to ack it.
After the elided IPI, other core simply goes into csd_lock_wait()
but never comes out as this core never sees the interrupt.

Fixes STAR 9001008624

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: <stable@vger.kernel.org>        [4.2]
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/irqflags-arcv2.h | 11 +++++++++++
 arch/arc/kernel/mcip.c                | 15 +++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/arc/include/asm/irqflags-arcv2.h b/arch/arc/include/asm/irqflags-arcv2.h
index 1fc18ee06cf2..37c2f751eebf 100644
--- a/arch/arc/include/asm/irqflags-arcv2.h
+++ b/arch/arc/include/asm/irqflags-arcv2.h
@@ -22,6 +22,7 @@
 #define AUX_IRQ_CTRL		0x00E
 #define AUX_IRQ_ACT		0x043	/* Active Intr across all levels */
 #define AUX_IRQ_LVL_PEND	0x200	/* Pending Intr across all levels */
+#define AUX_IRQ_HINT		0x201	/* For generating Soft Interrupts */
 #define AUX_IRQ_PRIORITY	0x206
 #define ICAUSE			0x40a
 #define AUX_IRQ_SELECT		0x40b
@@ -115,6 +116,16 @@ static inline int arch_irqs_disabled(void)
 	return arch_irqs_disabled_flags(arch_local_save_flags());
 }
 
+static inline void arc_softirq_trigger(int irq)
+{
+	write_aux_reg(AUX_IRQ_HINT, irq);
+}
+
+static inline void arc_softirq_clear(int irq)
+{
+	write_aux_reg(AUX_IRQ_HINT, 0);
+}
+
 #else
 
 .macro IRQ_DISABLE  scratch
diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index 002c5fcf8947..9e1bd03b87a6 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -11,9 +11,12 @@
 #include <linux/smp.h>
 #include <linux/irq.h>
 #include <linux/spinlock.h>
+#include <asm/irqflags-arcv2.h>
 #include <asm/mcip.h>
 #include <asm/setup.h>
 
+#define SOFTIRQ_IRQ	21
+
 static char smp_cpuinfo_buf[128];
 static int idu_detected;
 
@@ -22,6 +25,7 @@ static DEFINE_RAW_SPINLOCK(mcip_lock);
 static void mcip_setup_per_cpu(int cpu)
 {
 	smp_ipi_irq_setup(cpu, IPI_IRQ);
+	smp_ipi_irq_setup(cpu, SOFTIRQ_IRQ);
 }
 
 static void mcip_ipi_send(int cpu)
@@ -29,6 +33,12 @@ static void mcip_ipi_send(int cpu)
 	unsigned long flags;
 	int ipi_was_pending;
 
+	/* ARConnect can only send IPI to others */
+	if (unlikely(cpu == raw_smp_processor_id())) {
+		arc_softirq_trigger(SOFTIRQ_IRQ);
+		return;
+	}
+
 	/*
 	 * NOTE: We must spin here if the other cpu hasn't yet
 	 * serviced a previous message. This can burn lots
@@ -63,6 +73,11 @@ static void mcip_ipi_clear(int irq)
 	unsigned long flags;
 	unsigned int __maybe_unused copy;
 
+	if (unlikely(irq == SOFTIRQ_IRQ)) {
+		arc_softirq_clear(irq);
+		return;
+	}
+
 	raw_spin_lock_irqsave(&mcip_lock, flags);
 
 	/* Who sent the IPI */
-- 
2.5.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web