Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
From: Anup Patel <anup@brainfault.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Jason Cooper <jason@lakedaemon.net>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Palmer Dabbelt <palmer@sifive.com>,
	"linux-kernel@vger.kernel.org List"
	<linux-kernel@vger.kernel.org>, Atish Patra <atish.patra@wdc.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v3 6/6] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host
Date: Tue, 18 Dec 2018 16:02:09 +0530
Message-ID: <CAAhSdy3=Hz7uuT=AEJqmZ7OJZg5Zt9MSg8QiBjEzH5wcsOH1-Q@mail.gmail.com> (raw)
In-Reply-To: <20181217183220.GE7086@infradead.org>

On Tue, Dec 18, 2018 at 12:02 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Nov 30, 2018 at 01:32:07PM +0530, Anup Patel wrote:
> > 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.
>
> But normally our affinity masks are that - masks of CPUs that can take
> it.  It seems a bit odd to then just pick the first one, as this means
> with default all-CPU masks we'll have all interrupts handled by the
> first CPU only.

Yes, affinity mask are CPUs which can take but there is also effective
affinity mask which represent CPUs which will actually receive IRQ.

Interrupt controllers (unlike PLIC) can support hardware IRQ balancing.
For such interrupt controllers, we inform all CPUs that can take IRQ but
interrupt controller will only deliver IRQ to only one of the CPUs.

There are quite a few interrupt controllers which only allow IRQ to
be taken by exactly one CPU. For such interrupt controllers, the
interrupt controller driver has to to pick one CPU out of CPUs which
can take IRQ (Example GICv2, GICv3, etc).

>
>
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -106,14 +106,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);
>
> I think this should be WARN_ON_ONCE and actually return instead of then
> proceeding using the invalid cpu index.

Sure, will update.

>
> > +#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);
>
> maybe swap the two branches around to avoid the inversion of the force
> flag?

Sure, will update.

Regards,
Anup

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

  reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-30  8:02 [PATCH v3 0/6] IRQ affinity support in PLIC driver Anup Patel
2018-11-30  8:02 ` [PATCH v3 1/6] irqchip: sifive-plic: Pre-compute context hart base and enable base Anup Patel
2018-12-17 18:25   ` Christoph Hellwig
2018-12-18  8:30     ` Anup Patel
2018-11-30  8:02 ` [PATCH v3 2/6] irqchip: sifive-plic: Add struct plic_hw for global PLIC HW details Anup Patel
2018-12-17 18:24   ` Christoph Hellwig
2018-12-18  8:25     ` Anup Patel
2018-11-30  8:02 ` [PATCH v3 3/6] irqchip: sifive-plic: More flexible plic_irq_toggle() Anup Patel
2018-12-17 18:27   ` Christoph Hellwig
2018-12-18  8:50     ` Anup Patel
2018-12-19 16:28       ` Christoph Hellwig
2018-12-27  5:27         ` Anup Patel
2018-11-30  8:02 ` [PATCH v3 4/6] irqchip: sifive-plic: Add warning in plic_init() if handler already present Anup Patel
2018-12-17 18:28   ` Christoph Hellwig
2018-12-18  8:36     ` Anup Patel
2018-11-30  8:02 ` [PATCH v3 5/6] irqchip: sifive-plic: Differentiate between PLIC handler and context Anup Patel
2018-11-30  8:02 ` [PATCH v3 6/6] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host Anup Patel
2018-12-17 18:32   ` Christoph Hellwig
2018-12-18 10:32     ` Anup Patel [this message]
2018-12-17  9:37 ` [PATCH v3 0/6] IRQ affinity support in PLIC driver Anup Patel
2018-12-20 20:40   ` Palmer Dabbelt

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='CAAhSdy3=Hz7uuT=AEJqmZ7OJZg5Zt9MSg8QiBjEzH5wcsOH1-Q@mail.gmail.com' \
    --to=anup@brainfault.org \
    --cc="linux-kernel@vger.kernel.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-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