Re: [PATCH 1/4] riscv: entry: Convert ret_from_fork() to C

[Charlie Jenkins <charlie@rivosinc.com>]

On Sat, Jan 25, 2025 at 11:30:09PM +0000, Maciej W. Rozycki wrote:
> On Wed, 22 Jan 2025, Charlie Jenkins wrote:
> 
> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> > index 33a5a9f2a0d4e1eeccfb3621b9e518b88e1b0704..9225c322279aa90e737b1d7144db084319cf8103 100644
> > --- a/arch/riscv/kernel/entry.S
> > +++ b/arch/riscv/kernel/entry.S
> > @@ -319,17 +319,14 @@ SYM_CODE_END(handle_kernel_stack_overflow)
> >  ASM_NOKPROBE(handle_kernel_stack_overflow)
> >  #endif
> >  
> > -SYM_CODE_START(ret_from_fork)
> > +SYM_CODE_START(ret_from_fork_asm)
> >  	call schedule_tail
> > -	beqz s0, 1f	/* not from kernel thread */
> > -	/* Call fn(arg) */
> > -	move a0, s1
> > -	jalr s0
> > -1:
> > -	move a0, sp /* pt_regs */
> > -	call syscall_exit_to_user_mode
> > +	move a0, s1 /* fn */
> > +	move a1, s0 /* fn_arg */
> > +	move a2, sp /* pt_regs */
> > +	call ret_from_fork
> >  	j ret_from_exception
> 
>  Wouldn't it make sense to save a jump here and make it a tail call, i.e.:
> 
> 	la ra, ret_from_exception
> 	tail ret_from_fork
> 

I don't believe so due to the return address stack. It was shown in this
patch [1] that a 7% performance improvement can be seen on existing
riscv hardware by performing the extra jump. Doing tail calls should be
avoided on riscv since the hardware can be expected to predict the
return address incorrectly every time if the return address is manually
changed.

- Charlie

> ?
> 
>   Maciej