linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock
@ 2014-11-13 10:47 Maurice Petallo
  2014-11-25  5:23 ` Eduardo Valentin
  0 siblings, 1 reply; 4+ messages in thread
From: Maurice Petallo @ 2014-11-13 10:47 UTC (permalink / raw)
  To: Zhang Rui; +Cc: Srinivas Pandruvada, linux-pm, linux-kernel, Maurice Petallo

The driver calls spin_lock_irqsave during DTS interrupt. The interrupt
handle then calls thermal_zone_device_update which implicitly calls
a sleep function and produce the following bug:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
in_atomic(): 1, irqs_disabled(): 1, pid: 920, name: irq/86-soc_dts
CPU: 0 PID: 920 Comm: irq/86-soc_dts Tainted: G            E  3.17.0-rc2+ #1
Hardware name: Intel Corp. VALLEYVIEW B3 PLATFORM/NOTEBOOK, BIOS BYTICRB1.86C.0092.R31.1408290850 08/29/2014
 00000000 00000000 c25dbe74 c1818cfd f3cc488c c25dbe9c c1059305 c1b4063b
 00000001 00000001 00000398 f3cc488c f6817644 f6817644 f3ecc6c0 c25dbea8
 c18208f2 f6817400 c25dbebc c159b0bb c25dbedc f6817400 f32a2300 c25dbee8
Call Trace:
 [<c1818cfd>] dump_stack+0x48/0x60
 [<c1059305>] __might_sleep+0xec/0xf4
 [<c18208f2>] mutex_lock+0x1c/0x34
 [<c159b0bb>] thermal_zone_get_temp+0x34/0x59
 [<c159bde5>] thermal_zone_device_update+0x2d/0xcb
 [<f85da16a>] ? iosf_mbi_write+0x6c/0x74 [iosf_mbi]
 [<f7c7445d>] soc_irq_thread_fn+0x10c/0x163 [intel_soc_dts_thermal]
 [<c107b72b>] irq_thread_fn+0x18/0x2a
 [<c107bedb>] irq_thread+0x81/0x11f
 [<c107b713>] ? irq_finalize_oneshot+0x7c/0x7c
 [<c107bf79>] ? irq_thread+0x11f/0x11f
 [<c107be5a>] ? wake_threads_waitq+0x31/0x31
 [<c1054217>] kthread+0x87/0x8c
 [<c1821e41>] ret_from_kernel_thread+0x21/0x30
 [<c1054190>] ? __kthread_parkme+0x55/0x55

Signed-off-by: Maurice Petallo <mauricex.r.petallo@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/thermal/intel_soc_dts_thermal.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/intel_soc_dts_thermal.c b/drivers/thermal/intel_soc_dts_thermal.c
index a6a0a18..5580f5b 100644
--- a/drivers/thermal/intel_soc_dts_thermal.c
+++ b/drivers/thermal/intel_soc_dts_thermal.c
@@ -360,6 +360,9 @@ static void proc_thermal_interrupt(void)
 	u32 sticky_out;
 	int status;
 	u32 ptmc_out;
+	unsigned long flags;
+
+	spin_lock_irqsave(&intr_notify_lock, flags);
 
 	/* Clear APIC interrupt */
 	status = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
@@ -378,21 +381,20 @@ static void proc_thermal_interrupt(void)
 		/* reset sticky bit */
 		status = iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
 					SOC_DTS_OFFSET_PTTSS, sticky_out);
+		spin_unlock_irqrestore(&intr_notify_lock, flags);
+
 		for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
 			pr_debug("TZD update for zone %d\n", i);
 			thermal_zone_device_update(soc_dts[i]->tzone);
 		}
-	}
+	} else
+		spin_unlock_irqrestore(&intr_notify_lock, flags);
 
 }
 
 static irqreturn_t soc_irq_thread_fn(int irq, void *dev_data)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&intr_notify_lock, flags);
 	proc_thermal_interrupt();
-	spin_unlock_irqrestore(&intr_notify_lock, flags);
 	pr_debug("proc_thermal_interrupt\n");
 
 	return IRQ_HANDLED;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock
  2014-11-13 10:47 [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock Maurice Petallo
@ 2014-11-25  5:23 ` Eduardo Valentin
  2014-11-25 17:56   ` Srinivas Pandruvada
  0 siblings, 1 reply; 4+ messages in thread
From: Eduardo Valentin @ 2014-11-25  5:23 UTC (permalink / raw)
  To: Maurice Petallo; +Cc: Zhang Rui, Srinivas Pandruvada, linux-pm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]

Hi,

A minor thing. Your patch title makes us think this fix affects the
thermal subsystem completely, whereas it affects a specific driver.


On Thu, Nov 13, 2014 at 06:47:04PM +0800, Maurice Petallo wrote:
> The driver calls spin_lock_irqsave during DTS interrupt. The interrupt
> handle then calls thermal_zone_device_update which implicitly calls
> a sleep function and produce the following bug:
> 
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
> in_atomic(): 1, irqs_disabled(): 1, pid: 920, name: irq/86-soc_dts
> CPU: 0 PID: 920 Comm: irq/86-soc_dts Tainted: G            E  3.17.0-rc2+ #1
> Hardware name: Intel Corp. VALLEYVIEW B3 PLATFORM/NOTEBOOK, BIOS BYTICRB1.86C.0092.R31.1408290850 08/29/2014
>  00000000 00000000 c25dbe74 c1818cfd f3cc488c c25dbe9c c1059305 c1b4063b
>  00000001 00000001 00000398 f3cc488c f6817644 f6817644 f3ecc6c0 c25dbea8
>  c18208f2 f6817400 c25dbebc c159b0bb c25dbedc f6817400 f32a2300 c25dbee8
> Call Trace:
>  [<c1818cfd>] dump_stack+0x48/0x60
>  [<c1059305>] __might_sleep+0xec/0xf4
>  [<c18208f2>] mutex_lock+0x1c/0x34
>  [<c159b0bb>] thermal_zone_get_temp+0x34/0x59
>  [<c159bde5>] thermal_zone_device_update+0x2d/0xcb
>  [<f85da16a>] ? iosf_mbi_write+0x6c/0x74 [iosf_mbi]
>  [<f7c7445d>] soc_irq_thread_fn+0x10c/0x163 [intel_soc_dts_thermal]
>  [<c107b72b>] irq_thread_fn+0x18/0x2a
>  [<c107bedb>] irq_thread+0x81/0x11f
>  [<c107b713>] ? irq_finalize_oneshot+0x7c/0x7c
>  [<c107bf79>] ? irq_thread+0x11f/0x11f
>  [<c107be5a>] ? wake_threads_waitq+0x31/0x31
>  [<c1054217>] kthread+0x87/0x8c
>  [<c1821e41>] ret_from_kernel_thread+0x21/0x30
>  [<c1054190>] ? __kthread_parkme+0x55/0x55
> 
> Signed-off-by: Maurice Petallo <mauricex.r.petallo@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/thermal/intel_soc_dts_thermal.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/intel_soc_dts_thermal.c b/drivers/thermal/intel_soc_dts_thermal.c
> index a6a0a18..5580f5b 100644
> --- a/drivers/thermal/intel_soc_dts_thermal.c
> +++ b/drivers/thermal/intel_soc_dts_thermal.c
> @@ -360,6 +360,9 @@ static void proc_thermal_interrupt(void)
>  	u32 sticky_out;
>  	int status;
>  	u32 ptmc_out;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&intr_notify_lock, flags);
>  
>  	/* Clear APIC interrupt */
>  	status = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
> @@ -378,21 +381,20 @@ static void proc_thermal_interrupt(void)
>  		/* reset sticky bit */
>  		status = iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
>  					SOC_DTS_OFFSET_PTTSS, sticky_out);
> +		spin_unlock_irqrestore(&intr_notify_lock, flags);
> +
>  		for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
>  			pr_debug("TZD update for zone %d\n", i);
>  			thermal_zone_device_update(soc_dts[i]->tzone);
>  		}
> -	}
> +	} else
> +		spin_unlock_irqrestore(&intr_notify_lock, flags);
>  
>  }
>  
>  static irqreturn_t soc_irq_thread_fn(int irq, void *dev_data)
>  {
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&intr_notify_lock, flags);
>  	proc_thermal_interrupt();
> -	spin_unlock_irqrestore(&intr_notify_lock, flags);
>  	pr_debug("proc_thermal_interrupt\n");
>  
>  	return IRQ_HANDLED;

A part from the minor thing I mentioned before, you can add my:

Acked-by: Eduardo Valentin <edubezval@gmail.com>

> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock
  2014-11-25  5:23 ` Eduardo Valentin
