* [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@vger.kernel.org, linux-kernel@vger.kernel.org [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4624 bytes --] 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 ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ 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