All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anup Patel <anup@brainfault.org>
To: Christoph Hellwig <hch@infradead.org>
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>,
	Atish Patra <atish.patra@wdc.com>,
	linux-riscv@lists.infradead.org,
	"linux-kernel@vger.kernel.org List"
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/4] irqchip: sifive-plic: Pre-compute context hart base and enable base
Date: Mon, 12 Nov 2018 09:57:02 +0530	[thread overview]
Message-ID: <CAAhSdy1dYGShvtUEdDARw_8viC5P5MdMLBQOhe56nu6TMNgGaA@mail.gmail.com> (raw)
In-Reply-To: <20181109084256.GA6508@infradead.org>

On Fri, Nov 9, 2018 at 2:12 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Oct 22, 2018 at 05:15:14PM +0530, Anup Patel wrote:
> > This patch does following optimizations:
> > 1. Pre-compute hart base for each context handler
> > 2. Pre-compute enable base for each context handler
>
> Why?

This is micro-optimizations. We don't need to re-compute
hart base and hart enable base everytime.

>
> > 3. Have enable lock for each context handler instead
> > of global plic_toggle_lock
>
> Why?  Also even if you want this it should be a separate patch.

Well, the PLIC register space it a bit strange.

Most PLIC context specific registers are in one place
except context IRQ enable registers which are part of
global registers.

To handle this, we had a global plic_toggle_lock which
was taken whenever PLIC driver touched context IRQ
enable registers. Instead of this, we can have per-context
IRQ enable lock for more granular locking.

Later when we implement IRQ set_affinity, we touch
IRQ enable registers of each context whenever IRQ
affinity changes. This fine grained IRQ enable locking
helps when IRQ load-balancer is changing affinity of
different IRQs parallely on separate cores. Again this
is a micro-optimization.

>
> >  #define PRIORITY_BASE                        0
> > -#define     PRIORITY_PER_ID          4
> > +#define PRIORITY_PER_ID                      4
>
> Also please drop the random whitespace changes.

Instead of dropping I will make it separate patch because
we are replacing "\t" between #define and define_name
with a space.

Regards,
Anup

WARNING: multiple messages have this Message-ID (diff)
From: anup@brainfault.org (Anup Patel)
To: linux-riscv@lists.infradead.org
Subject: [PATCH 1/4] irqchip: sifive-plic: Pre-compute context hart base and enable base
Date: Mon, 12 Nov 2018 09:57:02 +0530	[thread overview]
Message-ID: <CAAhSdy1dYGShvtUEdDARw_8viC5P5MdMLBQOhe56nu6TMNgGaA@mail.gmail.com> (raw)
In-Reply-To: <20181109084256.GA6508@infradead.org>

On Fri, Nov 9, 2018 at 2:12 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Oct 22, 2018 at 05:15:14PM +0530, Anup Patel wrote:
> > This patch does following optimizations:
> > 1. Pre-compute hart base for each context handler
> > 2. Pre-compute enable base for each context handler
>
> Why?

This is micro-optimizations. We don't need to re-compute
hart base and hart enable base everytime.

>
> > 3. Have enable lock for each context handler instead
> > of global plic_toggle_lock
>
> Why?  Also even if you want this it should be a separate patch.

Well, the PLIC register space it a bit strange.

Most PLIC context specific registers are in one place
except context IRQ enable registers which are part of
global registers.

To handle this, we had a global plic_toggle_lock which
was taken whenever PLIC driver touched context IRQ
enable registers. Instead of this, we can have per-context
IRQ enable lock for more granular locking.

Later when we implement IRQ set_affinity, we touch
IRQ enable registers of each context whenever IRQ
affinity changes. This fine grained IRQ enable locking
helps when IRQ load-balancer is changing affinity of
different IRQs parallely on separate cores. Again this
is a micro-optimization.

