| Message ID | 20160420162354.GB3539@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 20, 2016 at 09:23:54AM -0700, Paul E. McKenney wrote: [...] > > > Maybe RCU_NONIDLE() will help here? > > > > Thanks for chiming in. > > > > CPU_PM notifiers are called from process context (which is not necessarily > > the idle thread) with IRQs disabled from: > > > > - CPUidle drivers state enter calls > > - syscore callbacks (ie suspend2RAM - suspend thread) > > - bL switcher > > - MCPM loopback > > > > The questions I have are: > > > > - Is it safe to wrap a call (in this case armpmu_start()) with RCU_NONIDLE > > if the core is not actually executing the idle thread ? The function > > requiring rcu locks/dereferences is perf_event_update_userpage(). > > Yes it is. > > > - What are RCU_NONIDLE side-effects (ie what can be actually called from > > within an RCU_NONIDLE wrapper ?) > > There are a few restrictions: > > 1. Code within RCU_NONIDLE() cannot block. Then again, neither > can the idle task. ;-) > > 2. RCU_NONIDLE() can be nested, but not indefinitely. Then again, > given that the limit even on a 32-bit system is something like > a million, I bet you hit compiler or stack-size limits long > before you overflow RCU_NONIDLE()'s counter. > > 3. You can neither branch into the middle of RCU_NONIDLE()'s code > nor branch out from the middle of RCU_NONIDLE()'s code. > Calling functions is just fine, but things like this are not: > > RCU_NONIDLE({ > do_something(); > goto bad_idea; /* BUG!!! */ > do_something_else();}); > do_yet_a_third_thing(); > bad_idea: > > Branching -within- the RCU_NONIDLE() code is just fine. > > Yes, and I am adding this information to RCU_NONIDLE()'s header > comment, apologies for its not being there to begin with! > (See below for patches.) Thank you for the explanation, that's now clear. For my own understanding: RCU_NONIDLE() is a way to inform the RCU subsystem that the CPU in question should be temporarily *watched* (ie it is not idle from an RCU standpoint), correct ? > > It would be nice if we can use it instead of merging this patch, I need > > more insights into RCU_NONIDLE usage though before proceeding. > > Please let me know if any of the above restrictions cause you a problem. I can't think of any, perf_event_update_userpage(), that I will call indirectly through: RCU_NONIDLE(armpmu_start()); is not allowed to block anyway, so I think we have a much better solution than this one, new patch coming, Catalin please drop this one. Thank you all ! Lorenzo
On Wed, Apr 20, 2016 at 05:52:05PM +0100, Lorenzo Pieralisi wrote: > On Wed, Apr 20, 2016 at 09:23:54AM -0700, Paul E. McKenney wrote: > > [...] > > > > > Maybe RCU_NONIDLE() will help here? > > > > > > Thanks for chiming in. > > > > > > CPU_PM notifiers are called from process context (which is not necessarily > > > the idle thread) with IRQs disabled from: > > > > > > - CPUidle drivers state enter calls > > > - syscore callbacks (ie suspend2RAM - suspend thread) > > > - bL switcher > > > - MCPM loopback > > > > > > The questions I have are: > > > > > > - Is it safe to wrap a call (in this case armpmu_start()) with RCU_NONIDLE > > > if the core is not actually executing the idle thread ? The function > > > requiring rcu locks/dereferences is perf_event_update_userpage(). > > > > Yes it is. > > > > > - What are RCU_NONIDLE side-effects (ie what can be actually called from > > > within an RCU_NONIDLE wrapper ?) > > > > There are a few restrictions: > > > > 1. Code within RCU_NONIDLE() cannot block. Then again, neither > > can the idle task. ;-) > > > > 2. RCU_NONIDLE() can be nested, but not indefinitely. Then again, > > given that the limit even on a 32-bit system is something like > > a million, I bet you hit compiler or stack-size limits long > > before you overflow RCU_NONIDLE()'s counter. > > > > 3. You can neither branch into the middle of RCU_NONIDLE()'s code > > nor branch out from the middle of RCU_NONIDLE()'s code. > > Calling functions is just fine, but things like this are not: > > > > RCU_NONIDLE({ > > do_something(); > > goto bad_idea; /* BUG!!! */ > > do_something_else();}); > > do_yet_a_third_thing(); > > bad_idea: > > > > Branching -within- the RCU_NONIDLE() code is just fine. > > > > Yes, and I am adding this information to RCU_NONIDLE()'s header > > comment, apologies for its not being there to begin with! > > (See below for patches.) > > Thank you for the explanation, that's now clear. For my own understanding: > RCU_NONIDLE() is a way to inform the RCU subsystem that the CPU in question > should be temporarily *watched* (ie it is not idle from an RCU standpoint), > correct ? Exactly! > > > It would be nice if we can use it instead of merging this patch, I need > > > more insights into RCU_NONIDLE usage though before proceeding. > > > > Please let me know if any of the above restrictions cause you a problem. > > I can't think of any, perf_event_update_userpage(), that I will call > indirectly through: > > RCU_NONIDLE(armpmu_start()); > > is not allowed to block anyway, so I think we have a much better > solution than this one, new patch coming, Catalin please drop this one. > > Thank you all ! Please let me know how it goes! Thanx, Paul
diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html index e7e24b3e86e2..ece410f40436 100644 --- a/Documentation/RCU/Design/Requirements/Requirements.html +++ b/Documentation/RCU/Design/Requirements/Requirements.html @@ -2391,6 +2391,41 @@ and <tt>RCU_NONIDLE()</tt> on the other while inspecting idle-loop code. Steven Rostedt supplied <tt>_rcuidle</tt> event tracing, which is used quite heavily in the idle loop. +However, there are some restrictions on the code placed within +<tt>RCU_NONIDLE()</tt>: + +<ol> +<li> Blocking is prohibited. + In practice, this is not a serious restriction given that idle + tasks are prohibited from blocking to begin with. +<li> Although nesting <tt>RCU_NONIDLE()</tt> is permited, they cannot + nest indefinitely deeply. + However, given that they can be nested on the order of a million + deep, even on 32-bit systems, this should not be a serious + restriction. + This nesting limit would probably be reached long after the + compiler OOMed or the stack overflowed. +<li> Any code path that enters <tt>RCU_NONIDLE()</tt> must sequence + out of that same <tt>RCU_NONIDLE()</tt>. + For example, the following is grossly illegal: + + <blockquote> + <pre> + 1 RCU_NONIDLE({ + 2 do_something(); + 3 goto bad_idea; /* BUG!!! */ + 4 do_something_else();}); + 5 bad_idea: + </pre> + </blockquote> + + <p> + It is just as illegal to transfer control into the middle of + <tt>RCU_NONIDLE()</tt>'s argument. + Yes, in theory, you could transfer in as long as you also + transferred out, but in practice you could also expect to get sharply + worded review comments. +</ol> <p> It is similarly socially unacceptable to interrupt an diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 5f1533e3d032..c61b6b9506e7 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -379,12 +379,13 @@ static inline void rcu_init_nohz(void) * in the inner idle loop. * * This macro provides the way out: RCU_NONIDLE(do_something_with_RCU()) - * will tell RCU that it needs to pay attending, invoke its argument - * (in this example, a call to the do_something_with_RCU() function), + * will tell RCU that it needs to pay attention, invoke its argument + * (in this example, calling the do_something_with_RCU() function), * and then tell RCU to go back to ignoring this CPU. It is permissible - * to nest RCU_NONIDLE() wrappers, but the nesting level is currently - * quite limited. If deeper nesting is required, it will be necessary - * to adjust DYNTICK_TASK_NESTING_VALUE accordingly. + * to nest RCU_NONIDLE() wrappers, but not indefinitely (but the limit is + * on the order of a million or so, even on 32-bit systems). It is + * not legal to block within RCU_NONIDLE(), nor is it permissible to + * transfer control either into or out of RCU_NONIDLE()'s statement. */ #define RCU_NONIDLE(a) \ do { \