* [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack
@ 2022-07-08 9:49 Qi Zheng
2022-07-08 9:49 ` [PATCH v1 1/2] " Qi Zheng
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Qi Zheng @ 2022-07-08 9:49 UTC (permalink / raw)
To: arnd, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel, Qi Zheng
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/20220707110511.52129-1-zhengqi.arch@bytedance.com/
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v1 1/2] arm64: run softirqs on the per-CPU IRQ stack 2022-07-08 9:49 [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng @ 2022-07-08 9:49 ` Qi Zheng 2022-07-14 11:32 ` Arnd Bergmann 2022-07-22 9:04 ` Will Deacon 2022-07-08 9:49 ` [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK Qi Zheng 2022-07-12 6:57 ` [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng 2 siblings, 2 replies; 13+ messages in thread From: Qi Zheng @ 2022-07-08 9:49 UTC (permalink / raw) To: arnd, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel, Qi Zheng 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 <zhengqi.arch@bytedance.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/2] arm64: run softirqs on the per-CPU IRQ stack 2022-07-08 9:49 ` [PATCH v1 1/2] " Qi Zheng @ 2022-07-14 11:32 ` Arnd Bergmann 2022-07-14 11:50 ` Qi Zheng 2022-07-22 9:04 ` Will Deacon 1 sibling, 1 reply; 13+ messages in thread From: Arnd Bergmann @ 2022-07-14 11:32 UTC (permalink / raw) To: Qi Zheng Cc: Arnd Bergmann, Catalin Marinas, Will Deacon, Linux ARM, Linux Kernel Mailing List On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> This seems like a nice improvement, and this version addresses my concern I raised on the RFC version. Reviewed-by: Arnd Bergmann <arnd@arndb.de> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/2] arm64: run softirqs on the per-CPU IRQ stack 2022-07-14 11:32 ` Arnd Bergmann @ 2022-07-14 11:50 ` Qi Zheng 0 siblings, 0 replies; 13+ messages in thread From: Qi Zheng @ 2022-07-14 11:50 UTC (permalink / raw) To: Arnd Bergmann Cc: Catalin Marinas, Will Deacon, Linux ARM, Linux Kernel Mailing List On 2022/7/14 19:32, Arnd Bergmann wrote: > On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> > > This seems like a nice improvement, and this version addresses > my concern I raised on the RFC version. > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> Thanks for your review. :) -- Thanks, Qi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/2] arm64: run softirqs on the per-CPU IRQ stack 2022-07-08 9:49 ` [PATCH v1 1/2] " Qi Zheng 2022-07-14 11:32 ` Arnd Bergmann @ 2022-07-22 9:04 ` Will Deacon 2022-07-22 9:13 ` Qi Zheng 1 sibling, 1 reply; 13+ messages in thread From: Will Deacon @ 2022-07-22 9:04 UTC (permalink / raw) To: Qi Zheng; +Cc: arnd, catalin.marinas, linux-arm-kernel, linux-kernel 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 <zhengqi.arch@bytedance.com> > --- > 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 <will@kernel.org> Please can you repost this at -rc1 and we can queue it up for 5.21? Thanks, Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/2] arm64: run softirqs on the per-CPU IRQ stack 2022-07-22 9:04 ` Will Deacon @ 2022-07-22 9:13 ` Qi Zheng 0 siblings, 0 replies; 13+ messages in thread From: Qi Zheng @ 2022-07-22 9:13 UTC (permalink / raw) To: Will Deacon; +Cc: arnd, catalin.marinas, linux-arm-kernel, linux-kernel 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 <zhengqi.arch@bytedance.com> >> --- >> 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 <will@kernel.org> > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK 2022-07-08 9:49 [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng 2022-07-08 9:49 ` [PATCH v1 1/2] " Qi Zheng @ 2022-07-08 9:49 ` Qi Zheng 2022-07-14 11:37 ` Arnd Bergmann 2022-07-12 6:57 ` [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng 2 siblings, 1 reply; 13+ messages in thread From: Qi Zheng @ 2022-07-08 9:49 UTC (permalink / raw) To: arnd, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel, Qi Zheng 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 <zhengqi.arch@bytedance.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK 2022-07-08 9:49 ` [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK Qi Zheng @ 2022-07-14 11:37 ` Arnd Bergmann 2022-07-14 11:54 ` Qi Zheng 2022-07-19 11:24 ` Mark Rutland 0 siblings, 2 replies; 13+ messages in thread From: Arnd Bergmann @ 2022-07-14 11:37 UTC (permalink / raw) To: Qi Zheng Cc: Arnd Bergmann, Catalin Marinas, Will Deacon, Linux ARM, Linux Kernel Mailing List, Mark Rutland On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK 2022-07-14 11:37 ` Arnd Bergmann @ 2022-07-14 11:54 ` Qi Zheng 2022-07-19 7:01 ` Qi Zheng 2022-07-19 11:24 ` Mark Rutland 1 sibling, 1 reply; 13+ messages in thread From: Qi Zheng @ 2022-07-14 11:54 UTC (permalink / raw) To: Arnd Bergmann Cc: Catalin Marinas, Will Deacon, Linux ARM, Linux Kernel Mailing List, Mark Rutland On 2022/7/14 19:37, Arnd Bergmann wrote: > On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK 2022-07-14 11:54 ` Qi Zheng @ 2022-07-19 7:01 ` Qi Zheng 0 siblings, 0 replies; 13+ messages in thread From: Qi Zheng @ 2022-07-19 7:01 UTC (permalink / raw) To: Will Deacon, Catalin Marinas, Mark Rutland Cc: Arnd Bergmann, Linux ARM, Linux Kernel Mailing List 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 <zhengqi.arch@bytedance.com> >> 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 <zhengqi.arch@bytedance.com> >> >> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK 2022-07-14 11:37 ` Arnd Bergmann 2022-07-14 11:54 ` Qi Zheng @ 2022-07-19 11:24 ` Mark Rutland 2022-07-19 12:11 ` Qi Zheng 1 sibling, 1 reply; 13+ messages in thread From: Mark Rutland @ 2022-07-19 11:24 UTC (permalink / raw) To: Arnd Bergmann Cc: Qi Zheng, Catalin Marinas, Will Deacon, Linux ARM, Linux Kernel Mailing List On Thu, Jul 14, 2022 at 01:37:31PM +0200, Arnd Bergmann wrote: > On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> > > 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 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK 2022-07-19 11:24 ` Mark Rutland @ 2022-07-19 12:11 ` Qi Zheng 0 siblings, 0 replies; 13+ messages in thread From: Qi Zheng @ 2022-07-19 12:11 UTC (permalink / raw) To: Mark Rutland Cc: Arnd Bergmann, Catalin Marinas, Will Deacon, Linux ARM, Linux Kernel Mailing List 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 <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> >> >> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack 2022-07-08 9:49 [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng 2022-07-08 9:49 ` [PATCH v1 1/2] " Qi Zheng 2022-07-08 9:49 ` [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK Qi Zheng @ 2022-07-12 6:57 ` Qi Zheng 2 siblings, 0 replies; 13+ messages in thread From: Qi Zheng @ 2022-07-12 6:57 UTC (permalink / raw) To: arnd, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel 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/20220707110511.52129-1-zhengqi.arch@bytedance.com/ > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-07-22 9:16 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-08 9:49 [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng 2022-07-08 9:49 ` [PATCH v1 1/2] " Qi Zheng 2022-07-14 11:32 ` Arnd Bergmann 2022-07-14 11:50 ` Qi Zheng 2022-07-22 9:04 ` Will Deacon 2022-07-22 9:13 ` Qi Zheng 2022-07-08 9:49 ` [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK Qi Zheng 2022-07-14 11:37 ` Arnd Bergmann 2022-07-14 11:54 ` Qi Zheng 2022-07-19 7:01 ` Qi Zheng 2022-07-19 11:24 ` Mark Rutland 2022-07-19 12:11 ` Qi Zheng 2022-07-12 6:57 ` [PATCH v1 0/2] arm64: run softirqs on the per-CPU IRQ stack Qi Zheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox