All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Valentin Schneider <valentin.schneider@arm.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>
Subject: Re: [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
Date: Thu, 27 May 2021 12:17:31 +0100	[thread overview]
Message-ID: <87zgwgs9x0.wl-maz@kernel.org> (raw)
In-Reply-To: <20210525173255.620606-1-valentin.schneider@arm.com>

On Tue, 25 May 2021 18:32:45 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> Hi folks!
> 
> This is the spiritual successor to [1], which was over 6 years ago (!).
> 
> Revisions
> =========
> 
> RFCv1 -> RFCv2
> ++++++++++++++
> 
> o Rebased against latest tip/irq/core
> o Applied cleanups suggested by Thomas
> 
> o Collected some performance results
> 
> Background
> ==========
> 
> GIC mechanics
> +++++++++++++
> 
> There are three IRQ operations:
> o Acknowledge. This gives us the IRQ number that interrupted us, and also
>   - raises the running priority of the CPU interface to that of the IRQ
>   - sets the active bit of the IRQ
> o Priority Drop. This "clears" the running priority.
> o Deactivate. This clears the active bit of the IRQ.
> 
> o The CPU interface has a running priority value. No interrupt of lower or
>   equal priority will be signaled to the CPU attached to that interface. On
>   Linux, we only have two priority values: pNMIs at highest priority, and
>   everything else at the other priority.
> o Most GIC interrupts have an "active" bit. This bit is set on Acknowledge
>   and cleared on Deactivate. A given interrupt cannot be re-signaled to a
>   CPU if it has its active bit set (i.e. if it "fires" again while it's
>   being handled).
> 
> EOImode fun
> +++++++++++
> 
> In EOImode=0, Priority Drop and Deactivate are undissociable. The
> (simplified) interrupt handling flow is as follows: 
> 
>   <~IRQ>
>     Acknowledge
>     Priority Drop + Deactivate
>     <interrupts can once again be signaled, once interrupts are re-enabled>
> 
> With EOImode=1, we can invoke each operation individually. This gives us:
> 
>   <~IRQ>
>     Acknowledge
>     Priority Drop
>     <*other* interrupts can be signaled from here, once interrupts are re-enabled>
>     Deactivate
>     <*this* interrupt can be signaled again>
> 
> What this means is that with EOImode=1, any interrupt is kept "masked" by
> its active bit between Priority Drop and Deactivate.
> 
> Threaded IRQs and ONESHOT
> =========================
> 
> ONESHOT threaded IRQs must remain masked between the main handler and the
> threaded handler. Right now we do this using the conventional irq_mask()
> operations, which looks like this: 
> 
>  <irq handler>
>    Acknowledge
>    Priority Drop   
>    irq_mask()
>    Deactivate
> 
>  <threaded handler>
>    irq_unmask()
> 
> However, masking for the GICs means poking the distributor, and there's no
> sysreg for that - it's an MMIO access. We've seen above that our IRQ
> handling can give us masking "for free", and this is what this patch set is
> all about. It turns the above handling into:
> 
>   <irq handler>
>     Acknowledge
>     Priority Drop
> 
>   <threaded handler>
>     Deactivate
> 
> No irq_mask() => fewer MMIO accesses => happier users (or so I've been
> told). This is especially relevant to PREEMPT_RT which forces threaded
> IRQs.
>     
> Functional testing
> ==================
> 
> GICv2
> +++++
> 
> I've tested this on my Juno with forced irqthreads. This makes the pl011
> IRQ into a threaded ONESHOT IRQ, so I spammed my keyboard into the console
> and verified via ftrace that there were no irq_mask() / irq_unmask()
> involved.
> 
> GICv3
> +++++
> 
> I've tested this on my Ampere eMAG, which uncovered "fun" interactions with
> the MSI domains. Did the same trick as the Juno with the pl011.
> 
> pNMIs cause said eMAG to freeze, but that's true even without my patches. I
> did try them out under QEMU+KVM and that looked fine, although that means I
> only got to test EOImode=0. I'll try to dig into this when I get some more
> cycles.

That's interesting/worrying. As far as I remember, this machine uses
GIC500, which is a well known quantity. If pNMIs are causing issues,
that'd probably be a CPU interface problem. Can you elaborate on how
you tried to test that part? Just using the below benchmark?

> 
> Performance impact
> ==================
> 
> Benchmark
> +++++++++
> 
> Finding a benchmark that leverages a force-threaded IRQ has proved to be
> somewhat of a pain, so I crafted my own. It's a bit daft, but so are most
> benchmarks (though this one might win a prize).

I love it (and wrote similar hacks in my time)! :D Can you put that up
somewhere so that I can run the same test on my own zoo and find out
how it fares?

> 
> Long story short, I'm picking an unused IRQ and have it be
> force-threaded. The benchmark then is:
> 
>   <bench thread>
>     loop:
>       irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, true);
>       wait_for_completion(&done);
> 
>   <threaded handler>
>     complete(&done);
> 
> A more complete picture would be:
> 
>   <bench thread>   <whatever is on CPU0>   <IRQ thread>
>     raise IRQ
>     wait
> 		    run flow handler
> 		      wake IRQ thread
> 					    finish handling
> 					    wake bench thread
>     
> Letting this run for a fixed amount of time lets me measure an entire IRQ
> handling cycle, which is what I'm after since there's one less mask() in
> the flow handler and one less unmask() in the threaded handler.
> 
> You'll note there's some potential "noise" in there due to scheduling both
> the benchmark thread and the IRQ thread. However, the IRQ thread is pinned
> to the IRQ's affinity, and I also pinned the benchmark thread in my tests,
> which should keep this noise to a minimum.
> 
> Results
> +++++++
> 
> On a Juno r0, 20 iterations of 5 seconds of that benchmark yields
> (measuring irqs/sec): 
> 
>   | mean | median | 90th percentile | 99th percentile |
>   |------+--------+-----------------+-----------------|
>   | +11% |   +11% |            +12% |            +14% |
> 
> On an Ampere eMAG, 20 iterations of 5 seconds of that benchmark yields
> (measuring irqs/sec):
> 
>   | mean | median | 90th percentile | 99th percentile |
>   |------+--------+-----------------+-----------------|
>   | +20% |   +20% |            +20% |            +20% |
> 
> This is still quite "artificial", but it reassures me in that skipping those
> (un)mask operations can yield some measurable improvement.

