On 03/18, Sudeep Holla wrote:
>
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -70,22 +70,16 @@ static long syscall_trace_enter(struct pt_regs *regs)
>
> struct thread_info *ti = current_thread_info();
> unsigned long ret = 0;
> - bool emulated = false;
> u32 work;
>
> if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
> BUG_ON(regs != task_pt_regs(current));
>
> - work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;
> -
> - if (unlikely(work & _TIF_SYSCALL_EMU))
> - emulated = true;
> -
> - if ((emulated || (work & _TIF_SYSCALL_TRACE)) &&
> - tracehook_report_syscall_entry(regs))
> + if (unlikely(ptrace_syscall_enter(regs)))
> return -1L;
>
> - if (emulated)
> + work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;
> + if ((work & _TIF_SYSCALL_TRACE) && tracehook_report_syscall_entry(regs))
> return -1L;
Well, I won't really argue, but to be honest I think this change doesn't make
the code better... With this patch tracehook_report_syscall_entry() has 2
callers,
to me this just adds some confusion.
I agree that the usage of emulated/_TIF_SYSCALL_EMU looks a bit overcomplicated,
I'd suggest a simple cleanup below.
And it seems that _TIF_WORK_SYSCALL_ENTRY needs some cleanups too... We don't
need
"& _TIF_WORK_SYSCALL_ENTRY" in syscall_trace_enter, and _TIF_WORK_SYSCALL_ENTRY
should not include _TIF_NOHZ?
Oleg.
--- x/arch/x86/entry/common.c
+++ x/arch/x86/entry/common.c
@@ -70,23 +70,18 @@ static long syscall_trace_enter(struct pt_regs *regs)
struct thread_info *ti = current_thread_info();
unsigned long ret = 0;
- bool emulated = false;
u32 work;
if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
BUG_ON(regs != task_pt_regs(current));
- work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;
+ work = READ_ONCE(ti->flags);
- if (unlikely(work & _TIF_SYSCALL_EMU))
- emulated = true;
-
- if ((emulated || (work & _TIF_SYSCALL_TRACE)) &&
- tracehook_report_syscall_entry(regs))
- return -1L;
-
- if (emulated)
- return -1L;
+ if (work & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
+ ret = tracehook_report_syscall_entry(regs);
+ if (ret || (work & _TIF_SYSCALL_EMU))
+ return -1L;
+ }
#ifdef CONFIG_SECCOMP
/*
