Re: [RFC PATCH bpf-next v4 1/4] bpf, x64: Comment tail_call_cnt initialisation

[Leon Hwang <hffilwlqm@gmail.com>]

On 6/9/23 03:26, Maciej Fijalkowski wrote:
> On Sun, Sep 03, 2023 at 11:14:45PM +0800, Leon Hwang wrote:
>> Without understanding emit_prologue(), it is really hard to figure out
>> where does tail_call_cnt come from, even though searching tail_call_cnt
>> in the whole kernel repo.
>>
>> By adding these comments, it is a little bit easy to understand
> 
> s/easy/easier

Got it.

> 
>> tail_call_cnt initialisation.
>>
>> Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
> 
> I was rather thinking about dropping these comments altogether. We should
> be trying to provide fixes as small as possible IMHO.
> 
> Having this as a separate commit also breaks logistics of this set as
> the fix + selftest should be routed towards bpf tree, whereas this
> particular commit is rather a bpf-next candidate.
> 
> PTAL at https://www.kernel.org/doc/html/latest/bpf/bpf_devel_QA.html

Thanks for your review.

I'll keep this commit as one of this set.

Thanks,
Leon

> 
>> ---
>>  arch/x86/net/bpf_jit_comp.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index a5930042139d3..bcca1c9b9a027 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -303,8 +303,12 @@ static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf,
>>  	prog += X86_PATCH_SIZE;
>>  	if (!ebpf_from_cbpf) {
>>  		if (tail_call_reachable && !is_subprog)
>> +			/* When it's the entry of the whole tailcall context,
>> +			 * zeroing rax means initialising tail_call_cnt.
>> +			 */
>>  			EMIT2(0x31, 0xC0); /* xor eax, eax */
>>  		else
>> +			/* Keep the same instruction layout. */
>>  			EMIT2(0x66, 0x90); /* nop2 */
>>  	}
>>  	EMIT1(0x55);             /* push rbp */
>> -- 
>> 2.41.0
>>