20% improvement is even higher than I suspected!

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Valentin Schneider <valentin.schneider@arm.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>
Subject: Re: [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
Date: Thu, 27 May 2021 12:17:31 +0100	[thread overview]
Message-ID: <87zgwgs9x0.wl-maz@kernel.org> (raw)
In-Reply-To: <20210525173255.620606-1-valentin.schneider@arm.com>

On Tue, 25 May 2021 18:32:45 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> Hi folks!
> 
> This is the spiritual successor to [1], which was over 6 years ago (!).
> 
> Revisions
> =========
> 
> RFCv1 -> RFCv2
> ++++++++++++++
> 
> o Rebased against latest tip/irq/core
> o Applied cleanups suggested by Thomas
> 
> o Collected some performance results
> 
> Background
> ==========
> 
> GIC mechanics
> +++++++++++++
> 
> There are three IRQ operations:
> o Acknowledge. This gives us the IRQ number that interrupted us, and also
>   - raises the running priority of the CPU interface to that of the IRQ
>   - sets the active bit of the IRQ
> o Priority Drop. This "clears" the running priority.
> o Deactivate. This clears the active bit of the IRQ.
> 
> o The CPU interface has a running priority value. No interrupt of lower or
>   equal priority will be signaled to the CPU attached to that interface. On
>   Linux, we only have two priority values: pNMIs at highest priority, and
>   everything else at the other priority.
> o Most GIC interrupts have an "active" bit. This bit is set on Acknowledge
>   and cleared on Deactivate. A given interrupt cannot be re-signaled to a
>   CPU if it has its active bit set (i.e. if it "fires" again while it's
>   being handled).
> 
> EOImode fun
> +++++++++++
> 
> In EOImode=0, Priority Drop and Deactivate are undissociable. The
> (simplified) interrupt handling flow is as follows: 
> 
>   <~IRQ>
>     Acknowledge
>     Priority Drop + Deactivate
>     <interrupts can once again be signaled, once interrupts are re-enabled>
> 
> With EOImode=1, we can invoke each operation individually. This gives us:
> 
>   <~IRQ>
>     Acknowledge
>     Priority Drop
>     <*other* interrupts can be signaled from here, once interrupts are re-enabled>
>     Deactivate
>     <*this* interrupt can be signaled again>
> 
> What this means is that with EOImode=1, any interrupt is kept "masked" by
> its active bit between Priority Drop and Deactivate.
> 
> Threaded IRQs and ONESHOT
> =========================
> 
> ONESHOT threaded IRQs must remain masked between the main handler and the
> threaded handler. Right now we do this using the conventional irq_mask()
> operations, which looks like this: 
> 
>  <irq handler>
>    Acknowledge
>    Priority Drop   
>    irq_mask()
>    Deactivate
> 
>  <threaded handler>
>    irq_unmask()
> 
> However, masking for the GICs means poking the distributor, and there's no
> sysreg for that - it's an MMIO access. We've seen above that our IRQ
> handling can give us masking "for free", and this is what this patch set is
> all about. It turns the above handling into:
> 
>   <irq handler>
>     Acknowledge
>     Priority Drop
> 
>   <threaded handler>
>     Deactivate
> 
> No irq_mask() => fewer MMIO accesses => happier users (or so I've been
> told). This is especially relevant to PREEMPT_RT which forces threaded
> IRQs.
>     
> Functional testing
> ==================
> 
> GICv2
> +++++
> 
> I've tested this on my Juno with forced irqthreads. This makes the pl011
> IRQ into a threaded ONESHOT IRQ, so I spammed my keyboard into the console
> and verified via ftrace that there were no irq_mask() / irq_unmask()
> involved.
> 
> GICv3
> +++++
> 
> I've tested this on my Ampere eMAG, which uncovered "fun" interactions with
> the MSI domains. Did the same trick as the Juno with the pl011.
> 
> pNMIs cause said eMAG to freeze, but that's true even without my patches. I
> did try them out under QEMU+KVM and that looked fine, although that means I
> only got to test EOImode=0. I'll try to dig into this when I get some more
> cycles.

That's interesting/worrying. As far as I remember, this machine uses
GIC500, which is a well known quantity. If pNMIs are causing issues,
that'd probably be a CPU interface problem. Can you elaborate on how
you tried to test that part? Just using the below benchmark?

> 
> Performance impact
> ==================
> 
> Benchmark
> +++++++++
> 
> Finding a benchmark that leverages a force-threaded IRQ has proved to be
> somewhat of a pain, so I crafted my own. It's a bit daft, but so are most
> benchmarks (though this one might win a prize).

I love it (and wrote similar hacks in my time)! :D Can you put that up
somewhere so that I can run the same test on my own zoo and find out
how it fares?

> 
> Long story short, I'm picking an unused IRQ and have it be
> force-threaded. The benchmark then is:
> 
>   <bench thread>
>     loop:
>       irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, true);
>       wait_for_completion(&done);
> 
>   <threaded handler>
>     complete(&done);
> 
> A more complete picture would be:
> 
>   <bench thread>   <whatever is on CPU0>   <IRQ thread>
>     raise IRQ
>     wait
> 		    run flow handler
> 		      wake IRQ thread
> 					    finish handling
> 					    wake bench thread
>     
> Letting this run for a fixed amount of time lets me measure an entire IRQ
> handling cycle, which is what I'm after since there's one less mask() in
> the flow handler and one less unmask() in the threaded handler.
> 
> You'll note there's some potential "noise" in there due to scheduling both
> the benchmark thread and the IRQ thread. However, the IRQ thread is pinned
> to the IRQ's affinity, and I also pinned the benchmark thread in my tests,
> which should keep this noise to a minimum.
> 
> Results
> +++++++
> 
> On a Juno r0, 20 iterations of 5 seconds of that benchmark yields
> (measuring irqs/sec): 
> 
>   | mean | median | 90th percentile | 99th percentile |
>   |------+--------+-----------------+-----------------|
>   | +11% |   +11% |            +12% |            +14% |
> 
> On an Ampere eMAG, 20 iterations of 5 seconds of that benchmark yields
> (measuring irqs/sec):
> 
>   | mean | median | 90th percentile | 99th percentile |
>   |------+--------+-----------------+-----------------|
>   | +20% |   +20% |            +20% |            +20% |
> 
> This is still quite "artificial", but it reassures me in that skipping those
> (un)mask operations can yield some measurable improvement.

