2021-10-01 18:37:16

by Pingfan Liu

[permalink] [raw]
Subject: [PATCHv4 0/3] arm64/irqentry: remove duplicate housekeeping of rcu

When an IRQ is taken, some accounting needs to be performed to enter and
exit IRQ context around the IRQ handler. Historically arch code would
leave this to the irqchip or core IRQ code, but these days we want this
to happen in exception entry code, and architectures such as arm64 do
this.

Currently handle_domain_irq() performs this entry/exit accounting, and
if used on an architecture where the entry code also does this, the
entry/exit accounting will be performed twice per IRQ. This is
problematic as core RCU code such as rcu_is_cpu_rrupt_from_idle()
depends on this happening once per IRQ, and will not detect quescent
periods correctly, leading to stall warnings.

As irqchip drivers which use handle_domain_irq() need to work on
architectures with or without their own entry/exit accounting, this
patch makes handle_domain_irq() conditionally perform the entry
accounting depending on a new HAVE_ARCH_IRQENTRY Kconfig symbol that
architectures can select if they perform this entry accounting
themselves.

V3 -> V4:
address Signed-off-by in [2/3]
improve commit log in [3/3]

V2 -> V3:
Drop other patches and concentrate on the purpose of [3-4/5] of V2.
And lift the level, where to add {irq_enter,exit}_rcu(), from the
interrupt controler to exception entry

History:
V1: https://lore.kernel.org/linux-arm-kernel/[email protected]
V2: https://lore.kernel.org/linux-arm-kernel/[email protected]
V3: https://lore.kernel.org/linux-arm-kernel/[email protected]


Cc: "Paul E. McKenney" <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Sami Tolvanen <[email protected]>
Cc: Julien Thierry <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Yuichi Ito <[email protected]>
Cc: [email protected]
To: [email protected]

Mark Rutland (1):
arm64: entry: refactor EL1 interrupt entry logic

Pingfan Liu (2):
kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch
optional
arm64: entry: avoid double-accounting IRQ RCU entry

arch/arm64/Kconfig | 1 +
arch/arm64/kernel/entry-common.c | 47 +++++++++++++++++---------------
kernel/irq/Kconfig | 3 ++
kernel/irq/irqdesc.c | 4 +++
4 files changed, 33 insertions(+), 22 deletions(-)

--
2.31.1


2021-10-01 18:37:40

by Pingfan Liu

[permalink] [raw]
Subject: [PATCHv4 1/3] kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch optional

When an IRQ is taken, some accounting needs to be performed to enter and
exit IRQ context around the IRQ handler. Historically arch code would
leave this to the irqchip or core IRQ code, but these days we want this
to happen in exception entry code, and architectures such as arm64 do
this.

Currently handle_domain_irq() performs this entry/exit accounting, and
if used on an architecture where the entry code also does this, the
entry/exit accounting will be performed twice per IRQ. This is
problematic as core RCU code such as rcu_is_cpu_rrupt_from_idle()
depends on this happening once per IRQ, and will not detect quescent
periods correctly, leading to stall warnings.

As irqchip drivers which use handle_domain_irq() need to work on
architectures with or without their own entry/exit accounting, this
patch makes handle_domain_irq() conditionally perform the entry
accounting depending on a new HAVE_ARCH_IRQENTRY Kconfig symbol that
architectures can select if they perform this entry accounting
themselves.

For architectures which do not select the symbol. there should be no
functional change as a result of this patch.

Signed-off-by: Pingfan Liu <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Joey Gouly <[email protected]>
Cc: Sami Tolvanen <[email protected]>
Cc: Julien Thierry <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Yuichi Ito <[email protected]>
Cc: [email protected]
To: [email protected]
---
kernel/irq/Kconfig | 3 +++
kernel/irq/irqdesc.c | 4 ++++
2 files changed, 7 insertions(+)

diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index fbc54c2a7f23..defa1db2d664 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -100,6 +100,9 @@ config IRQ_MSI_IOMMU
config HANDLE_DOMAIN_IRQ
bool

+config HAVE_ARCH_IRQENTRY
+ bool
+
config IRQ_TIMINGS
bool

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 4e3c29bb603c..fd5dd9d278b5 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -693,7 +693,9 @@ int handle_domain_irq(struct irq_domain *domain,
struct irq_desc *desc;
int ret = 0;

+#ifndef CONFIG_HAVE_ARCH_IRQENTRY
irq_enter();
+#endif

/* The irqdomain code provides boundary checks */
desc = irq_resolve_mapping(domain, hwirq);
@@ -702,7 +704,9 @@ int handle_domain_irq(struct irq_domain *domain,
else
ret = -EINVAL;

+#ifndef CONFIG_HAVE_ARCH_IRQENTRY
irq_exit();
+#endif
set_irq_regs(old_regs);
return ret;
}
--
2.31.1

2021-10-04 18:01:07

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCHv4 1/3] kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch optional

