Re: [PATCH v5 1/2] ftrace: disable preemption when recursion locked

[王贇]

On 2021/10/26 下午8:01, Steven Rostedt wrote:
> On Tue, 26 Oct 2021 17:48:10 +0800
> 王贇 <yun.wang@xxxxxxxxxxxxxxxxx> wrote:
> 
>>> The two comments should be updated too since Steven removed the "bit == 0" 
>>> trick.  
>>
>> Could you please give more hint on how will it be correct?
>>
>> I get the point that bit will no longer be 0, there are only -1 or > 0 now
>> so trace_test_and_set_recursion() will disable preemption on bit > 0 and
>> trace_clear_recursion() will enabled it since it should only be called when
>> bit > 0 (I remember we could use a WARN_ON here now :-P).
>>
>>>   
>>>> @@ -178,7 +187,7 @@ static __always_inline void trace_clear_recursion(int bit)
>>>>   * tracing recursed in the same context (normal vs interrupt),
>>>>   *
>>>>   * Returns: -1 if a recursion happened.
>>>> - *           >= 0 if no recursion
>>>> + *           > 0 if no recursion.
>>>>   */
>>>>  static __always_inline int ftrace_test_recursion_trylock(unsigned long ip,
>>>>  							 unsigned long parent_ip)  
>>>
>>> And this change would not be correct now.  
>>
>> I thought it will no longer return 0 so I change it to > 0, isn't that correct?
> 
> No it is not. I removed the bit + 1 return value, which means it returns the
> actual bit now. Which is 0 or more.

Ah, the return is bit not val, I must be drunk...

My apologize for the stupid comments... I'll send a v6 for this patch
only to fix that, please let me know if this is not a good way to fix
few lines of comments.

Regards,
Michael Wang

> 
> -- Steve
>