Re: [PATCH v7 08/11] entry: Add arch_ptrace_report_syscall_entry/exit()

[Kevin Brodsky <kevin.brodsky@arm.com>]

On 24/11/2025 10:34, Jinjie Ruan wrote:
>
> On 2025/11/19 1:13, Kevin Brodsky wrote:
>> On 17/11/2025 14:30, Jinjie Ruan wrote:
>>> [...]
>>>
>>> diff --git a/kernel/entry/syscall-common.c b/kernel/entry/syscall-common.c
>>> index 66e6ba7fa80c..27310e611567 100644
>>> --- a/kernel/entry/syscall-common.c
>>> +++ b/kernel/entry/syscall-common.c
>>> @@ -17,6 +17,25 @@ static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
>>>  	}
>>>  }
>>>  
>>> +/**
>>> + * arch_ptrace_report_syscall_entry - Architecture specific
>>> + *				      ptrace_report_syscall_entry().
>>> + *
>>> + * Invoked from syscall_trace_enter() to wrap ptrace_report_syscall_entry().
>>> + * Defaults to ptrace_report_syscall_entry.
>>> + *
>>> + * The main purpose is to support arch-specific ptrace_report_syscall_entry()
>>> + * implementation.
>>> + */
>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs);
>>> +
>>> +#ifndef arch_ptrace_report_syscall_entry
>>> +static __always_inline int arch_ptrace_report_syscall_entry(struct pt_regs *regs)
>>> +{
>>> +	return ptrace_report_syscall_entry(regs);
>> I saw that Thomas suggested this approach on v4, and it makes sense to
>> me, but I find the naming surprising. If an architecture does need extra
>> handling, then the generic function should never be called from generic
>> code. So it seems to me that the more logical change would be:
>>
>> * Rename: ptrace_report_syscall_entry -> __ptrace_report_syscall_entry
>> * Introduce ptrace_report_syscall_entry(), defaults to
>> __ptrace_report_syscall_entry()
> If ptrace_report_syscall_entry() is defined in linux/ptrace.h, and an
> architecture also needs to redefine this function, but the
> architecture's own <asm/entry-common.h> must include <linux/ptrace.h>,
> the function will end up being defined twice and cause a "duplicate
> definition" compile error.

There's plenty of arch-defined functions in <linux/ptrace.h> already.
__ptrace_report_syscall_entry() should be defined inside an #ifndef and
architectures can define their own implementation in <asm/ptrace.h>,
like force_successful_syscall_return() for instance.

- Kevin

>> All this would be done in <linux/ptrace.h>, where it clearly belongs.
>> The __ prefix makes it clear that the generic function is not the main
>> interface. Even better, no need to change any caller with that approach.
>>
>> - Kevin
>>
>>> [...]