Messages in this thread Patch in this message | | | Subject | Re: [PATCH v5 1/2] ftrace: disable preemption when recursion locked | From | 王贇 <> | Date | Tue, 26 Oct 2021 17:48:10 +0800 |
| |
Hi, Miroslav
On 2021/10/26 下午5:35, Miroslav Benes wrote: > Hi, > >> diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h >> index abe1a50..2bc1522 100644 >> --- a/include/linux/trace_recursion.h >> +++ b/include/linux/trace_recursion.h >> @@ -135,6 +135,9 @@ static __always_inline int trace_get_context_bit(void) >> # define do_ftrace_record_recursion(ip, pip) do { } while (0) >> #endif >> >> +/* >> + * Preemption is promised to be disabled when return bit > 0. >> + */ >> static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsigned long pip, >> int start) >> { >> @@ -162,11 +165,17 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign >> current->trace_recursion = val; >> barrier(); >> >> + preempt_disable_notrace(); >> + >> return bit; >> } >> >> +/* >> + * Preemption will be enabled (if it was previously enabled). >> + */ >> static __always_inline void trace_clear_recursion(int bit) >> { >> + preempt_enable_notrace(); >> barrier(); >> trace_recursion_clear(bit); >> } > > 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?
Regards, Michael Wang
> > Regards > Miroslav >
| |