| Message ID | 20220708094950.41944-3-zhengqi.arch@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: run softirqs on the per-CPU IRQ stack | expand |
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 >
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 >>
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 >>> >
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 > >
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 >>>
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
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(-)