All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vineet.Gupta1@synopsys.com (Vineet Gupta)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver didn't ack prev
Date: Fri, 26 Feb 2016 16:32:42 +0530	[thread overview]
Message-ID: <56D030D2.6070104@synopsys.com> (raw)
In-Reply-To: <1456218702-11911-5-git-send-email-vgupta@synopsys.com>

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 at synopsys.com>
> Cc: Peter Zijlstra <peterz at infradead.org>
> Signed-off-by: Vineet Gupta <vgupta at 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
> 

WARNING: multiple messages have this Message-ID (diff)
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: <linux-snps-arc@lists.infradead.org>
Cc: <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Chuck Jordan <Chuck.Jordan@synopsys.com>
Subject: Re: [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver didn't ack prev
Date: Fri, 26 Feb 2016 16:32:42 +0530	[thread overview]
Message-ID: <56D030D2.6070104@synopsys.com> (raw)
In-Reply-To: <1456218702-11911-5-git-send-email-vgupta@synopsys.com>

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
> 

  reply	other threads:[~2016-02-26 11:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-23  9:11 [PATCH 0/5] ARC IPI related fix Vineet Gupta
2016-02-23  9:11 ` Vineet Gupta
2016-02-23  9:11 ` [PATCH 1/5] ARCv2: SMP: Emulate IPI to self using software triggered interrupt Vineet Gupta
2016-02-23  9:11   ` Vineet Gupta
2016-02-23  9:11 ` [PATCH 2/5] ARC: [intc-compact] Remove IPI setup from ARCompact port Vineet Gupta
2016-02-23  9:11   ` Vineet Gupta
2016-02-23  9:11 ` [PATCH 3/5] ARCv2: SMP: Push IPI_IRQ into IPI provider Vineet Gupta
2016-02-23  9:11   ` Vineet Gupta
2016-02-23  9:11 ` [PATCH 4/5] ARCv2: Elide sending new cross core intr if receiver didn't ack prev Vineet Gupta
2016-02-23  9:11   ` Vineet Gupta
2016-02-26 11:02   ` Vineet Gupta [this message]
2016-02-26 11:02     ` Vineet Gupta
2016-02-23  9:11 ` [PATCH 5/5] ARC: SMP: No need for CONFIG_ARC_IPI_DBG Vineet Gupta
2016-02-23  9:11   ` Vineet Gupta

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=56D030D2.6070104@synopsys.com \
    --to=vineet.gupta1@synopsys.com \
    --cc=linux-snps-arc@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.