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

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

[Pingfan Liu]

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/1607912752-12481-1-git-send-email-kernelfans@gmail.com
V2: https://lore.kernel.org/linux-arm-kernel/20210924132837.45994-1-kernelfans@gmail.com
V3: https://lore.kernel.org/linux-arm-kernel/20210930131708.35328-1-kernelfans@gmail.com


Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org

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

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

[Pingfan Liu]

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/1607912752-12481-1-git-send-email-kernelfans@gmail.com
V2: https://lore.kernel.org/linux-arm-kernel/20210924132837.45994-1-kernelfans@gmail.com
V3: https://lore.kernel.org/linux-arm-kernel/20210930131708.35328-1-kernelfans@gmail.com


Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org

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


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

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

[Pingfan Liu]

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 <kernelfans@gmail.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 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

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

[Pingfan Liu]

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 <kernelfans@gmail.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 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


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

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

[Marc Zyngier]

On Fri, 01 Oct 2021 15:44:04 +0100,
Pingfan Liu <kernelfans@gmail.com> 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 <kernelfans@gmail.com>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
> Cc: linux-kernel@vger.kernel.org
> To: linux-arm-kernel@lists.infradead.org

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

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

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

[Marc Zyngier]

On Fri, 01 Oct 2021 15:44:04 +0100,
Pingfan Liu <kernelfans@gmail.com> 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 <kernelfans@gmail.com>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
> Cc: linux-kernel@vger.kernel.org
> To: linux-arm-kernel@lists.infradead.org

Reviewed-by: Marc Zyngier <maz@kernel.org>

	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

[PATCHv4 2/3] arm64: entry: refactor EL1 interrupt entry logic

[Pingfan Liu]

From: Mark Rutland <mark.rutland@arm.com>

Currently we distinguish IRQ and definitely-PNMI at entry/exit time
via the enter_el1_irq_or_nmi() and enter_el1_irq_or_nmi() helpers. In
subsequent patches we'll need to handle the two cases more distinctly
in the body of the exception handler.

To make this possible, this patch refactors el1_interrupt to be a
top-level dispatcher to separate handlers for the IRQ and PNMI cases,
removing the need for the enter_el1_irq_or_nmi() and
exit_el1_irq_or_nmi() helpers.

Note that since arm64_enter_nmi() calls __nmi_enter(), which
increments the preemt_count, we could never preempt when handling a
PNMI. We now only check for preemption in the IRQ case, which makes
this clearer.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/kernel/entry-common.c | 43 ++++++++++++++++----------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 32f9796c4ffe..fecf046f0708 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -219,22 +219,6 @@ static void noinstr arm64_exit_el1_dbg(struct pt_regs *regs)
 		lockdep_hardirqs_on(CALLER_ADDR0);
 }
 
-static void noinstr enter_el1_irq_or_nmi(struct pt_regs *regs)
-{
-	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
-		arm64_enter_nmi(regs);
-	else
-		enter_from_kernel_mode(regs);
-}
-
-static void noinstr exit_el1_irq_or_nmi(struct pt_regs *regs)
-{
-	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
-		arm64_exit_nmi(regs);
-	else
-		exit_to_kernel_mode(regs);
-}
-
 static void __sched arm64_preempt_schedule_irq(void)
 {
 	lockdep_assert_irqs_disabled();
@@ -432,14 +416,19 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
 	}
 }
 
-static void noinstr el1_interrupt(struct pt_regs *regs,
-				  void (*handler)(struct pt_regs *))
+static __always_inline void
+__el1_pnmi(struct pt_regs *regs, void (*handler)(struct pt_regs *))
 {
-	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
-
-	enter_el1_irq_or_nmi(regs);
+	arm64_enter_nmi(regs);
 	do_interrupt_handler(regs, handler);
+	arm64_exit_nmi(regs);
+}
 
