Re: [patch 13/18] entry: Make trace_syscall_enter() return type bool

David Laight <[email protected]>
Newsgroups dev.linux.lists.loongarch,org.infradead.lists.linux-riscv,org.infradead.lists.linux-snps-arc,org.infradead.lists.linux-um,org.kernel.vger.linux-alpha,org.kernel.vger.linux-arch,org.kernel.vger.linux-csky,org.kernel.vger.linux-doc,org.kernel.vger.linux-hexagon,org.kernel.vger.linux-kernel,org.kernel.vger.linux-m68k,org.kernel.vger.linux-mips,org.kernel.vger.linux-openrisc,org.kernel.vger.linux-parisc,org.kernel.vger.linux-s390,org.kernel.vger.linux-sh,org.kernel.vger.sparclinux,org.ozlabs.lists.linuxppc-dev
Message-ID <20260709172659.7040db33@pumpkin>
On Thu, 09 Jul 2026 01:14:24 +0200
Thomas Gleixner <[email protected]> wrote:

> On Wed, Jul 08 2026 at 22:34, Thomas Gleixner wrote:
> > On Wed, Jul 08 2026 at 17:52, Michal Suchánek wrote:
> > Q: Is it perfect?
> > A: No
> >
> > Q: Can it be made perfect?
> > A: No, because you can't change history and established practice.
> >
> > Just for illustration. Changing the logic in trace_syscall_enter() to:
> >
> > --- a/kernel/entry/syscall-common.c
> > +++ b/kernel/entry/syscall-common.c
> > @@ -9,13 +9,15 @@
> >  
> >  bool trace_syscall_enter(struct pt_regs *regs, long *syscall)
> >  {
> > +	long orig_syscall = *syscall;
> > +
> >  	trace_sys_enter(regs, *syscall);
> >  	/*
> >  	 * Probes or BPF hooks in the tracepoint may have changed the
> >  	 * system call number. Reread it.
> >  	 */
> >  	*syscall = syscall_get_nr(current, regs);
> > -	return *syscall != -1L;
> > +	return *syscall == orig_syscall || *syscall != -1L;
> >  }
> >  
> >  void trace_syscall_exit(struct pt_regs *regs, long ret)
> >
> > does not make #2 magically go away. It's still the same problem whether
> > you like it or not.  
> 
> And just to be entirely clear, the syscall() interface has to be correct
> in the first place, but then it's all about performance.
> 
> So the sequence of:
> 
>    pt_regs = PUSH_REGS();
>    syscall = pt_regs->syscall_reg;
>    pt_regs->result = -ENOSYS;
> 
>    arch_syscall(pt_regs, syscall) {
>       if (likely(syscall_enter_from_user_mode(pt_regs, &syscall) {

I guess most architectures inline that to avoid the &syscall.
Otherwise you'd want:
	syscall = syscall_enter_from_user_mode(pt_regs, syscall);
with the 'error' return being selected to fail the test below
(which -1L converted to ~0UL will do nicely).

	David

>          if (syscall < SYSCALL_max)
>             pt_regs->result = invoke_syscall(pt_regs, syscall);
>       }
>       ,,,,
>    }
>    pt_regs->($RETURN_VALUE) = pt_regs->result;
>    POP_REGS();
>    return;
> 
> is the correct and obviuosly most efficient way idependent of the -1L
> return value overload in the original implementation, which this series
> gets rid of for clarity.
> 
> If an architecture decide[sd] to do otherwise and makes up it's own rules
> which only cover parts of the problem then it _is_ an architecture
> problem and not something which has to be solved by claiming that every
> architecture has to implement the same nonsense as you falsely claimed
> in your RFC^WPOC^Whack thread:
> 
>   "However, the API should be specified in a way that does not require
>    everyone implementing such flag."
> 
> There is _ZERO_ requirement for any architecture to implement that
> flag. Just because S390 decided it's a brilliant idea to do so does not
> make it a requirement for everyone.
> 
> No. Every other architecture got it right because they looked at the
> historical patterns despite having correct documentation at hand.
> 
> Feel free to prove me wrong with actual facts.
> 
> Thanks,
> 
>         tglx
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.