From: Anup Patel <anup@brainfault.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: 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>,
Christoph Hellwig <hch@infradead.org>,
linux-riscv@lists.infradead.org,
"linux-kernel@vger.kernel.org List"
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host
Date: Fri, 30 Nov 2018 13:21:40 +0530 [thread overview]
Message-ID: <CAAhSdy2KkBT2gcxiJsV1XUOQvbp0CXA4OM1VCJzMeCBQ3FWTfQ@mail.gmail.com> (raw)
In-Reply-To: <71aaed41-c794-ea82-8d87-ddcde3506067@wdc.com>
On Fri, Nov 30, 2018 at 11:29 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> 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
Sure, I will update it.
>
> > 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.
The smp_affinity in procfs takes hex values as input.
1 = CPU0
2 = CPU1
3 = CPU0-1
4 = CPU2
... and so on ...
>
> # echo 0 > /proc[ 280.810000] plic: plic_set_affinity: set affinity [0-1]
> [ 280.810000] plic: plic_set_affinity: cpu = [0] irq = 4
OK, this is strange.
> # echo 1 > /proc[ 286.290000] plic: plic_set_affinity: set affinity [0]
> [ 286.290000] plic: plic_set_affinity: cpu = [0] irq = 4
This is correct.
> # echo 2 > /proc[ 292.130000] plic: plic_set_affinity: set affinity [1]
> [ 292.130000] plic: plic_set_affinity: cpu = [1] irq = 4
This is correct.
> # echo 3 > /proc[ 297.750000] plic: plic_set_affinity: set affinity [0-1]
> [ 297.750000] plic: plic_set_affinity: cpu = [0] irq = 4
This is correct.
>
> # 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
This is correct.
>
> 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.
Except value '0', all cases are correct.
Regards,
Anup
next prev parent reply other threads:[~2018-11-30 7:51 UTC|newest]
Thread overview: 14+ 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 Patel
2018-11-27 10:03 ` [PATCH v2 1/4] irqchip: sifive-plic: Pre-compute context hart base and enable base 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 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 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 Patel
2018-11-30 5:59 ` Atish Patra
2018-11-30 7:51 ` Anup Patel [this message]
2018-11-30 7:54 ` Atish Patra
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=CAAhSdy2KkBT2gcxiJsV1XUOQvbp0CXA4OM1VCJzMeCBQ3FWTfQ@mail.gmail.com \
--to=anup@brainfault.org \
--cc=aou@eecs.berkeley.edu \
--cc=atish.patra@wdc.com \
--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
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).