20% improvement is even higher than I suspected!

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

  parent reply	other threads:[~2021-05-27 11:17 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
2021-05-25 17:32 ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 20:54   ` kernel test robot
2021-05-27 10:55   ` Marc Zyngier
2021-05-27 10:55     ` Marc Zyngier
2021-05-27 10:58     ` Marc Zyngier
2021-05-27 10:58       ` Marc Zyngier
2021-06-01 10:25       ` Valentin Schneider
2021-06-01 10:25         ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 04/10] genirq: Add handle_strict_flow_irq() flow handler Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq() Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent() Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-27 12:17   ` Marc Zyngier
2021-05-27 12:17     ` Marc Zyngier
2021-06-01 10:25     ` Valentin Schneider
2021-06-01 10:25       ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq() Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-27 12:21   ` Marc Zyngier
2021-05-27 12:21     ` Marc Zyngier
2021-06-01 10:25     ` Valentin Schneider
2021-06-01 10:25       ` Valentin Schneider
2021-06-15 15:20       ` Valentin Schneider
2021-06-15 15:20         ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 10/10] irqchip/gic-v3: " Valentin Schneider
2021-05-25 17:32   ` Valentin Schneider
2021-05-25 17:34 ` [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
2021-05-25 17:34   ` Valentin Schneider
2021-05-27 11:17 ` Marc Zyngier [this message]
2021-05-27 11:17   ` Marc Zyngier
2021-06-01 10:25   ` Valentin Schneider
2021-06-01 10:25     ` Valentin Schneider
2021-06-03 15:32     ` Valentin Schneider
2021-06-03 15:32       ` Valentin Schneider
2021-06-08 15:29     ` Marc Zyngier
2021-06-08 15:29       ` Marc Zyngier
2021-06-08 17:58       ` Lorenzo Pieralisi
2021-06-08 17:58         ` Lorenzo Pieralisi

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=87zgwgs9x0.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=tglx@linutronix.de \
    --cc=valentin.schneider@arm.com \
    --cc=vincenzo.frascino@arm.com \
    /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.