On 2025/12/9 21:48, Kevin Brodsky wrote:
> On 04/12/2025 09:21, Jinjie Ruan wrote:
>> After switch arm64 to Generic Entry, a new hotspot syscall_exit_work()
>> appeared because syscall_exit_work() is no longer inlined. so inline
>
> Before this series the call to syscall_trace_exit() in el0_svc_common()
> could not be inlined, so "no longer inlined" doesn't seem to be accurate.
I think the original "syscall_trace_exit()" is on an equal footing with
new introduced syscall_exit_to_user_mode_prepare() which is now inlined.
>
>> syscall_exit_work(), and it has 2.6% performance uplift on perf bench
>> basic syscall on kunpeng920 as below.
>
> That seems strange. syscall_exit_work() is only called if some flag in
> SYSCALL_WORK_EXIT is set, which means that we're doing something special
> like tracing. That shouldn't be the case when running a simple perf
> bench syscall.
>
> Also worth nothing that its counterpart (syscall_trace_enter())) is not
> currently inlined, the asymmetry would have to be justified.
Will check the "syscall_trace_enter" inline performance impact.
>
>> | Metric | W/O this patch | With this patch | Change |
>> | ---------- | -------------- | --------------- | --------- |
>> | Total time | 2.171 [sec] | 2.114 [sec] | ↓2.6% |
>> | usecs/op | 0.217192 | 0.211453 | ↓2.6% |
>> | ops/sec | 4,604,225 | 4,729,178 | ↑2.7% |
>>
>> Signed-off-by: Jinjie Ruan <[email protected]>
>> ---
>> include/linux/entry-common.h | 63 ++++++++++++++++++++++++++++++++++-
>> kernel/entry/syscall-common.c | 59 ++------------------------------
>
> These changes are purely generic, surely all architectures using
> GENERIC_ENTRY should get similar benefits (assuming LTO isn't used)?
It would be great if someone could help test the performance differences.
>
>> 2 files changed, 64 insertions(+), 58 deletions(-)
>>
>> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
>> index cd6dacb2d8bf..2f84377fb016 100644
>> --- a/include/linux/entry-common.h
>> +++ b/include/linux/entry-common.h
>> @@ -2,6 +2,7 @@
>> #ifndef __LINUX_ENTRYCOMMON_H
>> #define __LINUX_ENTRYCOMMON_H
>>
>> +#include <linux/audit.h>
>> #include <linux/irq-entry-common.h>
>> #include <linux/ptrace.h>
>> #include <linux/seccomp.h>
>> @@ -128,6 +129,41 @@ static __always_inline long
>> syscall_enter_from_user_mode(struct pt_regs *regs, l
>> return ret;
>> }
>>
>> +/*
>> + * If SYSCALL_EMU is set, then the only reason to report is when
>> + * SINGLESTEP is set (i.e. PTRACE_SYSEMU_SINGLESTEP). This syscall
>> + * instruction has been already reported in syscall_enter_from_user_mode().
>> + */
>> +static __always_inline bool report_single_step(unsigned long work)
>> +{
>> + if (work & SYSCALL_WORK_SYSCALL_EMU)
>> + return false;
>> +
>> + return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
>> +}
>> +
>> +/**
>> + * arch_ptrace_report_syscall_exit - Architecture specific
>> + * ptrace_report_syscall_exit.
>> + *
>> + * Invoked from syscall_exit_work() to wrap ptrace_report_syscall_exit().
>> + *
>> + * The main purpose is to support arch-specific ptrace_report_syscall_exit
>> + * implementation.
>> + */
>> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs
>> *regs,
>> + int step);
>> +
>> +#ifndef arch_ptrace_report_syscall_exit
>> +static __always_inline void arch_ptrace_report_syscall_exit(struct pt_regs
>> *regs,
>> + int step)
>> +{
>> + ptrace_report_syscall_exit(regs, step);
>> +}
>> +#endif
>
> If we want syscall_exit_work() to be inline, then why would we define
> this hook in syscall-common.c in patch 12? Might as well define both
> hooks in entry-common.h right away and avoid some noise here.
Make sense.
>
> - Kevin
>
>> [...]
>
