From: Guo Hui guohui@uniontech.com
mainline inclusion from mainline-v6.5-rc1 commit 1da185fc8288fadb952e06881d0b75e924780103 category: performance bugzilla: https://gitee.com/openeuler/kernel/issues/IA5WFA
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
----------------------------------------------------------------------
The following code: static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, const syscall_fn_t syscall_table[]) { ...
if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) { local_daif_mask(); flags = read_thread_flags(); -------------------------------- A if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP)) return; local_daif_restore(DAIF_PROCCTX); }
trace_exit: syscall_trace_exit(regs); ------- B }
1. The flags in the if conditional statement should be used in the same way as the flags in the function syscall_trace_exit, because DAIF is not shielded in the function syscall_trace_exit, so when the flags are obtained in line A of the code, there is no need to shield DAIF. Don't care about the modification of flags after line A.
2. Masking DAIF caused syscall performance to deteriorate by about 10%. The Unixbench single core syscall performance data is as follows:
Machine: Kunpeng 920
Mask DAIF: System Call Overhead 1172314.1 lps (10.0 s, 7 samples)
System Benchmarks Partial Index BASELINE RESULT INDEX System Call Overhead 15000.0 1172314.1 781.5 ======== System Benchmarks Index Score (Partial Only) 781.5
Unmask DAIF: System Call Overhead 1287944.6 lps (10.0 s, 7 samples)
System Benchmarks Partial Index BASELINE RESULT INDEX System Call Overhead 15000.0 1287944.6 858.6 ======== System Benchmarks Index Score (Partial Only) 858.6
Rationale from Mark Rutland as for why this is safe:
This masking is an artifact of the old "ret_fast_syscall" assembly that was converted to C in commit:
f37099b6992a0b81 ("arm64: convert syscall trace logic to C")
The assembly would mask DAIF, check the thread flags, and fall through to kernel_exit without unmasking if no tracing was needed. The conversion copied this masking into the C version, though this wasn't strictly necessary.
As (in general) thread flags can be manipulated by other threads, it's not safe to manipulate the thread flags with plain reads and writes, and since commit:
342b3808786518ce ("arm64: Snapshot thread flags")
... we use read_thread_flags() to read the flags atomically.
With this, there is no need to mask DAIF transiently around reading the flags, as we only decide whether to trace while DAIF is masked, and the actual tracing occurs with DAIF unmasked. When el0_svc_common() returns its caller will unconditionally mask DAIF via exit_to_user_mode(), so the masking is redundant.
Signed-off-by: Guo Hui guohui@uniontech.com Reviewed-by: Mark Rutland mark.rutland@arm.com Link: https://lore.kernel.org/r/20230526024715.8773-1-guohui@uniontech.com [catalin.marinas@arm.com: updated comment with Mark's rationale] Signed-off-by: Catalin Marinas catalin.marinas@arm.com Signed-off-by: Zheng Zengkai zhengzengkai@huawei.com --- arch/arm64/kernel/syscall.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index d59471fd679c..9bd304568d90 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -176,11 +176,9 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, * exit regardless, as the old entry assembly did. */ if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) { - local_daif_mask(); flags = read_thread_flags(); if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP)) return; - local_daif_restore(DAIF_PROCCTX); }
trace_exit: