Re: [RESEND PATCH v2 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()

[王贇]

On 2021/10/13 下午3:55, Miroslav Benes wrote:
>> diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
>> index a9f9c57..101e1fb 100644
>> --- a/include/linux/trace_recursion.h
>> +++ b/include/linux/trace_recursion.h
>> @@ -208,13 +208,29 @@ static __always_inline void trace_clear_recursion(int bit)
>>   * Use this for ftrace callbacks. This will detect if the function
>>   * tracing recursed in the same context (normal vs interrupt),
>>   *
>> + * The ftrace_test_recursion_trylock() will disable preemption,
>> + * which is required for the variant of synchronize_rcu() that is
>> + * used to allow patching functions where RCU is not watching.
>> + * See klp_synchronize_transition() for more details.
>> + *
> 
> I think that you misunderstood. Steven proposed to put the comment before 
> ftrace_test_recursion_trylock() call site in klp_ftrace_handler().

Oh, I see... thanks for pointing out :-)

> 
>>   * Returns: -1 if a recursion happened.
[snip]
>>  }
> 
> Side note... the comment will eventually conflict with peterz's 
> https://lore.kernel.org/all/20210929152429.125997206@xxxxxxxxxxxxx/.

Steven, would you like to share your opinion on this patch?

If klp_synchronize_transition() will be removed anyway, the comments
will be meaningless and we can just drop it :-P

Regards,
Michael Wang


> 
> Miroslav
>