+static __always_inline void
+__el1_interrupt(struct pt_regs *regs, void (*handler)(struct pt_regs *))
+{
+	enter_from_kernel_mode(regs);
+	do_interrupt_handler(regs, handler);
 	/*
 	 * Note: thread_info::preempt_count includes both thread_info::count
 	 * and thread_info::need_resched, and is not equivalent to
@@ -448,8 +437,18 @@ static void noinstr el1_interrupt(struct pt_regs *regs,
 	if (IS_ENABLED(CONFIG_PREEMPTION) &&
 	    READ_ONCE(current_thread_info()->preempt_count) == 0)
 		arm64_preempt_schedule_irq();
+	exit_to_kernel_mode(regs);
+}
 
-	exit_el1_irq_or_nmi(regs);
+static void noinstr el1_interrupt(struct pt_regs *regs,
+				  void (*handler)(struct pt_regs *))
+{
+	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
+
+	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
+		__el1_pnmi(regs, handler);
+	else
+		__el1_interrupt(regs, handler);
 }
 
 asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
-- 
2.31.1

[PATCHv4 2/3] arm64: entry: refactor EL1 interrupt entry logic

[Pingfan Liu]

From: Mark Rutland <mark.rutland@arm.com>

Currently we distinguish IRQ and definitely-PNMI at entry/exit time
via the enter_el1_irq_or_nmi() and enter_el1_irq_or_nmi() helpers. In
subsequent patches we'll need to handle the two cases more distinctly
in the body of the exception handler.

To make this possible, this patch refactors el1_interrupt to be a
top-level dispatcher to separate handlers for the IRQ and PNMI cases,
removing the need for the enter_el1_irq_or_nmi() and
exit_el1_irq_or_nmi() helpers.

Note that since arm64_enter_nmi() calls __nmi_enter(), which
increments the preemt_count, we could never preempt when handling a
PNMI. We now only check for preemption in the IRQ case, which makes
this clearer.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/kernel/entry-common.c | 43 ++++++++++++++++----------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 32f9796c4ffe..fecf046f0708 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -219,22 +219,6 @@ static void noinstr arm64_exit_el1_dbg(struct pt_regs *regs)
 		lockdep_hardirqs_on(CALLER_ADDR0);
 }
 
-static void noinstr enter_el1_irq_or_nmi(struct pt_regs *regs)
-{
-	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
-		arm64_enter_nmi(regs);
-	else
-		enter_from_kernel_mode(regs);
-}
-
-static void noinstr exit_el1_irq_or_nmi(struct pt_regs *regs)
-{
-	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
-		arm64_exit_nmi(regs);
-	else
-		exit_to_kernel_mode(regs);
-}
-
 static void __sched arm64_preempt_schedule_irq(void)
 {
 	lockdep_assert_irqs_disabled();
@@ -432,14 +416,19 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
 	}
 }
 
-static void noinstr el1_interrupt(struct pt_regs *regs,
-				  void (*handler)(struct pt_regs *))
+static __always_inline void
+__el1_pnmi(struct pt_regs *regs, void (*handler)(struct pt_regs *))
 {
-	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
-
-	enter_el1_irq_or_nmi(regs);
+	arm64_enter_nmi(regs);
 	do_interrupt_handler(regs, handler);
+	arm64_exit_nmi(regs);
+}
 
+static __always_inline void
+__el1_interrupt(struct pt_regs *regs, void (*handler)(struct pt_regs *))
+{
+	enter_from_kernel_mode(regs);
+	do_interrupt_handler(regs, handler);
 	/*
 	 * Note: thread_info::preempt_count includes both thread_info::count
 	 * and thread_info::need_resched, and is not equivalent to
@@ -448,8 +437,18 @@ static void noinstr el1_interrupt(struct pt_regs *regs,
 	if (IS_ENABLED(CONFIG_PREEMPTION) &&
 	    READ_ONCE(current_thread_info()->preempt_count) == 0)
 		arm64_preempt_schedule_irq();
+	exit_to_kernel_mode(regs);
+}
 
-	exit_el1_irq_or_nmi(regs);
+static void noinstr el1_interrupt(struct pt_regs *regs,
+				  void (*handler)(struct pt_regs *))
+{
+	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
+
+	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
+		__el1_pnmi(regs, handler);
+	else
+		__el1_interrupt(regs, handler);
 }
 
 asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
-- 
2.31.1


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

[PATCHv4 3/3] arm64: entry: avoid double-accounting IRQ RCU entry

[Pingfan Liu]

When an IRQ is taken, some accounting needs to be performed to enter and
exit IRQ context around the IRQ handler. On arm64 some of this
accounting is performed by both the architecture code and the IRQ domain
code, resulting in calling rcu_irq_enter() twice per exception entry,
violating the expectations of the core RCU code, and resulting in
failing to identify quiescent periods correctly (e.g. in
rcu_is_cpu_rrupt_from_idle()).

To fix this, we must perform all the accounting from the architecture
code. We prevent the IRQ domain code from performing any accounting by
selecting HAVE_ARCH_IRQENTRY, and must call irq_enter_rcu() and
irq_exit_rcu() around invoking the root IRQ handler.

When we take a pNMI from a context with IRQs disabled, we'll perform the
necessary accounting as part of arm64_enter_nmi() and arm64_exit_nmi(),
and should only call irq_enter_rcu() and irq_exit_rcu() when we may have
taken a regular interrupt.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/Kconfig               | 1 +
 arch/arm64/kernel/entry-common.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5c7ae4c3954b..c0ba052116b2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -144,6 +144,7 @@ config ARM64
 	select HAVE_ARCH_BITREVERSE
 	select HAVE_ARCH_COMPILER_H
 	select HAVE_ARCH_HUGE_VMAP
+	select HAVE_ARCH_IRQENTRY
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN if !(ARM64_16K_PAGES && ARM64_VA_BITS_48)
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fecf046f0708..af931e63dc59 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -428,7 +428,9 @@ static __always_inline void
 __el1_interrupt(struct pt_regs *regs, void (*handler)(struct pt_regs *))
 {
 	enter_from_kernel_mode(regs);
+	irq_enter_rcu();
 	do_interrupt_handler(regs, handler);
+	irq_exit_rcu();
 	/*
 	 * Note: thread_info::preempt_count includes both thread_info::count
 	 * and thread_info::need_resched, and is not equivalent to
@@ -666,7 +668,9 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
 	if (regs->pc & BIT(55))
 		arm64_apply_bp_hardening();
 
+	irq_enter_rcu();
 	do_interrupt_handler(regs, handler);
+	irq_exit_rcu();
 
 	exit_to_user_mode(regs);
 }
-- 
2.31.1

[PATCHv4 3/3] arm64: entry: avoid double-accounting IRQ RCU entry

[Pingfan Liu]

When an IRQ is taken, some accounting needs to be performed to enter and
exit IRQ context around the IRQ handler. On arm64 some of this
accounting is performed by both the architecture code and the IRQ domain
code, resulting in calling rcu_irq_enter() twice per exception entry,
violating the expectations of the core RCU code, and resulting in
failing to identify quiescent periods correctly (e.g. in
rcu_is_cpu_rrupt_from_idle()).

To fix this, we must perform all the accounting from the architecture
code. We prevent the IRQ domain code from performing any accounting by
selecting HAVE_ARCH_IRQENTRY, and must call irq_enter_rcu() and
irq_exit_rcu() around invoking the root IRQ handler.

When we take a pNMI from a context with IRQs disabled, we'll perform the
necessary accounting as part of arm64_enter_nmi() and arm64_exit_nmi(),
and should only call irq_enter_rcu() and irq_exit_rcu() when we may have
taken a regular interrupt.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Julien Thierry <julien.thierry@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/Kconfig               | 1 +
 arch/arm64/kernel/entry-common.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5c7ae4c3954b..c0ba052116b2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -144,6 +144,7 @@ config ARM64
 	select HAVE_ARCH_BITREVERSE
 	select HAVE_ARCH_COMPILER_H
 	select HAVE_ARCH_HUGE_VMAP
+	select HAVE_ARCH_IRQENTRY
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN if !(ARM64_16K_PAGES && ARM64_VA_BITS_48)
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index fecf046f0708..af931e63dc59 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -428,7 +428,9 @@ static __always_inline void
 __el1_interrupt(struct pt_regs *regs, void (*handler)(struct pt_regs *))
 {
 	enter_from_kernel_mode(regs);
+	irq_enter_rcu();
 	do_interrupt_handler(regs, handler);
+	irq_exit_rcu();
 	/*
 	 * Note: thread_info::preempt_count includes both thread_info::count
 	 * and thread_info::need_resched, and is not equivalent to
@@ -666,7 +668,9 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
 	if (regs->pc & BIT(55))
 		arm64_apply_bp_hardening();
 
+	irq_enter_rcu();
 	do_interrupt_handler(regs, handler);
+	irq_exit_rcu();
 
 	exit_to_user_mode(regs);
 }
-- 
2.31.1


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

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

[Marc Zyngier]

On Fri, 01 Oct 2021 15:44:03 +0100,
Pingfan Liu <kernelfans@gmail.com> 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/1607912752-12481-1-git-send-email-kernelfans@gmail.com
> V2: https://lore.kernel.org/linux-arm-kernel/20210924132837.45994-1-kernelfans@gmail.com
> V3: https://lore.kernel.org/linux-arm-kernel/20210930131708.35328-1-kernelfans@gmail.com
> 
> 
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
> Cc: linux-kernel@vger.kernel.org
> To: linux-arm-kernel@lists.infradead.org
> 
> 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.

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

[Marc Zyngier]

On Fri, 01 Oct 2021 15:44:03 +0100,
Pingfan Liu <kernelfans@gmail.com> 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/1607912752-12481-1-git-send-email-kernelfans@gmail.com
> V2: https://lore.kernel.org/linux-arm-kernel/20210924132837.45994-1-kernelfans@gmail.com
> V3: https://lore.kernel.org/linux-arm-kernel/20210930131708.35328-1-kernelfans@gmail.com
> 
> 
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
> Cc: linux-kernel@vger.kernel.org
> To: linux-arm-kernel@lists.infradead.org
> 
> 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.

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

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

[Catalin Marinas]

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

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

[Catalin Marinas]

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


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