* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
@ 2021-05-25 23:01 kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-05-25 23:01 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4226 bytes --]
CC: kbuild-all(a)lists.01.org
CC: clang-built-linux(a)googlegroups.com
In-Reply-To: <20210525173255.620606-3-valentin.schneider@arm.com>
References: <20210525173255.620606-3-valentin.schneider@arm.com>
TO: Valentin Schneider <valentin.schneider@arm.com>
Hi Valentin,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on tip/irq/core]
[also build test WARNING on linux/master soc/for-next linus/master v5.13-rc3 next-20210525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210526-013542
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 006ae1970a8cde1d3e92da69b324d12880133a13
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: x86_64-randconfig-b001-20210525 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/9cb26d40715c7a84d90accb09d808141446dd270
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210526-013542
git checkout 9cb26d40715c7a84d90accb09d808141446dd270
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/irq/chip.c:411:6: warning: no previous prototype for function 'ack_irq' [-Wmissing-prototypes]
void ack_irq(struct irq_desc *desc)
^
kernel/irq/chip.c:411:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void ack_irq(struct irq_desc *desc)
^
static
>> kernel/irq/chip.c:419:6: warning: no previous prototype for function 'eoi_irq' [-Wmissing-prototypes]
void eoi_irq(struct irq_desc *desc)
^
kernel/irq/chip.c:419:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void eoi_irq(struct irq_desc *desc)
^
static
2 warnings generated.
vim +/ack_irq +411 kernel/irq/chip.c
31d9d9b6d83030 Marc Zyngier 2011-09-23 410
9cb26d40715c7a Valentin Schneider 2021-05-25 @411 void ack_irq(struct irq_desc *desc)
9cb26d40715c7a Valentin Schneider 2021-05-25 412 {
9cb26d40715c7a Valentin Schneider 2021-05-25 413 desc->irq_data.chip->irq_ack(&desc->irq_data);
9cb26d40715c7a Valentin Schneider 2021-05-25 414
9cb26d40715c7a Valentin Schneider 2021-05-25 415 if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
9cb26d40715c7a Valentin Schneider 2021-05-25 416 irq_state_set_flow_masked(desc);
9cb26d40715c7a Valentin Schneider 2021-05-25 417 }
9cb26d40715c7a Valentin Schneider 2021-05-25 418
9cb26d40715c7a Valentin Schneider 2021-05-25 @419 void eoi_irq(struct irq_desc *desc)
9cb26d40715c7a Valentin Schneider 2021-05-25 420 {
9cb26d40715c7a Valentin Schneider 2021-05-25 421 desc->irq_data.chip->irq_eoi(&desc->irq_data);
9cb26d40715c7a Valentin Schneider 2021-05-25 422
9cb26d40715c7a Valentin Schneider 2021-05-25 423 if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
9cb26d40715c7a Valentin Schneider 2021-05-25 424 irq_state_clr_flow_masked(desc);
9cb26d40715c7a Valentin Schneider 2021-05-25 425 }
9cb26d40715c7a Valentin Schneider 2021-05-25 426
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31446 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
@ 2021-05-25 17:32 Valentin Schneider
2021-05-25 17:32 ` Valentin Schneider
0 siblings, 1 reply; 10+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino
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.
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).
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.
Valentin Schneider (10):
genirq: Add chip flag to denote automatic IRQ (un)masking
genirq: Define irq_ack() and irq_eoi() helpers
genirq: Employ ack_irq() and eoi_irq() where relevant
genirq: Add handle_strict_flow_irq() flow handler
genirq: Let purely flow-masked ONESHOT irqs through
unmask_threaded_irq()
genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if
available
irqchip/gic-v3-its: Use irq_chip_ack_parent()
irqchip/gic: Convert to handle_strict_flow_irq()
irqchip/gic-v3: Convert to handle_strict_flow_irq()
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 1 +
drivers/irqchip/irq-gic-v3-its.c | 1 +
drivers/irqchip/irq-gic-v3.c | 27 +++--
drivers/irqchip/irq-gic.c | 14 ++-
include/linux/irq.h | 15 ++-
kernel/irq/chip.c | 122 ++++++++++++++++++++---
kernel/irq/debugfs.c | 2 +
kernel/irq/internals.h | 7 ++
kernel/irq/manage.c | 2 +-
9 files changed, 159 insertions(+), 32 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
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
0 siblings, 0 replies; 10+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino
The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
the chip has the IRQCHIP_AUTOMASKS_FLOW flag.
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
kernel/irq/chip.c | 16 ++++++++++++++++
kernel/irq/internals.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 21a21baa1366..793dbd8307b9 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
cpumask_clear_cpu(cpu, desc->percpu_enabled);
}
+void ack_irq(struct irq_desc *desc)
+{
+ desc->irq_data.chip->irq_ack(&desc->irq_data);
+
+ if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+ irq_state_set_flow_masked(desc);
+}
+
+void eoi_irq(struct irq_desc *desc)
+{
+ desc->irq_data.chip->irq_eoi(&desc->irq_data);
+
+ if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+ irq_state_clr_flow_masked(desc);
+}
+
static inline void mask_ack_irq(struct irq_desc *desc)
{
if (desc->irq_data.chip->irq_mask_ack) {
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index b6c1cceddec0..090bd7868845 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
extern void irq_disable(struct irq_desc *desc);
extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
+extern void irq_ack(struct irq_desc *desc);
+extern void irq_eoi(struct irq_desc *desc);
extern void mask_irq(struct irq_desc *desc);
extern void unmask_irq(struct irq_desc *desc);
extern void unmask_threaded_irq(struct irq_desc *desc);
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
@ 2021-05-25 17:32 ` Valentin Schneider
0 siblings, 0 replies; 10+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino
The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
the chip has the IRQCHIP_AUTOMASKS_FLOW flag.
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
kernel/irq/chip.c | 16 ++++++++++++++++
kernel/irq/internals.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 21a21baa1366..793dbd8307b9 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
cpumask_clear_cpu(cpu, desc->percpu_enabled);
}
+void ack_irq(struct irq_desc *desc)
+{
+ desc->irq_data.chip->irq_ack(&desc->irq_data);
+
+ if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+ irq_state_set_flow_masked(desc);
+}
+
+void eoi_irq(struct irq_desc *desc)
+{
+ desc->irq_data.chip->irq_eoi(&desc->irq_data);
+
+ if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+ irq_state_clr_flow_masked(desc);
+}
+
static inline void mask_ack_irq(struct irq_desc *desc)
{
if (desc->irq_data.chip->irq_mask_ack) {
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index b6c1cceddec0..090bd7868845 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
extern void irq_disable(struct irq_desc *desc);
extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
+extern void irq_ack(struct irq_desc *desc);
+extern void irq_eoi(struct irq_desc *desc);
extern void mask_irq(struct irq_desc *desc);
extern void unmask_irq(struct irq_desc *desc);
extern void unmask_threaded_irq(struct irq_desc *desc);
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
2021-05-25 17:32 ` Valentin Schneider
(?)
@ 2021-05-25 20:54 ` kernel test robot
-1 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-05-25 20:54 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]
Hi Valentin,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on tip/irq/core]
[also build test WARNING on linux/master soc/for-next linus/master v5.13-rc3 next-20210525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210526-013542
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 006ae1970a8cde1d3e92da69b324d12880133a13
config: ia64-randconfig-r034-20210525 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/9cb26d40715c7a84d90accb09d808141446dd270
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210526-013542
git checkout 9cb26d40715c7a84d90accb09d808141446dd270
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/irq/chip.c:411:6: warning: no previous prototype for 'ack_irq' [-Wmissing-prototypes]
411 | void ack_irq(struct irq_desc *desc)
| ^~~~~~~
>> kernel/irq/chip.c:419:6: warning: no previous prototype for 'eoi_irq' [-Wmissing-prototypes]
419 | void eoi_irq(struct irq_desc *desc)
| ^~~~~~~
vim +/ack_irq +411 kernel/irq/chip.c
410
> 411 void ack_irq(struct irq_desc *desc)
412 {
413 desc->irq_data.chip->irq_ack(&desc->irq_data);
414
415 if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
416 irq_state_set_flow_masked(desc);
417 }
418
> 419 void eoi_irq(struct irq_desc *desc)
420 {
421 desc->irq_data.chip->irq_eoi(&desc->irq_data);
422
423 if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
424 irq_state_clr_flow_masked(desc);
425 }
426
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38143 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
2021-05-25 17:32 ` Valentin Schneider
@ 2021-05-27 10:55 ` Marc Zyngier
-1 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2021-05-27 10:55 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
Lorenzo Pieralisi, Vincenzo Frascino
On Tue, 25 May 2021 18:32:47 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
>
> The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
> bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
> those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
> the chip has the IRQCHIP_AUTOMASKS_FLOW flag.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> ---
> kernel/irq/chip.c | 16 ++++++++++++++++
> kernel/irq/internals.h | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 21a21baa1366..793dbd8307b9 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
> cpumask_clear_cpu(cpu, desc->percpu_enabled);
> }
>
> +void ack_irq(struct irq_desc *desc)
> +{
> + desc->irq_data.chip->irq_ack(&desc->irq_data);
> +
> + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> + irq_state_set_flow_masked(desc);
> +}
> +
> +void eoi_irq(struct irq_desc *desc)
> +{
> + desc->irq_data.chip->irq_eoi(&desc->irq_data);
> +
> + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> + irq_state_clr_flow_masked(desc);
> +}
> +
> static inline void mask_ack_irq(struct irq_desc *desc)
> {
> if (desc->irq_data.chip->irq_mask_ack) {
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index b6c1cceddec0..090bd7868845 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
> extern void irq_disable(struct irq_desc *desc);
> extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
> extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
> +extern void irq_ack(struct irq_desc *desc);
> +extern void irq_eoi(struct irq_desc *desc);
Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
have some naming consistency (yes, this may/will clash with existing
code, but we can fix that as well).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
@ 2021-05-27 10:55 ` Marc Zyngier
0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2021-05-27 10:55 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
Lorenzo Pieralisi, Vincenzo Frascino
On Tue, 25 May 2021 18:32:47 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
>
> The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
> bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
> those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
> the chip has the IRQCHIP_AUTOMASKS_FLOW flag.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> ---
> kernel/irq/chip.c | 16 ++++++++++++++++
> kernel/irq/internals.h | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 21a21baa1366..793dbd8307b9 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
> cpumask_clear_cpu(cpu, desc->percpu_enabled);
> }
>
> +void ack_irq(struct irq_desc *desc)
> +{
> + desc->irq_data.chip->irq_ack(&desc->irq_data);
> +
> + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> + irq_state_set_flow_masked(desc);
> +}
> +
> +void eoi_irq(struct irq_desc *desc)
> +{
> + desc->irq_data.chip->irq_eoi(&desc->irq_data);
> +
> + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> + irq_state_clr_flow_masked(desc);
> +}
> +
> static inline void mask_ack_irq(struct irq_desc *desc)
> {
> if (desc->irq_data.chip->irq_mask_ack) {
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index b6c1cceddec0..090bd7868845 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
> extern void irq_disable(struct irq_desc *desc);
> extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
> extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
> +extern void irq_ack(struct irq_desc *desc);
> +extern void irq_eoi(struct irq_desc *desc);
Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
have some naming consistency (yes, this may/will clash with existing
code, but we can fix that as well).
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
2021-05-27 10:55 ` Marc Zyngier
@ 2021-05-27 10:58 ` Marc Zyngier
-1 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2021-05-27 10:58 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
Lorenzo Pieralisi, Vincenzo Frascino
On Thu, 27 May 2021 11:55:50 +0100,
Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 25 May 2021 18:32:47 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
> >
> > The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
> > bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
> > those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
> > the chip has the IRQCHIP_AUTOMASKS_FLOW flag.
> >
> > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> > ---
> > kernel/irq/chip.c | 16 ++++++++++++++++
> > kernel/irq/internals.h | 2 ++
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index 21a21baa1366..793dbd8307b9 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
> > cpumask_clear_cpu(cpu, desc->percpu_enabled);
> > }
> >
> > +void ack_irq(struct irq_desc *desc)
> > +{
> > + desc->irq_data.chip->irq_ack(&desc->irq_data);
> > +
> > + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> > + irq_state_set_flow_masked(desc);
> > +}
> > +
> > +void eoi_irq(struct irq_desc *desc)
> > +{
> > + desc->irq_data.chip->irq_eoi(&desc->irq_data);
> > +
> > + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> > + irq_state_clr_flow_masked(desc);
> > +}
> > +
> > static inline void mask_ack_irq(struct irq_desc *desc)
> > {
> > if (desc->irq_data.chip->irq_mask_ack) {
> > diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> > index b6c1cceddec0..090bd7868845 100644
> > --- a/kernel/irq/internals.h
> > +++ b/kernel/irq/internals.h
> > @@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
> > extern void irq_disable(struct irq_desc *desc);
> > extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
> > extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
> > +extern void irq_ack(struct irq_desc *desc);
> > +extern void irq_eoi(struct irq_desc *desc);
>
> Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
> have some naming consistency (yes, this may/will clash with existing
> code, but we can fix that as well).
Actually, the helpers do have the right naming, but the internal
declarations are the ones that are wrong...
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
@ 2021-05-27 10:58 ` Marc Zyngier
0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2021-05-27 10:58 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
Lorenzo Pieralisi, Vincenzo Frascino
On Thu, 27 May 2021 11:55:50 +0100,
Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 25 May 2021 18:32:47 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
> >
> > The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
> > bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
> > those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
> > the chip has the IRQCHIP_AUTOMASKS_FLOW flag.
> >
> > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> > ---
> > kernel/irq/chip.c | 16 ++++++++++++++++
> > kernel/irq/internals.h | 2 ++
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index 21a21baa1366..793dbd8307b9 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
> > cpumask_clear_cpu(cpu, desc->percpu_enabled);
> > }
> >
> > +void ack_irq(struct irq_desc *desc)
> > +{
> > + desc->irq_data.chip->irq_ack(&desc->irq_data);
> > +
> > + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> > + irq_state_set_flow_masked(desc);
> > +}
> > +
> > +void eoi_irq(struct irq_desc *desc)
> > +{
> > + desc->irq_data.chip->irq_eoi(&desc->irq_data);
> > +
> > + if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
> > + irq_state_clr_flow_masked(desc);
> > +}
> > +
> > static inline void mask_ack_irq(struct irq_desc *desc)
> > {
> > if (desc->irq_data.chip->irq_mask_ack) {
> > diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> > index b6c1cceddec0..090bd7868845 100644
> > --- a/kernel/irq/internals.h
> > +++ b/kernel/irq/internals.h
> > @@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
> > extern void irq_disable(struct irq_desc *desc);
> > extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
> > extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
> > +extern void irq_ack(struct irq_desc *desc);
> > +extern void irq_eoi(struct irq_desc *desc);
>
> Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
> have some naming consistency (yes, this may/will clash with existing
> code, but we can fix that as well).
Actually, the helpers do have the right naming, but the internal
declarations are the ones that are wrong...
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
2021-05-27 10:58 ` Marc Zyngier
@ 2021-06-01 10:25 ` Valentin Schneider
-1 siblings, 0 replies; 10+ messages in thread
From: Valentin Schneider @ 2021-06-01 10:25 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
Lorenzo Pieralisi, Vincenzo Frascino
On 27/05/21 11:58, Marc Zyngier wrote:
> On Thu, 27 May 2021 11:55:50 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Tue, 25 May 2021 18:32:47 +0100,
>> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> > diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
>> > index b6c1cceddec0..090bd7868845 100644
>> > --- a/kernel/irq/internals.h
>> > +++ b/kernel/irq/internals.h
>> > @@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
>> > extern void irq_disable(struct irq_desc *desc);
>> > extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
>> > extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
>> > +extern void irq_ack(struct irq_desc *desc);
>> > +extern void irq_eoi(struct irq_desc *desc);
>>
>> Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
>> have some naming consistency (yes, this may/will clash with existing
>> code, but we can fix that as well).
>
> Actually, the helpers do have the right naming, but the internal
> declarations are the ones that are wrong...
>
Doh!
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
@ 2021-06-01 10:25 ` Valentin Schneider
0 siblings, 0 replies; 10+ messages in thread
From: Valentin Schneider @ 2021-06-01 10:25 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
Lorenzo Pieralisi, Vincenzo Frascino
On 27/05/21 11:58, Marc Zyngier wrote:
> On Thu, 27 May 2021 11:55:50 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Tue, 25 May 2021 18:32:47 +0100,
>> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> > diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
>> > index b6c1cceddec0..090bd7868845 100644
>> > --- a/kernel/irq/internals.h
>> > +++ b/kernel/irq/internals.h
>> > @@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
>> > extern void irq_disable(struct irq_desc *desc);
>> > extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
>> > extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
>> > +extern void irq_ack(struct irq_desc *desc);
>> > +extern void irq_eoi(struct irq_desc *desc);
>>
>> Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
>> have some naming consistency (yes, this may/will clash with existing
>> code, but we can fix that as well).
>
> Actually, the helpers do have the right naming, but the internal
> declarations are the ones that are wrong...
>
Doh!
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-01 10:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 23:01 [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers kernel test robot
-- strict thread matches above, loose matches on Subject: below --
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 ` [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
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.