Hi all,
Currently arm64 supports per-CPU IRQ stack, but softirqs are
still handled in the task context.
Since any call to local_bh_enable() at any level in the task's
call stack may trigger a softirq processing run, which could
potentially cause a task stack overflow if the combined stack
footprints exceed the stack's size. And we did encounter this
situation in the real environment:
Call trace:
dump_backtrace+0x0/0x1cc,
show_stack+0x14/0x1c,
dump_stack+0xc4/0xfc,
panic+0x150/0x2c8,
panic+0x0/0x2c8,
handle_bad_stack+0x11c/0x130,
__bad_stack+0x88/0x8c,
vsnprintf+0x2c/0x524,
vscnprintf+0x38/0x7c,
scnprintf+0x6c/0x90,
/* ... */
__do_softirq+0x1e0/0x370,
do_softirq+0x40/0x50,
__local_bh_enable_ip+0x8c/0x90,
_raw_spin_unlock_bh+0x1c/0x24,
/* ... */
process_one_work+0x1dc/0x3e4,
worker_thread+0x260/0x360,
kthread+0x118/0x128,
ret_from_fork+0x10/0x18,
So let's run these softirqs on the IRQ stack as well.
This series is based on next-20220707.
Comments and suggestions are welcome.
Thanks,
Qi
RFC: https://lore.kernel.org/lkml/[email protected]/
Changelog in RFC -> v1:
- fix conflicts with commit f2c5092190f2 ("arch/*: Disable softirq stacks on PREEMPT_RT.")
Qi Zheng (2):
arm64: run softirqs on the per-CPU IRQ stack
arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK
arch/arm64/Kconfig | 2 ++
arch/arm64/include/asm/exception.h | 4 +++-
arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
arch/arm64/kernel/entry.S | 6 ++++--
arch/arm64/kernel/irq.c | 14 ++++++++++++++
5 files changed, 43 insertions(+), 13 deletions(-)
--
2.20.1
Currently arm64 supports per-CPU IRQ stack, but softirqs
are still handled in the task context.
Since any call to local_bh_enable() at any level in the task's
call stack may trigger a softirq processing run, which could
potentially cause a task stack overflow if the combined stack
footprints exceed the stack's size, let's run these softirqs
on the IRQ stack as well.
Signed-off-by: Qi Zheng <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/irq.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4c1e1d2d2f8b..be0a9f0052ee 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -230,6 +230,7 @@ config ARM64
select HAVE_ARCH_USERFAULTFD_MINOR if USERFAULTFD
select TRACE_IRQFLAGS_SUPPORT
select TRACE_IRQFLAGS_NMI_SUPPORT
+ select HAVE_SOFTIRQ_ON_OWN_STACK
help
ARM 64-bit (AArch64) Linux support.
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index bda49430c9ea..c36ad20a52f3 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -22,6 +22,7 @@
#include <linux/vmalloc.h>
#include <asm/daifflags.h>
#include <asm/vmap_stack.h>
+#include <asm/exception.h>
/* Only access this in an NMI enter/exit */
DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
@@ -71,6 +72,18 @@ static void init_irq_stacks(void)
}
#endif
+#ifndef CONFIG_PREEMPT_RT
+static void ____do_softirq(struct pt_regs *regs)
+{
+ __do_softirq();
+}
+
+void do_softirq_own_stack(void)
+{
+ call_on_irq_stack(NULL, ____do_softirq);
+}
+#endif
+
static void default_handle_irq(struct pt_regs *regs)
{
panic("IRQ taken without a root IRQ handler\n");
--
2.20.1
Since softirqs are handled on the per-CPU IRQ stack,
let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
the core code to invoke __do_softirq() directly without
going through do_softirq_own_stack().
Signed-off-by: Qi Zheng <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/exception.h | 4 +++-
arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
arch/arm64/kernel/entry.S | 6 ++++--
arch/arm64/kernel/irq.c | 5 +++--
5 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index be0a9f0052ee..d2cc7daecce3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -231,6 +231,7 @@ config ARM64
select TRACE_IRQFLAGS_SUPPORT
select TRACE_IRQFLAGS_NMI_SUPPORT
select HAVE_SOFTIRQ_ON_OWN_STACK
+ select HAVE_IRQ_EXIT_ON_IRQ_STACK
help
ARM 64-bit (AArch64) Linux support.
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index d94aecff9690..8bff0aa7ab50 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs);
asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
asmlinkage void call_on_irq_stack(struct pt_regs *regs,
- void (*func)(struct pt_regs *));
+ void (*func)(struct pt_regs *),
+ void (*do_func)(struct pt_regs *,
+ void (*)(struct pt_regs *)));
asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c75ca36b4a49..935d1ab150b5 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void)
}
static void do_interrupt_handler(struct pt_regs *regs,
- void (*handler)(struct pt_regs *))
+ void (*handler)(struct pt_regs *),
+ void (*do_handler)(struct pt_regs *,
+ void (*)(struct pt_regs *)))
{
struct pt_regs *old_regs = set_irq_regs(regs);
if (on_thread_stack())
- call_on_irq_stack(regs, handler);
+ call_on_irq_stack(regs, handler, do_handler);
else
- handler(regs);
+ do_handler(regs, handler);
set_irq_regs(old_regs);
}
@@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
}
}
+static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
+{
+ handler(regs);
+}
+
static __always_inline void __el1_pnmi(struct pt_regs *regs,
void (*handler)(struct pt_regs *))
{
arm64_enter_nmi(regs);
- do_interrupt_handler(regs, handler);
+ do_interrupt_handler(regs, handler, nmi_handler);
arm64_exit_nmi(regs);
}
+static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
+{
+ irq_enter_rcu();
+ handler(regs);
+ irq_exit_rcu();
+}
+
static __always_inline void __el1_irq(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();
+ do_interrupt_handler(regs, handler, irq_handler);
arm64_preempt_schedule_irq();
@@ -699,9 +711,7 @@ 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();
+ do_interrupt_handler(regs, handler, irq_handler);
exit_to_user_mode(regs);
}
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 254fe31c03a0..1c351391f6bd 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
/*
* void call_on_irq_stack(struct pt_regs *regs,
- * void (*func)(struct pt_regs *));
+ * void (*func)(struct pt_regs *)
+ * void (*do_func)(struct pt_regs *,
+ * void (*)(struct pt_regs *)));
*
* Calls func(regs) using this CPU's irq stack and shadow irq stack.
*/
@@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
/* Move to the new stack and call the function there */
mov sp, x16
- blr x1
+ blr x2
/*
* Restore the SP from the FP, and restore the FP and LR from the frame
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index c36ad20a52f3..003db605bc4f 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -73,14 +73,15 @@ static void init_irq_stacks(void)
#endif
#ifndef CONFIG_PREEMPT_RT
-static void ____do_softirq(struct pt_regs *regs)
+static void ____do_softirq(struct pt_regs *regs,
+ void (*handler)(struct pt_regs *))
{
__do_softirq();
}
void do_softirq_own_stack(void)
{
- call_on_irq_stack(NULL, ____do_softirq);
+ call_on_irq_stack(NULL, NULL, ____do_softirq);
}
#endif
--
2.20.1
On 2022/7/8 17:49, Qi Zheng wrote:
> Hi all,
>
> Currently arm64 supports per-CPU IRQ stack, but softirqs are
> still handled in the task context.
>
> Since any call to local_bh_enable() at any level in the task's
> call stack may trigger a softirq processing run, which could
> potentially cause a task stack overflow if the combined stack
> footprints exceed the stack's size. And we did encounter this
> situation in the real environment:
>
> Call trace:
> dump_backtrace+0x0/0x1cc,
> show_stack+0x14/0x1c,
> dump_stack+0xc4/0xfc,
> panic+0x150/0x2c8,
> panic+0x0/0x2c8,
> handle_bad_stack+0x11c/0x130,
> __bad_stack+0x88/0x8c,
> vsnprintf+0x2c/0x524,
> vscnprintf+0x38/0x7c,
> scnprintf+0x6c/0x90,
> /* ... */
> __do_softirq+0x1e0/0x370,
> do_softirq+0x40/0x50,
> __local_bh_enable_ip+0x8c/0x90,
> _raw_spin_unlock_bh+0x1c/0x24,
> /* ... */
> process_one_work+0x1dc/0x3e4,
> worker_thread+0x260/0x360,
> kthread+0x118/0x128,
> ret_from_fork+0x10/0x18,
>
> So let's run these softirqs on the IRQ stack as well.
>
> This series is based on next-20220707.
>
> Comments and suggestions are welcome.
>
> Thanks,
> Qi
>
> RFC: https://lore.kernel.org/lkml/[email protected]/
>
> Changelog in RFC -> v1:
> - fix conflicts with commit f2c5092190f2 ("arch/*: Disable softirq stacks on PREEMPT_RT.")
>
> Qi Zheng (2):
> arm64: run softirqs on the per-CPU IRQ stack
> arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK
>
> arch/arm64/Kconfig | 2 ++
> arch/arm64/include/asm/exception.h | 4 +++-
> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
> arch/arm64/kernel/entry.S | 6 ++++--
> arch/arm64/kernel/irq.c | 14 ++++++++++++++
> 5 files changed, 43 insertions(+), 13 deletions(-)
>
Hi all,
Any other suggestions and comments for this patch set? If not, can
this patch set be merged into some trees for testing? :)
Thanks,
Qi
On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]> wrote:
>
> Currently arm64 supports per-CPU IRQ stack, but softirqs
> are still handled in the task context.
>
> Since any call to local_bh_enable() at any level in the task's
> call stack may trigger a softirq processing run, which could
> potentially cause a task stack overflow if the combined stack
> footprints exceed the stack's size, let's run these softirqs
> on the IRQ stack as well.
>
> Signed-off-by: Qi Zheng <[email protected]>
This seems like a nice improvement, and this version addresses
my concern I raised on the RFC version.
Reviewed-by: Arnd Bergmann <[email protected]>
On 2022/7/14 19:32, Arnd Bergmann wrote:
> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]> wrote:
>>
>> Currently arm64 supports per-CPU IRQ stack, but softirqs
>> are still handled in the task context.
>>
>> Since any call to local_bh_enable() at any level in the task's
>> call stack may trigger a softirq processing run, which could
>> potentially cause a task stack overflow if the combined stack
>> footprints exceed the stack's size, let's run these softirqs
>> on the IRQ stack as well.
>>
>> Signed-off-by: Qi Zheng <[email protected]>
>
> This seems like a nice improvement, and this version addresses
> my concern I raised on the RFC version.
>
> Reviewed-by: Arnd Bergmann <[email protected]>
Thanks for your review. :)
--
Thanks,
Qi
On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]> wrote:
>
> Since softirqs are handled on the per-CPU IRQ stack,
> let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
> the core code to invoke __do_softirq() directly without
> going through do_softirq_own_stack().
>
> Signed-off-by: Qi Zheng <[email protected]>
Adding Mark Rutland to Cc, he's the one that worked on this area the most in the
past and should probably review your patch. I still feel like there
should be a way
to improve readability of the IRQ entry path rather than just adding another
level of indirection, but the ideas I had so far have not led to
anything useful.
Overall I suppose your version is an improvement over the extra double stack
switch when entering softirq.
Arnd
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/exception.h | 4 +++-
> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
> arch/arm64/kernel/entry.S | 6 ++++--
> arch/arm64/kernel/irq.c | 5 +++--
> 5 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index be0a9f0052ee..d2cc7daecce3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -231,6 +231,7 @@ config ARM64
> select TRACE_IRQFLAGS_SUPPORT
> select TRACE_IRQFLAGS_NMI_SUPPORT
> select HAVE_SOFTIRQ_ON_OWN_STACK
> + select HAVE_IRQ_EXIT_ON_IRQ_STACK
> help
> ARM 64-bit (AArch64) Linux support.
>
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index d94aecff9690..8bff0aa7ab50 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs);
> asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
>
> asmlinkage void call_on_irq_stack(struct pt_regs *regs,
> - void (*func)(struct pt_regs *));
> + void (*func)(struct pt_regs *),
> + void (*do_func)(struct pt_regs *,
> + void (*)(struct pt_regs *)));
> asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
>
> void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index c75ca36b4a49..935d1ab150b5 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void)
> }
>
> static void do_interrupt_handler(struct pt_regs *regs,
> - void (*handler)(struct pt_regs *))
> + void (*handler)(struct pt_regs *),
> + void (*do_handler)(struct pt_regs *,
> + void (*)(struct pt_regs *)))
> {
> struct pt_regs *old_regs = set_irq_regs(regs);
>
> if (on_thread_stack())
> - call_on_irq_stack(regs, handler);
> + call_on_irq_stack(regs, handler, do_handler);
> else
> - handler(regs);
> + do_handler(regs, handler);
>
> set_irq_regs(old_regs);
> }
> @@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
> }
> }
>
> +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
> +{
> + handler(regs);
> +}
> +
> static __always_inline void __el1_pnmi(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> {
> arm64_enter_nmi(regs);
> - do_interrupt_handler(regs, handler);
> + do_interrupt_handler(regs, handler, nmi_handler);
> arm64_exit_nmi(regs);
> }
>
> +static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
> +{
> + irq_enter_rcu();
> + handler(regs);
> + irq_exit_rcu();
> +}
> +
> static __always_inline void __el1_irq(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();
> + do_interrupt_handler(regs, handler, irq_handler);
>
> arm64_preempt_schedule_irq();
>
> @@ -699,9 +711,7 @@ 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();
> + do_interrupt_handler(regs, handler, irq_handler);
>
> exit_to_user_mode(regs);
> }
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 254fe31c03a0..1c351391f6bd 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
>
> /*
> * void call_on_irq_stack(struct pt_regs *regs,
> - * void (*func)(struct pt_regs *));
> + * void (*func)(struct pt_regs *)
> + * void (*do_func)(struct pt_regs *,
> + * void (*)(struct pt_regs *)));
> *
> * Calls func(regs) using this CPU's irq stack and shadow irq stack.
> */
> @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
>
> /* Move to the new stack and call the function there */
> mov sp, x16
> - blr x1
> + blr x2
>
> /*
> * Restore the SP from the FP, and restore the FP and LR from the frame
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index c36ad20a52f3..003db605bc4f 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -73,14 +73,15 @@ static void init_irq_stacks(void)
> #endif
>
> #ifndef CONFIG_PREEMPT_RT
> -static void ____do_softirq(struct pt_regs *regs)
> +static void ____do_softirq(struct pt_regs *regs,
> + void (*handler)(struct pt_regs *))
> {
> __do_softirq();
> }
>
> void do_softirq_own_stack(void)
> {
> - call_on_irq_stack(NULL, ____do_softirq);
> + call_on_irq_stack(NULL, NULL, ____do_softirq);
> }
> #endif
>
> --
> 2.20.1
>
On 2022/7/14 19:37, Arnd Bergmann wrote:
> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]> wrote:
>>
>> Since softirqs are handled on the per-CPU IRQ stack,
>> let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
>> the core code to invoke __do_softirq() directly without
>> going through do_softirq_own_stack().
>>
>> Signed-off-by: Qi Zheng <[email protected]>
>
> Adding Mark Rutland to Cc, he's the one that worked on this area the most in the
> past and should probably review your patch. I still feel like there
> should be a way
> to improve readability of the IRQ entry path rather than just adding another
Got it. And looking forward to reviews and suggestions from Mark or
anyone else.
> level of indirection, but the ideas I had so far have not led to
> anything useful.
>
> Overall I suppose your version is an improvement over the extra double stack
> switch when entering softirq.
>
> Arnd
Thanks,
Qi
>
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/include/asm/exception.h | 4 +++-
>> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
>> arch/arm64/kernel/entry.S | 6 ++++--
>> arch/arm64/kernel/irq.c | 5 +++--
>> 5 files changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index be0a9f0052ee..d2cc7daecce3 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -231,6 +231,7 @@ config ARM64
>> select TRACE_IRQFLAGS_SUPPORT
>> select TRACE_IRQFLAGS_NMI_SUPPORT
>> select HAVE_SOFTIRQ_ON_OWN_STACK
>> + select HAVE_IRQ_EXIT_ON_IRQ_STACK
>> help
>> ARM 64-bit (AArch64) Linux support.
>>
>> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
>> index d94aecff9690..8bff0aa7ab50 100644
>> --- a/arch/arm64/include/asm/exception.h
>> +++ b/arch/arm64/include/asm/exception.h
>> @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs);
>> asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
>>
>> asmlinkage void call_on_irq_stack(struct pt_regs *regs,
>> - void (*func)(struct pt_regs *));
>> + void (*func)(struct pt_regs *),
>> + void (*do_func)(struct pt_regs *,
>> + void (*)(struct pt_regs *)));
>> asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
>>
>> void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>> index c75ca36b4a49..935d1ab150b5 100644
>> --- a/arch/arm64/kernel/entry-common.c
>> +++ b/arch/arm64/kernel/entry-common.c
>> @@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void)
>> }
>>
>> static void do_interrupt_handler(struct pt_regs *regs,
>> - void (*handler)(struct pt_regs *))
>> + void (*handler)(struct pt_regs *),
>> + void (*do_handler)(struct pt_regs *,
>> + void (*)(struct pt_regs *)))
>> {
>> struct pt_regs *old_regs = set_irq_regs(regs);
>>
>> if (on_thread_stack())
>> - call_on_irq_stack(regs, handler);
>> + call_on_irq_stack(regs, handler, do_handler);
>> else
>> - handler(regs);
>> + do_handler(regs, handler);
>>
>> set_irq_regs(old_regs);
>> }
>> @@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
>> }
>> }
>>
>> +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
>> +{
>> + handler(regs);
>> +}
>> +
>> static __always_inline void __el1_pnmi(struct pt_regs *regs,
>> void (*handler)(struct pt_regs *))
>> {
>> arm64_enter_nmi(regs);
>> - do_interrupt_handler(regs, handler);
>> + do_interrupt_handler(regs, handler, nmi_handler);
>> arm64_exit_nmi(regs);
>> }
>>
>> +static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
>> +{
>> + irq_enter_rcu();
>> + handler(regs);
>> + irq_exit_rcu();
>> +}
>> +
>> static __always_inline void __el1_irq(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();
>> + do_interrupt_handler(regs, handler, irq_handler);
>>
>> arm64_preempt_schedule_irq();
>>
>> @@ -699,9 +711,7 @@ 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();
>> + do_interrupt_handler(regs, handler, irq_handler);
>>
>> exit_to_user_mode(regs);
>> }
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 254fe31c03a0..1c351391f6bd 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
>>
>> /*
>> * void call_on_irq_stack(struct pt_regs *regs,
>> - * void (*func)(struct pt_regs *));
>> + * void (*func)(struct pt_regs *)
>> + * void (*do_func)(struct pt_regs *,
>> + * void (*)(struct pt_regs *)));
>> *
>> * Calls func(regs) using this CPU's irq stack and shadow irq stack.
>> */
>> @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
>>
>> /* Move to the new stack and call the function there */
>> mov sp, x16
>> - blr x1
>> + blr x2
>>
>> /*
>> * Restore the SP from the FP, and restore the FP and LR from the frame
>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>> index c36ad20a52f3..003db605bc4f 100644
>> --- a/arch/arm64/kernel/irq.c
>> +++ b/arch/arm64/kernel/irq.c
>> @@ -73,14 +73,15 @@ static void init_irq_stacks(void)
>> #endif
>>
>> #ifndef CONFIG_PREEMPT_RT
>> -static void ____do_softirq(struct pt_regs *regs)
>> +static void ____do_softirq(struct pt_regs *regs,
>> + void (*handler)(struct pt_regs *))
>> {
>> __do_softirq();
>> }
>>
>> void do_softirq_own_stack(void)
>> {
>> - call_on_irq_stack(NULL, ____do_softirq);
>> + call_on_irq_stack(NULL, NULL, ____do_softirq);
>> }
>> #endif
>>
>> --
>> 2.20.1
>>
--
Thanks,
Qi
On 2022/7/14 19:54, Qi Zheng wrote:
>
>
> On 2022/7/14 19:37, Arnd Bergmann wrote:
>> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]>
>> wrote:
>>>
>>> Since softirqs are handled on the per-CPU IRQ stack,
>>> let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
>>> the core code to invoke __do_softirq() directly without
>>> going through do_softirq_own_stack().
>>>
>>> Signed-off-by: Qi Zheng <[email protected]>
>>
>> Adding Mark Rutland to Cc, he's the one that worked on this area the
>> most in the
>> past and should probably review your patch. I still feel like there
>> should be a way
>> to improve readability of the IRQ entry path rather than just adding
>> another
>
> Got it. And looking forward to reviews and suggestions from Mark or
> anyone else.
Hi Will, Catalin and Mark,
I don't see any action items left for me. Any suggestions on this patch?
Thanks,
Qi
>
>> level of indirection, but the ideas I had so far have not led to
>> anything useful.
>>
>> Overall I suppose your version is an improvement over the extra double
>> stack
>> switch when entering softirq.
>>
>> Arnd
>
> Thanks,
> Qi
>
>>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/include/asm/exception.h | 4 +++-
>>> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
>>> arch/arm64/kernel/entry.S | 6 ++++--
>>> arch/arm64/kernel/irq.c | 5 +++--
>>> 5 files changed, 31 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index be0a9f0052ee..d2cc7daecce3 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -231,6 +231,7 @@ config ARM64
>>> select TRACE_IRQFLAGS_SUPPORT
>>> select TRACE_IRQFLAGS_NMI_SUPPORT
>>> select HAVE_SOFTIRQ_ON_OWN_STACK
>>> + select HAVE_IRQ_EXIT_ON_IRQ_STACK
>>> help
>>> ARM 64-bit (AArch64) Linux support.
>>>
>>> diff --git a/arch/arm64/include/asm/exception.h
>>> b/arch/arm64/include/asm/exception.h
>>> index d94aecff9690..8bff0aa7ab50 100644
>>> --- a/arch/arm64/include/asm/exception.h
>>> +++ b/arch/arm64/include/asm/exception.h
>>> @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs
>>> *regs);
>>> asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
>>>
>>> asmlinkage void call_on_irq_stack(struct pt_regs *regs,
>>> - void (*func)(struct pt_regs *));
>>> + void (*func)(struct pt_regs *),
>>> + void (*do_func)(struct pt_regs *,
>>> + void (*)(struct
>>> pt_regs *)));
>>> asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
>>>
>>> void do_mem_abort(unsigned long far, unsigned long esr, struct
>>> pt_regs *regs);
>>> diff --git a/arch/arm64/kernel/entry-common.c
>>> b/arch/arm64/kernel/entry-common.c
>>> index c75ca36b4a49..935d1ab150b5 100644
>>> --- a/arch/arm64/kernel/entry-common.c
>>> +++ b/arch/arm64/kernel/entry-common.c
>>> @@ -266,14 +266,16 @@ static void __sched
>>> arm64_preempt_schedule_irq(void)
>>> }
>>>
>>> static void do_interrupt_handler(struct pt_regs *regs,
>>> - void (*handler)(struct pt_regs *))
>>> + void (*handler)(struct pt_regs *),
>>> + void (*do_handler)(struct pt_regs *,
>>> + void (*)(struct
>>> pt_regs *)))
>>> {
>>> struct pt_regs *old_regs = set_irq_regs(regs);
>>>
>>> if (on_thread_stack())
>>> - call_on_irq_stack(regs, handler);
>>> + call_on_irq_stack(regs, handler, do_handler);
>>> else
>>> - handler(regs);
>>> + do_handler(regs, handler);
>>>
>>> set_irq_regs(old_regs);
>>> }
>>> @@ -441,22 +443,32 @@ asmlinkage void noinstr
>>> el1h_64_sync_handler(struct pt_regs *regs)
>>> }
>>> }
>>>
>>> +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct
>>> pt_regs *))
>>> +{
>>> + handler(regs);
>>> +}
>>> +
>>> static __always_inline void __el1_pnmi(struct pt_regs *regs,
>>> void (*handler)(struct
>>> pt_regs *))
>>> {
>>> arm64_enter_nmi(regs);
>>> - do_interrupt_handler(regs, handler);
>>> + do_interrupt_handler(regs, handler, nmi_handler);
>>> arm64_exit_nmi(regs);
>>> }
>>>
>>> +static void irq_handler(struct pt_regs *regs, void (*handler)(struct
>>> pt_regs *))
>>> +{
>>> + irq_enter_rcu();
>>> + handler(regs);
>>> + irq_exit_rcu();
>>> +}
>>> +
>>> static __always_inline void __el1_irq(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();
>>> + do_interrupt_handler(regs, handler, irq_handler);
>>>
>>> arm64_preempt_schedule_irq();
>>>
>>> @@ -699,9 +711,7 @@ 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();
>>> + do_interrupt_handler(regs, handler, irq_handler);
>>>
>>> exit_to_user_mode(regs);
>>> }
>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>> index 254fe31c03a0..1c351391f6bd 100644
>>> --- a/arch/arm64/kernel/entry.S
>>> +++ b/arch/arm64/kernel/entry.S
>>> @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
>>>
>>> /*
>>> * void call_on_irq_stack(struct pt_regs *regs,
>>> - * void (*func)(struct pt_regs *));
>>> + * void (*func)(struct pt_regs *)
>>> + * void (*do_func)(struct pt_regs *,
>>> + * void (*)(struct pt_regs *)));
>>> *
>>> * Calls func(regs) using this CPU's irq stack and shadow irq stack.
>>> */
>>> @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
>>>
>>> /* Move to the new stack and call the function there */
>>> mov sp, x16
>>> - blr x1
>>> + blr x2
>>>
>>> /*
>>> * Restore the SP from the FP, and restore the FP and LR
>>> from the frame
>>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>>> index c36ad20a52f3..003db605bc4f 100644
>>> --- a/arch/arm64/kernel/irq.c
>>> +++ b/arch/arm64/kernel/irq.c
>>> @@ -73,14 +73,15 @@ static void init_irq_stacks(void)
>>> #endif
>>>
>>> #ifndef CONFIG_PREEMPT_RT
>>> -static void ____do_softirq(struct pt_regs *regs)
>>> +static void ____do_softirq(struct pt_regs *regs,
>>> + void (*handler)(struct pt_regs *))
>>> {
>>> __do_softirq();
>>> }
>>>
>>> void do_softirq_own_stack(void)
>>> {
>>> - call_on_irq_stack(NULL, ____do_softirq);
>>> + call_on_irq_stack(NULL, NULL, ____do_softirq);
>>> }
>>> #endif
>>>
>>> --
>>> 2.20.1
>>>
>
--
Thanks,
Qi
On Thu, Jul 14, 2022 at 01:37:31PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]> wrote:
> >
> > Since softirqs are handled on the per-CPU IRQ stack,
> > let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
> > the core code to invoke __do_softirq() directly without
> > going through do_softirq_own_stack().
> >
> > Signed-off-by: Qi Zheng <[email protected]>
>
> Adding Mark Rutland to Cc, he's the one that worked on this area the most in the
> past and should probably review your patch. I still feel like there
> should be a way
> to improve readability of the IRQ entry path rather than just adding another
> level of indirection, but the ideas I had so far have not led to
> anything useful.
>
> Overall I suppose your version is an improvement over the extra double stack
> switch when entering softirq.
The cost of the switch is fairly trivial, so performance-wise I would expect
that to fall within the noise. So if this is just about avoiding the extra
switch, I'd prefer to leave this as-is.
If we want to move more work after the stack switch, I think we'd want a more
in depth rethink of the structure of this code (since e.g. we could make the
common case have almost everything on the IRQ stack, at the cost of making
preemption require more work).
Thanks,
Mark.
>
> Arnd
>
> > ---
> > arch/arm64/Kconfig | 1 +
> > arch/arm64/include/asm/exception.h | 4 +++-
> > arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
> > arch/arm64/kernel/entry.S | 6 ++++--
> > arch/arm64/kernel/irq.c | 5 +++--
> > 5 files changed, 31 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index be0a9f0052ee..d2cc7daecce3 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -231,6 +231,7 @@ config ARM64
> > select TRACE_IRQFLAGS_SUPPORT
> > select TRACE_IRQFLAGS_NMI_SUPPORT
> > select HAVE_SOFTIRQ_ON_OWN_STACK
> > + select HAVE_IRQ_EXIT_ON_IRQ_STACK
> > help
> > ARM 64-bit (AArch64) Linux support.
> >
> > diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> > index d94aecff9690..8bff0aa7ab50 100644
> > --- a/arch/arm64/include/asm/exception.h
> > +++ b/arch/arm64/include/asm/exception.h
> > @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs);
> > asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
> >
> > asmlinkage void call_on_irq_stack(struct pt_regs *regs,
> > - void (*func)(struct pt_regs *));
> > + void (*func)(struct pt_regs *),
> > + void (*do_func)(struct pt_regs *,
> > + void (*)(struct pt_regs *)));
> > asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
> >
> > void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
> > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> > index c75ca36b4a49..935d1ab150b5 100644
> > --- a/arch/arm64/kernel/entry-common.c
> > +++ b/arch/arm64/kernel/entry-common.c
> > @@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void)
> > }
> >
> > static void do_interrupt_handler(struct pt_regs *regs,
> > - void (*handler)(struct pt_regs *))
> > + void (*handler)(struct pt_regs *),
> > + void (*do_handler)(struct pt_regs *,
> > + void (*)(struct pt_regs *)))
> > {
> > struct pt_regs *old_regs = set_irq_regs(regs);
> >
> > if (on_thread_stack())
> > - call_on_irq_stack(regs, handler);
> > + call_on_irq_stack(regs, handler, do_handler);
> > else
> > - handler(regs);
> > + do_handler(regs, handler);
> >
> > set_irq_regs(old_regs);
> > }
> > @@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
> > }
> > }
> >
> > +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
> > +{
> > + handler(regs);
> > +}
> > +
> > static __always_inline void __el1_pnmi(struct pt_regs *regs,
> > void (*handler)(struct pt_regs *))
> > {
> > arm64_enter_nmi(regs);
> > - do_interrupt_handler(regs, handler);
> > + do_interrupt_handler(regs, handler, nmi_handler);
> > arm64_exit_nmi(regs);
> > }
> >
> > +static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
> > +{
> > + irq_enter_rcu();
> > + handler(regs);
> > + irq_exit_rcu();
> > +}
> > +
> > static __always_inline void __el1_irq(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();
> > + do_interrupt_handler(regs, handler, irq_handler);
> >
> > arm64_preempt_schedule_irq();
> >
> > @@ -699,9 +711,7 @@ 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();
> > + do_interrupt_handler(regs, handler, irq_handler);
> >
> > exit_to_user_mode(regs);
> > }
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index 254fe31c03a0..1c351391f6bd 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
> >
> > /*
> > * void call_on_irq_stack(struct pt_regs *regs,
> > - * void (*func)(struct pt_regs *));
> > + * void (*func)(struct pt_regs *)
> > + * void (*do_func)(struct pt_regs *,
> > + * void (*)(struct pt_regs *)));
> > *
> > * Calls func(regs) using this CPU's irq stack and shadow irq stack.
> > */
> > @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
> >
> > /* Move to the new stack and call the function there */
> > mov sp, x16
> > - blr x1
> > + blr x2
> >
> > /*
> > * Restore the SP from the FP, and restore the FP and LR from the frame
> > diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> > index c36ad20a52f3..003db605bc4f 100644
> > --- a/arch/arm64/kernel/irq.c
> > +++ b/arch/arm64/kernel/irq.c
> > @@ -73,14 +73,15 @@ static void init_irq_stacks(void)
> > #endif
> >
> > #ifndef CONFIG_PREEMPT_RT
> > -static void ____do_softirq(struct pt_regs *regs)
> > +static void ____do_softirq(struct pt_regs *regs,
> > + void (*handler)(struct pt_regs *))
> > {
> > __do_softirq();
> > }
> >
> > void do_softirq_own_stack(void)
> > {
> > - call_on_irq_stack(NULL, ____do_softirq);
> > + call_on_irq_stack(NULL, NULL, ____do_softirq);
> > }
> > #endif
> >
> > --
> > 2.20.1
> >
On 2022/7/19 19:24, Mark Rutland wrote:
> On Thu, Jul 14, 2022 at 01:37:31PM +0200, Arnd Bergmann wrote:
>> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <[email protected]> wrote:
>>>
>>> Since softirqs are handled on the per-CPU IRQ stack,
>>> let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
>>> the core code to invoke __do_softirq() directly without
>>> going through do_softirq_own_stack().
>>>
>>> Signed-off-by: Qi Zheng <[email protected]>
>>
>> Adding Mark Rutland to Cc, he's the one that worked on this area the most in the
>> past and should probably review your patch. I still feel like there
>> should be a way
>> to improve readability of the IRQ entry path rather than just adding another
>> level of indirection, but the ideas I had so far have not led to
>> anything useful.
>>
>> Overall I suppose your version is an improvement over the extra double stack
>> switch when entering softirq.
Hi Mark,
>
> The cost of the switch is fairly trivial, so performance-wise I would expect
> that to fall within the noise. So if this is just about avoiding the extra
> switch, I'd prefer to leave this as-is.
In this patch, I just want to avoid the extra switch.
>
> If we want to move more work after the stack switch, I think we'd want a more
> in depth rethink of the structure of this code (since e.g. we could make the
> common case have almost everything on the IRQ stack, at the cost of making
For now, I don't think it's necessary to put anything other than
irq_{enter|exit}_rcu() on the IRQ stack. For things that we want
to run on the IRQ stack in the future, we can also easily stuff
them into do_handler(). So I think the method of nesting pointers
is good for extensibility except that it is not easy to read. And
this method does not change much. :)
> preemption require more work).
Overall, I'm ok to drop this optimized patch if we don't care about
the performance overhead of extra switching.
Thanks,
Qi
>
> Thanks,
> Mark.
>
>>
>> Arnd
>>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/include/asm/exception.h | 4 +++-
>>> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
>>> arch/arm64/kernel/entry.S | 6 ++++--
>>> arch/arm64/kernel/irq.c | 5 +++--
>>> 5 files changed, 31 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index be0a9f0052ee..d2cc7daecce3 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -231,6 +231,7 @@ config ARM64
>>> select TRACE_IRQFLAGS_SUPPORT
>>> select TRACE_IRQFLAGS_NMI_SUPPORT
>>> select HAVE_SOFTIRQ_ON_OWN_STACK
>>> + select HAVE_IRQ_EXIT_ON_IRQ_STACK
>>> help
>>> ARM 64-bit (AArch64) Linux support.
>>>
>>> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
>>> index d94aecff9690..8bff0aa7ab50 100644
>>> --- a/arch/arm64/include/asm/exception.h
>>> +++ b/arch/arm64/include/asm/exception.h
>>> @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs);
>>> asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
>>>
>>> asmlinkage void call_on_irq_stack(struct pt_regs *regs,
>>> - void (*func)(struct pt_regs *));
>>> + void (*func)(struct pt_regs *),
>>> + void (*do_func)(struct pt_regs *,
>>> + void (*)(struct pt_regs *)));
>>> asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
>>>
>>> void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
>>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>>> index c75ca36b4a49..935d1ab150b5 100644
>>> --- a/arch/arm64/kernel/entry-common.c
>>> +++ b/arch/arm64/kernel/entry-common.c
>>> @@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void)
>>> }
>>>
>>> static void do_interrupt_handler(struct pt_regs *regs,
>>> - void (*handler)(struct pt_regs *))
>>> + void (*handler)(struct pt_regs *),
>>> + void (*do_handler)(struct pt_regs *,
>>> + void (*)(struct pt_regs *)))
>>> {
>>> struct pt_regs *old_regs = set_irq_regs(regs);
>>>
>>> if (on_thread_stack())
>>> - call_on_irq_stack(regs, handler);
>>> + call_on_irq_stack(regs, handler, do_handler);
>>> else
>>> - handler(regs);
>>> + do_handler(regs, handler);
>>>
>>> set_irq_regs(old_regs);
>>> }
>>> @@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
>>> }
>>> }
>>>
>>> +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
>>> +{
>>> + handler(regs);
>>> +}
>>> +
>>> static __always_inline void __el1_pnmi(struct pt_regs *regs,
>>> void (*handler)(struct pt_regs *))
>>> {
>>> arm64_enter_nmi(regs);
>>> - do_interrupt_handler(regs, handler);
>>> + do_interrupt_handler(regs, handler, nmi_handler);
>>> arm64_exit_nmi(regs);
>>> }
>>>
>>> +static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
>>> +{
>>> + irq_enter_rcu();
>>> + handler(regs);
>>> + irq_exit_rcu();
>>> +}
>>> +
>>> static __always_inline void __el1_irq(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();
>>> + do_interrupt_handler(regs, handler, irq_handler);
>>>
>>> arm64_preempt_schedule_irq();
>>>
>>> @@ -699,9 +711,7 @@ 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();
>>> + do_interrupt_handler(regs, handler, irq_handler);
>>>
>>> exit_to_user_mode(regs);
>>> }
>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>> index 254fe31c03a0..1c351391f6bd 100644
>>> --- a/arch/arm64/kernel/entry.S
>>> +++ b/arch/arm64/kernel/entry.S
>>> @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
>>>
>>> /*
>>> * void call_on_irq_stack(struct pt_regs *regs,
>>> - * void (*func)(struct pt_regs *));
>>> + * void (*func)(struct pt_regs *)
>>> + * void (*do_func)(struct pt_regs *,
>>> + * void (*)(struct pt_regs *)));
>>> *
>>> * Calls func(regs) using this CPU's irq stack and shadow irq stack.
>>> */
>>> @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
>>>
>>> /* Move to the new stack and call the function there */
>>> mov sp, x16
>>> - blr x1
>>> + blr x2
>>>
>>> /*
>>> * Restore the SP from the FP, and restore the FP and LR from the frame
>>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>>> index c36ad20a52f3..003db605bc4f 100644
>>> --- a/arch/arm64/kernel/irq.c
>>> +++ b/arch/arm64/kernel/irq.c
>>> @@ -73,14 +73,15 @@ static void init_irq_stacks(void)
>>> #endif
>>>
>>> #ifndef CONFIG_PREEMPT_RT
>>> -static void ____do_softirq(struct pt_regs *regs)
>>> +static void ____do_softirq(struct pt_regs *regs,
>>> + void (*handler)(struct pt_regs *))
>>> {
>>> __do_softirq();
>>> }
>>>
>>> void do_softirq_own_stack(void)
>>> {
>>> - call_on_irq_stack(NULL, ____do_softirq);
>>> + call_on_irq_stack(NULL, NULL, ____do_softirq);
>>> }
>>> #endif
>>>
>>> --
>>> 2.20.1
>>>
--
Thanks,
Qi
On Fri, Jul 08, 2022 at 05:49:49PM +0800, Qi Zheng wrote:
> Currently arm64 supports per-CPU IRQ stack, but softirqs
> are still handled in the task context.
>
> Since any call to local_bh_enable() at any level in the task's
> call stack may trigger a softirq processing run, which could
> potentially cause a task stack overflow if the combined stack
> footprints exceed the stack's size, let's run these softirqs
> on the IRQ stack as well.
>
> Signed-off-by: Qi Zheng <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/irq.c | 13 +++++++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4c1e1d2d2f8b..be0a9f0052ee 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -230,6 +230,7 @@ config ARM64
> select HAVE_ARCH_USERFAULTFD_MINOR if USERFAULTFD
> select TRACE_IRQFLAGS_SUPPORT
> select TRACE_IRQFLAGS_NMI_SUPPORT
> + select HAVE_SOFTIRQ_ON_OWN_STACK
> help
> ARM 64-bit (AArch64) Linux support.
>
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index bda49430c9ea..c36ad20a52f3 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -22,6 +22,7 @@
> #include <linux/vmalloc.h>
> #include <asm/daifflags.h>
> #include <asm/vmap_stack.h>
> +#include <asm/exception.h>
>
> /* Only access this in an NMI enter/exit */
> DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
> @@ -71,6 +72,18 @@ static void init_irq_stacks(void)
> }
> #endif
>
> +#ifndef CONFIG_PREEMPT_RT
> +static void ____do_softirq(struct pt_regs *regs)
> +{
> + __do_softirq();
> +}
> +
> +void do_softirq_own_stack(void)
> +{
> + call_on_irq_stack(NULL, ____do_softirq);
> +}
> +#endif
Acked-by: Will Deacon <[email protected]>
Please can you repost this at -rc1 and we can queue it up for 5.21?
Thanks,
Will
On 2022/7/22 17:04, Will Deacon wrote:
> On Fri, Jul 08, 2022 at 05:49:49PM +0800, Qi Zheng wrote:
>> Currently arm64 supports per-CPU IRQ stack, but softirqs
>> are still handled in the task context.
>>
>> Since any call to local_bh_enable() at any level in the task's
>> call stack may trigger a softirq processing run, which could
>> potentially cause a task stack overflow if the combined stack
>> footprints exceed the stack's size, let's run these softirqs
>> on the IRQ stack as well.
>>
>> Signed-off-by: Qi Zheng <[email protected]>
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/kernel/irq.c | 13 +++++++++++++
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 4c1e1d2d2f8b..be0a9f0052ee 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -230,6 +230,7 @@ config ARM64
>> select HAVE_ARCH_USERFAULTFD_MINOR if USERFAULTFD
>> select TRACE_IRQFLAGS_SUPPORT
>> select TRACE_IRQFLAGS_NMI_SUPPORT
>> + select HAVE_SOFTIRQ_ON_OWN_STACK
>> help
>> ARM 64-bit (AArch64) Linux support.
>>
>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>> index bda49430c9ea..c36ad20a52f3 100644
>> --- a/arch/arm64/kernel/irq.c
>> +++ b/arch/arm64/kernel/irq.c
>> @@ -22,6 +22,7 @@
>> #include <linux/vmalloc.h>
>> #include <asm/daifflags.h>
>> #include <asm/vmap_stack.h>
>> +#include <asm/exception.h>
>>
>> /* Only access this in an NMI enter/exit */
>> DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
>> @@ -71,6 +72,18 @@ static void init_irq_stacks(void)
>> }
>> #endif
>>
>> +#ifndef CONFIG_PREEMPT_RT
>> +static void ____do_softirq(struct pt_regs *regs)
>> +{
>> + __do_softirq();
>> +}
>> +
>> +void do_softirq_own_stack(void)
>> +{
>> + call_on_irq_stack(NULL, ____do_softirq);
>> +}
>> +#endif
>
> Acked-by: Will Deacon <[email protected]>
>
> Please can you repost this at -rc1 and we can queue it up for 5.21?
Sure, will do.
Thanks,
Qi
>
> Thanks,
>
> Will
--
Thanks,
Qi