On Mon, 22 May 2023 23:59:28 -0700 "Paul E. McKenney" <paulmck@xxxxxxxxxx> wrote: > On Tue, May 23, 2023 at 01:30:19PM +0800, Masami Hiramatsu wrote: > > On Mon, 22 May 2023 10:07:42 +0800 > > Ze Gao <zegao2021@xxxxxxxxx> wrote: > > > > > Oops, I missed that. Thanks for pointing that out, which I thought is > > > conditional use of rcu_is_watching before. > > > > > > One last point, I think we should double check on this > > > "fentry does not filter with !rcu_is_watching" > > > as quoted from Yonghong and argue whether it needs > > > the same check for fentry as well. > > > > rcu_is_watching() comment says; > > > > * if the current CPU is not in its idle loop or is in an interrupt or > > * NMI handler, return true. > > > > Thus it returns *fault* if the current CPU is in the idle loop and not > > any interrupt(including NMI) context. This means if any tracable function > > is called from idle loop, it can be !rcu_is_watching(). I meant, this is > > 'context' based check, thus fentry can not filter out that some commonly > > used functions is called from that context but it can be detected. > > It really does return false (rather than faulting?) if the current CPU > is deep within the idle loop. > > In addition, the recent x86/entry rework (thank you Peter and > Thomas!) mean that the "idle loop" is quite restricted, as can be > seen by the invocations of ct_cpuidle_enter() and ct_cpuidle_exit(). > For example, in default_idle_call(), these are immediately before and > after the call to arch_cpu_idle(). Thanks! I also found that the default_idle_call() is enough small and it seems not happening on fentry because there are no commonly used functions on that path. > > Would the following help? Or am I missing your point? Yes, thank you for the update! > > Thanx, Paul > > ------------------------------------------------------------------------ > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 1449cb69a0e0..fae9b4e29c93 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -679,10 +679,14 @@ static void rcu_disable_urgency_upon_qs(struct rcu_data *rdp) > /** > * rcu_is_watching - see if RCU thinks that the current CPU is not idle > * > - * Return true if RCU is watching the running CPU, which means that this > - * CPU can safely enter RCU read-side critical sections. In other words, > - * if the current CPU is not in its idle loop or is in an interrupt or > - * NMI handler, return true. > + * Return @true if RCU is watching the running CPU and @false otherwise. > + * An @true return means that this CPU can safely enter RCU read-side > + * critical sections. > + * > + * More specifically, if the current CPU is not deep within its idle > + * loop, return @true. Note that rcu_is_watching() will return @true if > + * invoked from an interrupt or NMI handler, even if that interrupt or > + * NMI interrupted the CPU while it was deep within its idle loop. > * > * Make notrace because it can be called by the internal functions of > * ftrace, and making this notrace removes unnecessary recursion calls. -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
