diff mbox series

ARM: imx6q: Fixup RCU usage for cpuidle

Message ID 20200930112023.121821-1-ulf.hansson@linaro.org
State New
Headers show
Series ARM: imx6q: Fixup RCU usage for cpuidle | expand

Commit Message

Ulf Hansson Sept. 30, 2020, 11:20 a.m. UTC
The commit eb1f00237aca ("lockdep,trace: Expose tracepoints"), started to
expose us for tracepoints. For imx6q cpuidle, this leads to an RCU splat
according to below.

[6.870684] [<c0db7690>] (_raw_spin_lock) from [<c011f6a4>] (imx6q_enter_wait+0x18/0x9c)
[6.878846] [<c011f6a4>] (imx6q_enter_wait) from [<c09abfb0>] (cpuidle_enter_state+0x168/0x5e4)

To fix the problem, let's assign the corresponding idlestate->flags the
CPUIDLE_FLAG_RCU_IDLE bit, which enables us to call rcu_idle_enter|exit()
at the proper point.

Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 arch/arm/mach-imx/cpuidle-imx6q.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Peter Zijlstra Sept. 30, 2020, 11:33 a.m. UTC | #1
On Wed, Sep 30, 2020 at 01:20:23PM +0200, Ulf Hansson wrote:
> The commit eb1f00237aca ("lockdep,trace: Expose tracepoints"), started to
> expose us for tracepoints. For imx6q cpuidle, this leads to an RCU splat
> according to below.
> 
> [6.870684] [<c0db7690>] (_raw_spin_lock) from [<c011f6a4>] (imx6q_enter_wait+0x18/0x9c)
> [6.878846] [<c011f6a4>] (imx6q_enter_wait) from [<c09abfb0>] (cpuidle_enter_state+0x168/0x5e4)
> 
> To fix the problem, let's assign the corresponding idlestate->flags the
> CPUIDLE_FLAG_RCU_IDLE bit, which enables us to call rcu_idle_enter|exit()
> at the proper point.
> 
> Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Yep, that looks about right.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  arch/arm/mach-imx/cpuidle-imx6q.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
> index 24dd5bbe60e4..094337dc1bc7 100644
> --- a/arch/arm/mach-imx/cpuidle-imx6q.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
> @@ -24,7 +24,9 @@ static int imx6q_enter_wait(struct cpuidle_device *dev,
>  		imx6_set_lpm(WAIT_UNCLOCKED);
>  	raw_spin_unlock(&cpuidle_lock);
>  
> +	rcu_idle_enter();
>  	cpu_do_idle();
> +	rcu_idle_exit();
>  
>  	raw_spin_lock(&cpuidle_lock);
>  	if (num_idle_cpus-- == num_online_cpus())
> @@ -44,7 +46,7 @@ static struct cpuidle_driver imx6q_cpuidle_driver = {
>  		{
>  			.exit_latency = 50,
>  			.target_residency = 75,
> -			.flags = CPUIDLE_FLAG_TIMER_STOP,
> +			.flags = CPUIDLE_FLAG_TIMER_STOP | CPUIDLE_FLAG_RCU_IDLE,
>  			.enter = imx6q_enter_wait,
>  			.name = "WAIT",
>  			.desc = "Clock off",
> -- 
> 2.25.1
>
Rafael J. Wysocki Sept. 30, 2020, 3:44 p.m. UTC | #2
On Wed, Sep 30, 2020 at 1:33 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Sep 30, 2020 at 01:20:23PM +0200, Ulf Hansson wrote:
> > The commit eb1f00237aca ("lockdep,trace: Expose tracepoints"), started to
> > expose us for tracepoints. For imx6q cpuidle, this leads to an RCU splat
> > according to below.
> >
> > [6.870684] [<c0db7690>] (_raw_spin_lock) from [<c011f6a4>] (imx6q_enter_wait+0x18/0x9c)
> > [6.878846] [<c011f6a4>] (imx6q_enter_wait) from [<c09abfb0>] (cpuidle_enter_state+0x168/0x5e4)
> >
> > To fix the problem, let's assign the corresponding idlestate->flags the
> > CPUIDLE_FLAG_RCU_IDLE bit, which enables us to call rcu_idle_enter|exit()
> > at the proper point.
> >
> > Reported-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Yep, that looks about right.
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> > ---
> >  arch/arm/mach-imx/cpuidle-imx6q.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
> > index 24dd5bbe60e4..094337dc1bc7 100644
> > --- a/arch/arm/mach-imx/cpuidle-imx6q.c
> > +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
> > @@ -24,7 +24,9 @@ static int imx6q_enter_wait(struct cpuidle_device *dev,
> >               imx6_set_lpm(WAIT_UNCLOCKED);
> >       raw_spin_unlock(&cpuidle_lock);
> >
> > +     rcu_idle_enter();
> >       cpu_do_idle();
> > +     rcu_idle_exit();
> >
> >       raw_spin_lock(&cpuidle_lock);
> >       if (num_idle_cpus-- == num_online_cpus())
> > @@ -44,7 +46,7 @@ static struct cpuidle_driver imx6q_cpuidle_driver = {
> >               {
> >                       .exit_latency = 50,
> >                       .target_residency = 75,
> > -                     .flags = CPUIDLE_FLAG_TIMER_STOP,
> > +                     .flags = CPUIDLE_FLAG_TIMER_STOP | CPUIDLE_FLAG_RCU_IDLE,
> >                       .enter = imx6q_enter_wait,
> >                       .name = "WAIT",
> >                       .desc = "Clock off",
> > --

Applied as 5.9-rc8 material, thanks!
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c
index 24dd5bbe60e4..094337dc1bc7 100644
--- a/arch/arm/mach-imx/cpuidle-imx6q.c
+++ b/arch/arm/mach-imx/cpuidle-imx6q.c
@@ -24,7 +24,9 @@  static int imx6q_enter_wait(struct cpuidle_device *dev,
 		imx6_set_lpm(WAIT_UNCLOCKED);
 	raw_spin_unlock(&cpuidle_lock);
 
+	rcu_idle_enter();
 	cpu_do_idle();
+	rcu_idle_exit();
 
 	raw_spin_lock(&cpuidle_lock);
 	if (num_idle_cpus-- == num_online_cpus())
@@ -44,7 +46,7 @@  static struct cpuidle_driver imx6q_cpuidle_driver = {
 		{
 			.exit_latency = 50,
 			.target_residency = 75,
-			.flags = CPUIDLE_FLAG_TIMER_STOP,
+			.flags = CPUIDLE_FLAG_TIMER_STOP | CPUIDLE_FLAG_RCU_IDLE,
 			.enter = imx6q_enter_wait,
 			.name = "WAIT",
 			.desc = "Clock off",