On Tue, 26 Oct 2021 17:48:10 +0800
王贇 <[email protected]> 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.
-- Steve
On 2021/10/26 下午8:01, Steven Rostedt wrote:
> On Tue, 26 Oct 2021 17:48:10 +0800
> 王贇 <[email protected]> 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
>
On Wed, 27 Oct 2021 09:54:13 +0800
王贇 <[email protected]> wrote:
> 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.
Actually, please resend both patches, as a new patch set, on its own thread.
Just replying here won't trigger my patchwork scripts.
And also, if you don't include the other patch, the scripts will drop
it.
Thanks,
-- Steve