@ 2014-11-25 17:56   ` Srinivas Pandruvada
  2014-11-28  2:08     ` Petallo, MauriceX R
  0 siblings, 1 reply; 4+ messages in thread
From: Srinivas Pandruvada @ 2014-11-25 17:56 UTC (permalink / raw)
  To: Eduardo Valentin; +Cc: Maurice Petallo, Zhang Rui, linux-pm, linux-kernel

On Tue, 2014-11-25 at 01:23 -0400, Eduardo Valentin wrote:
Hi, 
> Hi,
> 
> A minor thing. Your patch title makes us think this fix affects the
> thermal subsystem completely, whereas it affects a specific driver.
> 
Good point. I overlooked this.

Maurice,
Change the title to something like
"thermal: Intel SoC DTS: Don't call thermal_zone_device_update inside
spinlock"

Thanks,
Srinivas 
> 
> On Thu, Nov 13, 2014 at 06:47:04PM +0800, Maurice Petallo wrote:
> > The driver calls spin_lock_irqsave during DTS interrupt. The interrupt
> > handle then calls thermal_zone_device_update which implicitly calls
> > a sleep function and produce the following bug:
> >
>  
> > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
> > in_atomic(): 1, irqs_disabled(): 1, pid: 920, name: irq/86-soc_dts
> > CPU: 0 PID: 920 Comm: irq/86-soc_dts Tainted: G            E  3.17.0-rc2+ #1
> > Hardware name: Intel Corp. VALLEYVIEW B3 PLATFORM/NOTEBOOK, BIOS BYTICRB1.86C.0092.R31.1408290850 08/29/2014
> >  00000000 00000000 c25dbe74 c1818cfd f3cc488c c25dbe9c c1059305 c1b4063b
> >  00000001 00000001 00000398 f3cc488c f6817644 f6817644 f3ecc6c0 c25dbea8
> >  c18208f2 f6817400 c25dbebc c159b0bb c25dbedc f6817400 f32a2300 c25dbee8
> > Call Trace:
> >  [<c1818cfd>] dump_stack+0x48/0x60
> >  [<c1059305>] __might_sleep+0xec/0xf4
> >  [<c18208f2>] mutex_lock+0x1c/0x34
> >  [<c159b0bb>] thermal_zone_get_temp+0x34/0x59
> >  [<c159bde5>] thermal_zone_device_update+0x2d/0xcb
> >  [<f85da16a>] ? iosf_mbi_write+0x6c/0x74 [iosf_mbi]
> >  [<f7c7445d>] soc_irq_thread_fn+0x10c/0x163 [intel_soc_dts_thermal]
> >  [<c107b72b>] irq_thread_fn+0x18/0x2a
> >  [<c107bedb>] irq_thread+0x81/0x11f
> >  [<c107b713>] ? irq_finalize_oneshot+0x7c/0x7c
> >  [<c107bf79>] ? irq_thread+0x11f/0x11f
> >  [<c107be5a>] ? wake_threads_waitq+0x31/0x31
> >  [<c1054217>] kthread+0x87/0x8c
> >  [<c1821e41>] ret_from_kernel_thread+0x21/0x30
> >  [<c1054190>] ? __kthread_parkme+0x55/0x55
> > 
> > Signed-off-by: Maurice Petallo <mauricex.r.petallo@intel.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > ---
> >  drivers/thermal/intel_soc_dts_thermal.c |   12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/thermal/intel_soc_dts_thermal.c b/drivers/thermal/intel_soc_dts_thermal.c
> > index a6a0a18..5580f5b 100644
> > --- a/drivers/thermal/intel_soc_dts_thermal.c
> > +++ b/drivers/thermal/intel_soc_dts_thermal.c
> > @@ -360,6 +360,9 @@ static void proc_thermal_interrupt(void)
> >  	u32 sticky_out;
> >  	int status;
> >  	u32 ptmc_out;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&intr_notify_lock, flags);
> >  
> >  	/* Clear APIC interrupt */
> >  	status = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
> > @@ -378,21 +381,20 @@ static void proc_thermal_interrupt(void)
> >  		/* reset sticky bit */
> >  		status = iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
> >  					SOC_DTS_OFFSET_PTTSS, sticky_out);
> > +		spin_unlock_irqrestore(&intr_notify_lock, flags);
> > +
> >  		for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
> >  			pr_debug("TZD update for zone %d\n", i);
> >  			thermal_zone_device_update(soc_dts[i]->tzone);
> >  		}
> > -	}
> > +	} else
> > +		spin_unlock_irqrestore(&intr_notify_lock, flags);
> >  
> >  }
> >  
> >  static irqreturn_t soc_irq_thread_fn(int irq, void *dev_data)
> >  {
> > -	unsigned long flags;
> > -
> > -	spin_lock_irqsave(&intr_notify_lock, flags);
> >  	proc_thermal_interrupt();
> > -	spin_unlock_irqrestore(&intr_notify_lock, flags);
> >  	pr_debug("proc_thermal_interrupt\n");
> >  
> >  	return IRQ_HANDLED;
> 
> A part from the minor thing I mentioned before, you can add my:
> 
> Acked-by: Eduardo Valentin <edubezval@gmail.com>
> 
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock
  2014-11-25 17:56   ` Srinivas Pandruvada
@ 2014-11-28  2:08     ` Petallo, MauriceX R
  0 siblings, 0 replies; 4+ messages in thread
From: Petallo, MauriceX R @ 2014-11-28  2:08 UTC (permalink / raw)
  To: Srinivas Pandruvada, Eduardo Valentin; +Cc: Zhang, Rui, linux-pm, linux-kernel

Hi,
Ok. I will update the title.
Thanks for the feedbacks and Ack.

Regards,
-Maurice

-----Original Message-----
From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com] 
Sent: Wednesday, November 26, 2014 1:57 AM
To: Eduardo Valentin
Cc: Petallo, MauriceX R; Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock

