[PATCH v7 01/11] arm64/ptrace: Split report_syscall()

[Jinjie Ruan]

On 2025/11/19 1:09, Kevin Brodsky wrote:
> On 17/11/2025 14:30, Jinjie Ruan wrote:
>> The generic syscall entry code has the form:
>>
>> | syscall_trace_enter()
>> | {
>> |	ptrace_report_syscall_entry()
>> | }
>> |
>> | syscall_exit_work()
>> | {
>> |	ptrace_report_syscall_exit()
>> | }
>>
>> In preparation for moving arm64 over to the generic entry code, split
>> report_syscall() to two separate enter and exit functions to align
>> the structure of the arm64 code with syscall_trace_enter() and
>> syscall_exit_work() from the generic entry code.
>>
>> No functional changes.
>>
>> Suggested-by: Mark Rutland <mark.rutland at arm.com>
>> Signed-off-by: Jinjie Ruan <ruanjinjie at huawei.com>
>> ---
>>  arch/arm64/kernel/ptrace.c | 29 ++++++++++++++++++++---------
>>  1 file changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index 4b001121c72d..5534c175ceb7 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -2317,7 +2317,7 @@ enum ptrace_syscall_dir {
>>  	PTRACE_SYSCALL_EXIT,
>>  };
> 
> This is now unused so it should be removed.

Sure.

> 
>>  
>> -static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir)
>> +static void report_syscall_enter(struct pt_regs *regs)
> 
> Nit: better to call it "report_syscall_entry" to match its final name.

That makes sense, thanks for the review.

> 
>>  {
>>  	int regno;
>>  	unsigned long saved_reg;
>> @@ -2340,13 +2340,24 @@ static void report_syscall(struct pt_regs *regs, enum ptrace_syscall_dir dir)
>>  	 */
>>  	regno = (is_compat_task() ? 12 : 7);
>>  	saved_reg = regs->regs[regno];
>> -	regs->regs[regno] = dir;
>> +	regs->regs[regno] = PTRACE_SYSCALL_ENTER;
>>  
>> -	if (dir == PTRACE_SYSCALL_ENTER) {
>> -		if (ptrace_report_syscall_entry(regs))
>> -			forget_syscall(regs);
>> -		regs->regs[regno] = saved_reg;
>> -	} else if (!test_thread_flag(TIF_SINGLESTEP)) {
>> +	if (ptrace_report_syscall_entry(regs))
>> +		forget_syscall(regs);
>> +	regs->regs[regno] = saved_reg;
>> +}
>> +
>> +static void report_syscall_exit(struct pt_regs *regs)
>> +{
>> +	int regno;
>> +	unsigned long saved_reg;
>> +
>> +	/* See comment for report_syscall_enter() above */
>> +	regno = (is_compat_task() ? 12 : 7);
> 
> Probably best not to duplicate such magic numbers, moving this line to a
> helper would be good.

That makes sense.  A helper will remove the duplicate.

> 
> - Kevin
> 
>> +	saved_reg = regs->regs[regno];
>> +	regs->regs[regno] = PTRACE_SYSCALL_EXIT;
>> +
>> +	if (!test_thread_flag(TIF_SINGLESTEP)) {
>>  		ptrace_report_syscall_exit(regs, 0);
>>  		regs->regs[regno] = saved_reg;
>>  	} else {
>> @@ -2366,7 +2377,7 @@ int syscall_trace_enter(struct pt_regs *regs)
>>  	unsigned long flags = read_thread_flags();
>>  
>>  	if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
>> -		report_syscall(regs, PTRACE_SYSCALL_ENTER);
>> +		report_syscall_enter(regs);
>>  		if (flags & _TIF_SYSCALL_EMU)
>>  			return NO_SYSCALL;
>>  	}
>> @@ -2394,7 +2405,7 @@ void syscall_trace_exit(struct pt_regs *regs)
>>  		trace_sys_exit(regs, syscall_get_return_value(current, regs));
>>  
>>  	if (flags & (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP))
>> -		report_syscall(regs, PTRACE_SYSCALL_EXIT);
>> +		report_syscall_exit(regs);
>>  
>>  	rseq_syscall(regs);
>>  }
>