On Fri, 01 Oct 2021 15:44:04 +0100,
Pingfan Liu <[email protected]> wrote:
>
> When an IRQ is taken, some accounting needs to be performed to enter and
> exit IRQ context around the IRQ handler. Historically arch code would
> leave this to the irqchip or core IRQ code, but these days we want this
> to happen in exception entry code, and architectures such as arm64 do
> this.
>
> Currently handle_domain_irq() performs this entry/exit accounting, and
> if used on an architecture where the entry code also does this, the
> entry/exit accounting will be performed twice per IRQ. This is
> problematic as core RCU code such as rcu_is_cpu_rrupt_from_idle()
> depends on this happening once per IRQ, and will not detect quescent
> periods correctly, leading to stall warnings.
>
> As irqchip drivers which use handle_domain_irq() need to work on
> architectures with or without their own entry/exit accounting, this
> patch makes handle_domain_irq() conditionally perform the entry
> accounting depending on a new HAVE_ARCH_IRQENTRY Kconfig symbol that
> architectures can select if they perform this entry accounting
> themselves.
>
> For architectures which do not select the symbol. there should be no
> functional change as a result of this patch.
>
> Signed-off-by: Pingfan Liu <[email protected]>
> Reviewed-by: Mark Rutland <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Joey Gouly <[email protected]>
> Cc: Sami Tolvanen <[email protected]>
> Cc: Julien Thierry <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Yuichi Ito <[email protected]>
> Cc: [email protected]
> To: [email protected]

Reviewed-by: Marc Zyngier <[email protected]>

M.

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

2021-10-05 00:09:55

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCHv4 0/3] arm64/irqentry: remove duplicate housekeeping of rcu

On Fri, 01 Oct 2021 15:44:03 +0100,
Pingfan Liu <[email protected]> wrote:
>
> When an IRQ is taken, some accounting needs to be performed to enter and
> exit IRQ context around the IRQ handler. Historically arch code would
> leave this to the irqchip or core IRQ code, but these days we want this
> to happen in exception entry code, and architectures such as arm64 do
> this.
>
> Currently handle_domain_irq() performs this entry/exit accounting, and
> if used on an architecture where the entry code also does this, the
> entry/exit accounting will be performed twice per IRQ. This is
> problematic as core RCU code such as rcu_is_cpu_rrupt_from_idle()
> depends on this happening once per IRQ, and will not detect quescent
> periods correctly, leading to stall warnings.
>
> As irqchip drivers which use handle_domain_irq() need to work on
> architectures with or without their own entry/exit accounting, this
> patch makes handle_domain_irq() conditionally perform the entry
> accounting depending on a new HAVE_ARCH_IRQENTRY Kconfig symbol that
> architectures can select if they perform this entry accounting
> themselves.
>
> V3 -> V4:
> address Signed-off-by in [2/3]
> improve commit log in [3/3]
>
> V2 -> V3:
> Drop other patches and concentrate on the purpose of [3-4/5] of V2.
> And lift the level, where to add {irq_enter,exit}_rcu(), from the
> interrupt controler to exception entry
>
> History:
> V1: https://lore.kernel.org/linux-arm-kernel/[email protected]
> V2: https://lore.kernel.org/linux-arm-kernel/[email protected]
> V3: https://lore.kernel.org/linux-arm-kernel/[email protected]
>
>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Joey Gouly <[email protected]>
> Cc: Sami Tolvanen <[email protected]>
> Cc: Julien Thierry <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Yuichi Ito <[email protected]>
> Cc: [email protected]
> To: [email protected]
>
> Mark Rutland (1):
> arm64: entry: refactor EL1 interrupt entry logic
>
> Pingfan Liu (2):
> kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch
> optional
> arm64: entry: avoid double-accounting IRQ RCU entry
>
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/entry-common.c | 47 +++++++++++++++++---------------
> kernel/irq/Kconfig | 3 ++
> kernel/irq/irqdesc.c | 4 +++
> 4 files changed, 33 insertions(+), 22 deletions(-)

Given that most of the changes are in the arm64 tree, it'd be good if
the whole series went via the arm64 tree, most probably as a fix.

Thanks,

M.

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

2021-10-05 17:23:42

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCHv4 0/3] arm64/irqentry: remove duplicate housekeeping of rcu

On Fri, 1 Oct 2021 22:44:03 +0800, Pingfan Liu wrote:
> When an IRQ is taken, some accounting needs to be performed to enter and
> exit IRQ context around the IRQ handler. Historically arch code would
> leave this to the irqchip or core IRQ code, but these days we want this
> to happen in exception entry code, and architectures such as arm64 do
> this.
>
> Currently handle_domain_irq() performs this entry/exit accounting, and
> if used on an architecture where the entry code also does this, the
> entry/exit accounting will be performed twice per IRQ. This is
> problematic as core RCU code such as rcu_is_cpu_rrupt_from_idle()
> depends on this happening once per IRQ, and will not detect quescent
> periods correctly, leading to stall warnings.
>
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/3] kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch optional
https://git.kernel.org/arm64/c/db795cf55b21
[2/3] arm64: entry: refactor EL1 interrupt entry logic
https://git.kernel.org/arm64/c/ad0d5cfb9535
[3/3] arm64: entry: avoid double-accounting IRQ RCU entry
https://git.kernel.org/arm64/c/12074b059fdc

--
Catalin