Re: [PATCH v2 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook

[Sudeep Holla]

On Mon, Mar 18, 2019 at 04:33:22PM +0100, Oleg Nesterov wrote:
> 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;
>
[...]

>
> 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?
>

I was about to post the updated version and checked this to make sure I have
covered everything or not. I had missed the above comment. All architectures
have _TIF_NOHZ in their mask that they check to do work. And from x86, I read
"...syscall_trace_enter(). Also includes TIF_NOHZ for enter_from_user_mode()"
So I don't understand why _TIF_NOHZ needs to be dropped.

Also if we need to drop, we can address that separately examining all archs.
I will post the cleanup as you suggested for now.

--
Regards,
Sudeep