On Mon, Apr 25, 2016 at 10:12:39AM -0700, Paul E. McKenney wrote:
> On Sun, Apr 24, 2016 at 11:26:41PM -0700, Guenter Roeck wrote:
> > On 04/24/2016 10:49 PM, Paul E. McKenney wrote:
> > >On Sun, Apr 24, 2016 at 10:37:25PM -0700, Guenter Roeck wrote:
> > >>On 04/24/2016 10:28 PM, Paul E. McKenney wrote:
> > >>>On Sun, Apr 24, 2016 at 04:56:38PM -0700, Guenter Roeck wrote:
> 
> [ . . . ]
> 
> > >>>>After making the same change in _pwrdm_state_switch(), the traceback is 
> > >>>>gone
> > >>>>from my tests (beagle, beagle-xm, and overo-tobi).
> > >>>
> > >>>Very good!
> > >>>
> > >>>(And yes, you normally find these one at a time...)
> > >>>
> > >>Are you going to submit a formal patch ?
> > >
> > >I can, but please feel free to send mine along with yours, if you wish.
> > >
> > I think it would be best if you send a single patch which fixes both calls.
> 
> Like this one?
> 
> If so, could you please run it to make sure that it actually fixes the
> problem?  And if it does, would you be willing to give me a Tested-by?
> 
It does. Tested-by: inline below.

Thanks,
Guenter

>                                                       Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> commit e3c9e1a46984b8c3e4204f138aced7630c4b85c2
> Author: Paul E. McKenney <paul...@linux.vnet.ibm.com>
> Date:   Sun Apr 24 14:30:16 2016 -0700
> 
>     arm: Use _rcuidle tracepoint to allow use from idle
>     
>     Testing on ARM encountered the following pair of lockdep-RCU splats:
>     
>     ------------------------------------------------------------------------
>     
>     ===============================
>     [ INFO: suspicious RCU usage. ]
>     4.6.0-rc4-next-20160422 #1 Not tainted
>     -------------------------------
>     include/trace/events/power.h:328 suspicious rcu_dereference_check() usage!
>     
>     other info that might help us debug this:
>     
>     RCU used illegally from idle CPU!
>     rcu_scheduler_active = 1, debug_locks = 0
>     RCU used illegally from extended quiescent state!
>     no locks held by swapper/0/0.
>     
>     stack backtrace:
>     CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.6.0-rc4-next-20160422 #1
>     Hardware name: Generic OMAP3-GP (Flattened Device Tree)
>     [<c010f55c>] (unwind_backtrace) from [<c010b64c>] (show_stack+0x10/0x14)
>     [<c010b64c>] (show_stack) from [<c047acbc>] (dump_stack+0xa8/0xe0)
>     [<c047acbc>] (dump_stack) from [<c012bc10>] 
> (pwrdm_set_next_pwrst+0xf8/0x1cc)
>     [<c012bc10>] (pwrdm_set_next_pwrst) from [<c01269fc>] 
> (omap3_enter_idle_bm+0x1b8/0x1e8)
>     [<c01269fc>] (omap3_enter_idle_bm) from [<c05fa0b8>] 
> (cpuidle_enter_state+0x84/0x408)
>     [<c05fa0b8>] (cpuidle_enter_state) from [<c0182c1c>] 
> (cpu_startup_entry+0x1c8/0x3f0)
>     [<c0182c1c>] (cpu_startup_entry) from [<c0b00c20>] 
> (start_kernel+0x354/0x3cc)
>     
>     ------------------------------------------------------------------------
>     
>     [<c010f55c>] (unwind_backtrace) from [<c010b64c>] (show_stack+0x10/0x14)
>     [<c010b64c>] (show_stack) from [<c047ac3c>] (dump_stack+0xa8/0xe0)
>     [<c047ac3c>] (dump_stack) from [<c012c340>] 
> (_pwrdm_state_switch+0x188/0x32c)
>     [<c012c340>] (_pwrdm_state_switch) from [<c012c4f0>] 
> (_pwrdm_post_transition_cb+0xc/0x14)
>     [<c012c4f0>] (_pwrdm_post_transition_cb) from [<c012ba74>] 
> (pwrdm_for_each+0x30/0x5c)
>     [<c012ba74>] (pwrdm_for_each) from [<c012c72c>] 
> (pwrdm_post_transition+0x24/0x30)
>     [<c012c72c>] (pwrdm_post_transition) from [<c012548c>] 
> (omap_sram_idle+0xfc/0x240)
>     [<c012548c>] (omap_sram_idle) from [<c0126934>] 
> (omap3_enter_idle_bm+0xf0/0x1e8)
>     [<c0126934>] (omap3_enter_idle_bm) from [<c05fa038>] 
> (cpuidle_enter_state+0x84/0x408)
>     [<c05fa038>] (cpuidle_enter_state) from [<c0182b90>] 
> (cpu_startup_entry+0x1c8/0x3f0)
>     [<c0182b90>] (cpu_startup_entry) from [<c0b00c20>] 
> (start_kernel+0x354/0x3cc)
>     
>     ------------------------------------------------------------------------
>     
>     These are caused by event tracing from the idle loop.  This commit
>     therefore adds the _rcuidle suffix to make RCU aware of this implicit
>     use of RCU by event tracing, thus preventing both splats.
>     
>     Reported-by: Guenter Roeck <li...@roeck-us.net>

Tested-by: Guenter Roeck <li...@roeck-us.net>

>     Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
>     Cc: Tony Lindgren <t...@atomide.com>
>     Cc: Russell King <li...@arm.linux.org.uk>
>     Cc: Steven Rostedt <rost...@goodmis.org>
>     Cc: <linux-o...@vger.kernel.org>
>     Cc: <linux-arm-ker...@lists.infradead.org>
> 
> diff --git a/arch/arm/mach-omap2/powerdomain.c 
> b/arch/arm/mach-omap2/powerdomain.c
> index 78af6d8cf2e2..daf2753de7aa 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -186,8 +186,9 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, 
> int flag)
>                       trace_state = (PWRDM_TRACE_STATES_FLAG |
>                                      ((next & OMAP_POWERSTATE_MASK) << 8) |
>                                      ((prev & OMAP_POWERSTATE_MASK) << 0));
> -                     trace_power_domain_target(pwrdm->name, trace_state,
> -                                               smp_processor_id());
> +                     trace_power_domain_target_rcuidle(pwrdm->name,
> +                                                       trace_state,
> +                                                       smp_processor_id());
>               }
>               break;
>       default:
> @@ -523,8 +524,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 
> pwrst)
>  
>       if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
>               /* Trace the pwrdm desired target state */
> -             trace_power_domain_target(pwrdm->name, pwrst,
> -                                       smp_processor_id());
> +             trace_power_domain_target_rcuidle(pwrdm->name, pwrst,
> +                                               smp_processor_id());
>               /* Program the pwrdm desired target state */
>               ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
>       }
> 

Reply via email to