Linux-RISC-V Archive on lore.kernel.org
 help / Atom feed
From: Atish Patra <atish.patra@wdc.com>
To: Anup Patel <anup@brainfault.org>,
	Palmer Dabbelt <palmer@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host
Date: Thu, 29 Nov 2018 21:59:55 -0800
Message-ID: <71aaed41-c794-ea82-8d87-ddcde3506067@wdc.com> (raw)
In-Reply-To: <20181127100317.12809-5-anup@brainfault.org>

On 11/27/18 2:04 AM, Anup Patel wrote:
> Currently on SMP host, all CPUs take external interrupts routed via
> PLIC. All CPUs will try to claim a given external interrupt but only
> one of them will succeed while other CPUs would simply resume whatever
> they were doing before. This means if we have N CPUs then for every
> external interrupt N-1 CPUs will always fail to claim it and waste
> their CPU time.
> 
> Instead of above, external interrupts should be taken by only one CPU
> and we should have provision to explicity specify IRQ affinity from
s/explicity/explicitly

> kernel-space or user-space.
> 
> This patch provides irq_set_affinity() implementation for PLIC driver.
> It also updates irq_enable() such that PLIC interrupts are only enabled
> for one of CPUs specified in IRQ affinity mask.
> 
> With this patch in-place, we can change IRQ affinity at any-time from
> user-space using procfs.
> 
> Example:
> 
> / # cat /proc/interrupts
>             CPU0       CPU1       CPU2       CPU3
>    8:         44          0          0          0  SiFive PLIC   8  virtio0
>   10:         48          0          0          0  SiFive PLIC  10  ttyS0
> IPI0:        55        663         58        363  Rescheduling interrupts
> IPI1:         0          1          3         16  Function call interrupts
> / #
> / #
> / # echo 4 > /proc/irq/10/smp_affinity
> / #
> / # cat /proc/interrupts
>             CPU0       CPU1       CPU2       CPU3
>    8:         45          0          0          0  SiFive PLIC   8  virtio0
>   10:        160          0         17          0  SiFive PLIC  10  ttyS0
> IPI0:        68        693         77        410  Rescheduling interrupts
> IPI1:         0          2          3         16  Function call interrupts
> 
> Signed-off-by: Anup Patel <anup@brainfault.org>
> ---
>   drivers/irqchip/irq-sifive-plic.c | 35 +++++++++++++++++++++++++++++--
>   1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index ffd4deaca057..fec7da3797fa 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -98,14 +98,42 @@ static void plic_irq_toggle(const struct cpumask *mask, int hwirq, int enable)
>   
>   static void plic_irq_enable(struct irq_data *d)
>   {
> -	plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 1);
> +	unsigned int cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
> +					   cpu_online_mask);
> +	WARN_ON(cpu >= nr_cpu_ids);
> +	plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
>   }
>   
>   static void plic_irq_disable(struct irq_data *d)
>   {
> -	plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 0);
> +	plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
>   }
>   
> +#ifdef CONFIG_SMP
> +static int plic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
> +			    bool force)
> +{
> +	unsigned int cpu;
> +
> +	if (!force)
> +		cpu = cpumask_any_and(mask_val, cpu_online_mask);
> +	else
> +		cpu = cpumask_first(mask_val);
> +
> +	if (cpu >= nr_cpu_ids)
> +		return -EINVAL;
> +
> +	if (!irqd_irq_disabled(d)) {
> +		plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
> +		plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);

irq is disabled for a fraction of time for cpu as well.
You can use cpumask_andnot to avoid that.


Moreover, something is weird here. I tested the patch in Unleashed with 
a debug statement.

Here are the cpumask plic_set_affinity receives.

# echo 0 > /proc[  280.810000] plic: plic_set_affinity: set affinity [0-1]
[  280.810000] plic: plic_set_affinity: cpu = [0] irq = 4
# echo 1 > /proc[  286.290000] plic: plic_set_affinity: set affinity [0]
[  286.290000] plic: plic_set_affinity: cpu = [0] irq = 4
# echo 2 > /proc[  292.130000] plic: plic_set_affinity: set affinity [1]
[  292.130000] plic: plic_set_affinity: cpu = [1] irq = 4
# echo 3 > /proc[  297.750000] plic: plic_set_affinity: set affinity [0-1]
[  297.750000] plic: plic_set_affinity: cpu = [0] irq = 4

# echo 2 > /proc/irq/4/smp_affinity
[  322.850000] plic: plic_set_affinity: set affinity [1]
[  322.850000] plic: plic_set_affinity: cpu = [1] irq = 4

I have not figured out why it receive cpu mask for 0 & 3.
Not sure if logical cpu id to hart id mapping is responsible for other 
two case. I will continue to test tomorrow.

Regards,
Atish
> +	}
> +

> +	irq_data_update_effective_affinity(d, cpumask_of(cpu));
> +
> +	return IRQ_SET_MASK_OK_DONE;
> +}
> +#endif
> +
>   static struct irq_chip plic_chip = {
>   	.name		= "SiFive PLIC",
>   	/*
> @@ -114,6 +142,9 @@ static struct irq_chip plic_chip = {
>   	 */
>   	.irq_enable	= plic_irq_enable,
>   	.irq_disable	= plic_irq_disable,
> +#ifdef CONFIG_SMP
> +	.irq_set_affinity = plic_set_affinity,
> +#endif
>   };
>   
>   static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  parent reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-27 10:03 [PATCH v2 0/4] IRQ affinity support in PLIC driver anup
2018-11-27 10:03 ` Anup Patel
2018-11-27 10:03 ` [PATCH v2 1/4] irqchip: sifive-plic: Pre-compute context hart base and enable base anup
2018-11-27 10:03   ` Anup Patel
2018-11-30  0:35   ` Atish Patra
2018-11-30  3:34     ` Anup Patel
2018-11-27 10:03 ` [PATCH v2 2/4] irqchip: sifive-plic: More flexible plic_irq_toggle() anup
2018-11-27 10:03   ` Anup Patel
2018-11-30  1:39   ` Atish Patra
2018-11-30  3:51     ` Anup Patel
2018-11-27 10:03 ` [PATCH v2 3/4] irqchip: sifive-plic: Differentiate between PLIC handler and context anup
2018-11-27 10:03   ` Anup Patel
2018-11-30  1:57   ` Atish Patra
2018-11-30  3:55     ` Anup Patel
2018-11-27 10:03 ` [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host anup
2018-11-27 10:03   ` Anup Patel
2018-11-30  5:59   ` Atish Patra [this message]
2018-11-30  7:51     ` Anup Patel
2018-11-30  7:54     ` Atish Patra

Reply instructions:

You may reply publically 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=71aaed41-c794-ea82-8d87-ddcde3506067@wdc.com \
    --to=atish.patra@wdc.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=daniel.lezcano@linaro.org \
    --cc=hch@infradead.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=palmer@sifive.com \
    --cc=tglx@linutronix.de \
    /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

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org infradead-linux-riscv@archiver.kernel.org
	public-inbox-index linux-riscv


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/ public-inbox