>
> >  #define PRIORITY_BASE                        0
> > -#define     PRIORITY_PER_ID          4
> > +#define PRIORITY_PER_ID                      4
>
> Also please drop the random whitespace changes.

Instead of dropping I will make it separate patch because
we are replacing "\t" between #define and define_name
with a space.

Regards,
Anup

WARNING: multiple messages have this Message-ID (diff)
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 1/4] irqchip: sifive-plic: Pre-compute context hart base and enable base
Date: Mon, 12 Nov 2018 09:57:02 +0530	[thread overview]
Message-ID: <CAAhSdy1dYGShvtUEdDARw_8viC5P5MdMLBQOhe56nu6TMNgGaA@mail.gmail.com> (raw)
Message-ID: <20181112042702.Y6rZOcIKzl62AQPh9stJDb15C9lsHQhTxHAJ6SZ2Yjk@z> (raw)
In-Reply-To: <20181109084256.GA6508@infradead.org>

On Fri, Nov 9, 2018 at 2:12 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Oct 22, 2018 at 05:15:14PM +0530, Anup Patel wrote:
> > This patch does following optimizations:
> > 1. Pre-compute hart base for each context handler
> > 2. Pre-compute enable base for each context handler
>
> Why?

This is micro-optimizations. We don't need to re-compute
hart base and hart enable base everytime.

>
> > 3. Have enable lock for each context handler instead
> > of global plic_toggle_lock
>
> Why?  Also even if you want this it should be a separate patch.

Well, the PLIC register space it a bit strange.

Most PLIC context specific registers are in one place
except context IRQ enable registers which are part of
global registers.

To handle this, we had a global plic_toggle_lock which
was taken whenever PLIC driver touched context IRQ
enable registers. Instead of this, we can have per-context
IRQ enable lock for more granular locking.

Later when we implement IRQ set_affinity, we touch
IRQ enable registers of each context whenever IRQ
affinity changes. This fine grained IRQ enable locking
helps when IRQ load-balancer is changing affinity of
different IRQs parallely on separate cores. Again this
is a micro-optimization.

>
> >  #define PRIORITY_BASE                        0
> > -#define     PRIORITY_PER_ID          4
> > +#define PRIORITY_PER_ID                      4
>
> Also please drop the random whitespace changes.

Instead of dropping I will make it separate patch because
we are replacing "\t" between #define and define_name
with a space.

Regards,
Anup

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

  reply	other threads:[~2018-11-12  4:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-22 11:45 [PATCH 0/4] IRQ affinity support in PLIC driver Anup Patel
2018-10-22 11:45 ` Anup Patel
2018-10-22 11:45 ` Anup Patel
2018-10-22 11:45 ` [PATCH 1/4] irqchip: sifive-plic: Pre-compute context hart base and enable base Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-11-09  8:42   ` Christoph Hellwig
2018-11-09  8:42     ` Christoph Hellwig
2018-11-09  8:42     ` Christoph Hellwig
2018-11-12  4:27     ` Anup Patel [this message]
2018-11-12  4:27       ` Anup Patel
2018-11-12  4:27       ` Anup Patel
2018-10-22 11:45 ` [PATCH 2/4] irqchip: sifive-plic: More flexible plic_irq_toggle() Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-11-09  8:43   ` Christoph Hellwig
2018-11-09  8:43     ` Christoph Hellwig
2018-11-09  8:43     ` Christoph Hellwig
2018-11-12 12:33     ` Anup Patel
2018-11-12 12:33       ` Anup Patel
2018-11-12 12:33       ` Anup Patel
2018-10-22 11:45 ` [PATCH 3/4] irqchip: sifive-plic: Differentiate between PLIC handler and context Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-10-22 11:45 ` [PATCH 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host Anup Patel
2018-10-22 11:45   ` Anup Patel
2018-10-22 11:45   ` Anup Patel

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=CAAhSdy1dYGShvtUEdDARw_8viC5P5MdMLBQOhe56nu6TMNgGaA@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 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.