On Tue, 2014-11-25 at 01:23 -0400, Eduardo Valentin wrote:
Hi, 
> Hi,
> 
> A minor thing. Your patch title makes us think this fix affects the 
> thermal subsystem completely, whereas it affects a specific driver.
> 
Good point. I overlooked this.

Maurice,
Change the title to something like
"thermal: Intel SoC DTS: Don't call thermal_zone_device_update inside spinlock"

Thanks,
Srinivas 
> 
> On Thu, Nov 13, 2014 at 06:47:04PM +0800, Maurice Petallo wrote:
> > The driver calls spin_lock_irqsave during DTS interrupt. The 
> > interrupt handle then calls thermal_zone_device_update which 
> > implicitly calls a sleep function and produce the following bug:
> >
>  
> > BUG: sleeping function called from invalid context at 
> > kernel/locking/mutex.c:97
> > in_atomic(): 1, irqs_disabled(): 1, pid: 920, name: irq/86-soc_dts
> > CPU: 0 PID: 920 Comm: irq/86-soc_dts Tainted: G            E  3.17.0-rc2+ #1
> > Hardware name: Intel Corp. VALLEYVIEW B3 PLATFORM/NOTEBOOK, BIOS 
> > BYTICRB1.86C.0092.R31.1408290850 08/29/2014
> >  00000000 00000000 c25dbe74 c1818cfd f3cc488c c25dbe9c c1059305 
> > c1b4063b
> >  00000001 00000001 00000398 f3cc488c f6817644 f6817644 f3ecc6c0 
> > c25dbea8
> >  c18208f2 f6817400 c25dbebc c159b0bb c25dbedc f6817400 f32a2300 
> > c25dbee8 Call Trace:
> >  [<c1818cfd>] dump_stack+0x48/0x60
> >  [<c1059305>] __might_sleep+0xec/0xf4  [<c18208f2>] 
> > mutex_lock+0x1c/0x34  [<c159b0bb>] thermal_zone_get_temp+0x34/0x59  
> > [<c159bde5>] thermal_zone_device_update+0x2d/0xcb
> >  [<f85da16a>] ? iosf_mbi_write+0x6c/0x74 [iosf_mbi]  [<f7c7445d>] 
> > soc_irq_thread_fn+0x10c/0x163 [intel_soc_dts_thermal]  [<c107b72b>] 
> > irq_thread_fn+0x18/0x2a  [<c107bedb>] irq_thread+0x81/0x11f  
> > [<c107b713>] ? irq_finalize_oneshot+0x7c/0x7c  [<c107bf79>] ? 
> > irq_thread+0x11f/0x11f  [<c107be5a>] ? wake_threads_waitq+0x31/0x31  
> > [<c1054217>] kthread+0x87/0x8c  [<c1821e41>] 
> > ret_from_kernel_thread+0x21/0x30  [<c1054190>] ? 
> > __kthread_parkme+0x55/0x55
> > 
> > Signed-off-by: Maurice Petallo <mauricex.r.petallo@intel.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > ---
> >  drivers/thermal/intel_soc_dts_thermal.c |   12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/thermal/intel_soc_dts_thermal.c 
> > b/drivers/thermal/intel_soc_dts_thermal.c
> > index a6a0a18..5580f5b 100644
> > --- a/drivers/thermal/intel_soc_dts_thermal.c
> > +++ b/drivers/thermal/intel_soc_dts_thermal.c
> > @@ -360,6 +360,9 @@ static void proc_thermal_interrupt(void)
> >  	u32 sticky_out;
> >  	int status;
> >  	u32 ptmc_out;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&intr_notify_lock, flags);
> >  
> >  	/* Clear APIC interrupt */
> >  	status = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ, @@ 
> > -378,21 +381,20 @@ static void proc_thermal_interrupt(void)
> >  		/* reset sticky bit */
> >  		status = iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
> >  					SOC_DTS_OFFSET_PTTSS, sticky_out);
> > +		spin_unlock_irqrestore(&intr_notify_lock, flags);
> > +
> >  		for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
> >  			pr_debug("TZD update for zone %d\n", i);
> >  			thermal_zone_device_update(soc_dts[i]->tzone);
> >  		}
> > -	}
> > +	} else
> > +		spin_unlock_irqrestore(&intr_notify_lock, flags);
> >  
> >  }
> >  
> >  static irqreturn_t soc_irq_thread_fn(int irq, void *dev_data)  {
> > -	unsigned long flags;
> > -
> > -	spin_lock_irqsave(&intr_notify_lock, flags);
> >  	proc_thermal_interrupt();
> > -	spin_unlock_irqrestore(&intr_notify_lock, flags);
> >  	pr_debug("proc_thermal_interrupt\n");
> >  
> >  	return IRQ_HANDLED;
> 
> A part from the minor thing I mentioned before, you can add my:
> 
> Acked-by: Eduardo Valentin <edubezval@gmail.com>
> 
> > --
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pm" 
> > in the body of a message to majordomo@vger.kernel.org More majordomo 
> > info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-11-28  2:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-13 10:47 [PATCH] thermal: Don't call thermal_zone_device_update inside spinlock Maurice Petallo
2014-11-25  5:23 ` Eduardo Valentin
2014-11-25 17:56   ` Srinivas Pandruvada
2014-11-28  2:08     ` Petallo, MauriceX R

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).