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

王贇
2021-10-27T01:55:37Z

On 2021/10/26 下午8:01, Steven Rostedt wrote:
> On Tue, 26 Oct 2021 17:48:10 +0800
> 王贇 <yun.wang@linux.alibaba.com> 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
>