* [PATCH] ASoC: sst_platform: Fix lock acquring @ 2011-04-08 7:38 Lu Guanqun 2011-04-08 7:40 ` Lu Guanqun 2011-04-11 20:15 ` Mark Brown 0 siblings, 2 replies; 7+ messages in thread From: Lu Guanqun @ 2011-04-08 7:38 UTC (permalink / raw) To: ALSA; +Cc: Koul Vinod, Takashi Iwai, Liam Girdwood, Harsha Priya, Mark Brown Fix the possible dead lock shown below: spin_lock sst_get_stream_status sst_period_elapsed intel_sst_interrupt handle_IRQ_event handle_fasteoi_irq do_IRQ common_interrupt spin_lock sst_set_stream_status sst_platform_pcm_trigger Signed-off-by: Lu Guanqun <guanqun.lu@intel.com> --- sound/soc/mid-x86/sst_platform.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/sound/soc/mid-x86/sst_platform.c b/sound/soc/mid-x86/sst_platform.c index 9ba9414..d827edb 100644 --- a/sound/soc/mid-x86/sst_platform.c +++ b/sound/soc/mid-x86/sst_platform.c @@ -116,18 +116,20 @@ struct snd_soc_dai_driver sst_platform_dai[] = { static inline void sst_set_stream_status(struct sst_runtime_stream *stream, int state) { - spin_lock(&stream->status_lock); + unsigned long flags; + spin_lock_irqsave(&stream->status_lock, flags); stream->stream_status = state; - spin_unlock(&stream->status_lock); + spin_unlock_irqrestore(&stream->status_lock, flags); } static inline int sst_get_stream_status(struct sst_runtime_stream *stream) { int state; + unsigned long flags; - spin_lock(&stream->status_lock); + spin_lock_irqsave(&stream->status_lock, flags); state = stream->stream_status; - spin_unlock(&stream->status_lock); + spin_unlock_irqrestore(&stream->status_lock, flags); return state; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ASoC: sst_platform: Fix lock acquring 2011-04-08 7:38 [PATCH] ASoC: sst_platform: Fix lock acquring Lu Guanqun @ 2011-04-08 7:40 ` Lu Guanqun 2011-04-09 4:51 ` Koul, Vinod 2011-04-11 20:15 ` Mark Brown 1 sibling, 1 reply; 7+ messages in thread From: Lu Guanqun @ 2011-04-08 7:40 UTC (permalink / raw) To: ALSA; +Cc: Koul, Vinod, Takashi Iwai, Harsha, Priya, Mark Brown, Liam Girdwood On Fri, Apr 08, 2011 at 03:38:48PM +0800, Lu Guanqun wrote: > Fix the possible dead lock shown below: > > spin_lock > sst_get_stream_status > sst_period_elapsed > intel_sst_interrupt > handle_IRQ_event > handle_fasteoi_irq > do_IRQ > common_interrupt > spin_lock > sst_set_stream_status > sst_platform_pcm_trigger > > Signed-off-by: Lu Guanqun <guanqun.lu@intel.com> This patch is based on Mark Brown's for-2.6.40 branch. It should be applied cleanly. Thanks. > --- > sound/soc/mid-x86/sst_platform.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/sound/soc/mid-x86/sst_platform.c b/sound/soc/mid-x86/sst_platform.c > index 9ba9414..d827edb 100644 > --- a/sound/soc/mid-x86/sst_platform.c > +++ b/sound/soc/mid-x86/sst_platform.c > @@ -116,18 +116,20 @@ struct snd_soc_dai_driver sst_platform_dai[] = { > static inline void sst_set_stream_status(struct sst_runtime_stream *stream, > int state) > { > - spin_lock(&stream->status_lock); > + unsigned long flags; > + spin_lock_irqsave(&stream->status_lock, flags); > stream->stream_status = state; > - spin_unlock(&stream->status_lock); > + spin_unlock_irqrestore(&stream->status_lock, flags); > } > > static inline int sst_get_stream_status(struct sst_runtime_stream *stream) > { > int state; > + unsigned long flags; > > - spin_lock(&stream->status_lock); > + spin_lock_irqsave(&stream->status_lock, flags); > state = stream->stream_status; > - spin_unlock(&stream->status_lock); > + spin_unlock_irqrestore(&stream->status_lock, flags); > return state; > } > > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- guanqun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ASoC: sst_platform: Fix lock acquring 2011-04-08 7:40 ` Lu Guanqun @ 2011-04-09 4:51 ` Koul, Vinod 2011-04-09 10:22 ` Lu Guanqun 0 siblings, 1 reply; 7+ messages in thread From: Koul, Vinod @ 2011-04-09 4:51 UTC (permalink / raw) To: Lu, Guanqun, ALSA; +Cc: Takashi Iwai, Harsha, Priya, Mark Brown, Liam Girdwood On Fri, Apr 08, 2011 at 01:11:48PM +0530, Lu Guanqun wrote: > On Fri, Apr 08, 2011 at 03:38:48PM +0800, Lu Guanqun wrote: > > Fix the possible dead lock shown below: > > > > spin_lock > > sst_get_stream_status > > sst_period_elapsed > > intel_sst_interrupt > > handle_IRQ_event > > handle_fasteoi_irq > > do_IRQ > > common_interrupt > > spin_lock > > sst_set_stream_status > > sst_platform_pcm_trigger > > > > Signed-off-by: Lu Guanqun <guanqun.lu@intel.com> Sorry, I am little unsure about this yet. Can you send more details of the deadlock you see. Which scenario it is hit, would help to send the debug trace :) ~Vinod ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ASoC: sst_platform: Fix lock acquring 2011-04-09 4:51 ` Koul, Vinod @ 2011-04-09 10:22 ` Lu Guanqun 2011-04-09 10:41 ` Lu Guanqun 0 siblings, 1 reply; 7+ messages in thread From: Lu Guanqun @ 2011-04-09 10:22 UTC (permalink / raw) To: Koul, Vinod; +Cc: Takashi Iwai, ALSA, Harsha, Priya, Mark Brown, Liam Girdwood On Sat, Apr 09, 2011 at 12:51:07PM +0800, Koul, Vinod wrote: > On Fri, Apr 08, 2011 at 01:11:48PM +0530, Lu Guanqun wrote: > > On Fri, Apr 08, 2011 at 03:38:48PM +0800, Lu Guanqun wrote: > > > Fix the possible dead lock shown below: > > > > > > spin_lock > > > sst_get_stream_status > > > sst_period_elapsed > > > intel_sst_interrupt > > > handle_IRQ_event > > > handle_fasteoi_irq > > > do_IRQ > > > common_interrupt > > > spin_lock > > > sst_set_stream_status > > > sst_platform_pcm_trigger > > > > > > Signed-off-by: Lu Guanqun <guanqun.lu@intel.com> > Sorry, I am little unsure about this yet. > Can you send more details of the deadlock you see. > Which scenario it is hit, would help to send the debug trace :) Hi Vinod, I don't get the actual debug trace, so I have to manually reveal the possible deadlock... As sst_set_stream_status could be called both in interrupt context and process context, a simple spin_lock can't protect the mutal exclusion properly. I'm afraid spin_lock_irqsave is better for this use case here. How do you think? > > ~Vinod > -- guanqun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ASoC: sst_platform: Fix lock acquring 2011-04-09 10:22 ` Lu Guanqun @ 2011-04-09 10:41 ` Lu Guanqun 2011-04-11 19:52 ` Liam Girdwood 0 siblings, 1 reply; 7+ messages in thread From: Lu Guanqun @ 2011-04-09 10:41 UTC (permalink / raw) To: Koul, Vinod; +Cc: Takashi Iwai, ALSA, Harsha, Priya, Mark Brown, Liam Girdwood On Sat, Apr 09, 2011 at 06:22:06PM +0800, Lu Guanqun wrote: > On Sat, Apr 09, 2011 at 12:51:07PM +0800, Koul, Vinod wrote: > > On Fri, Apr 08, 2011 at 01:11:48PM +0530, Lu Guanqun wrote: > > > On Fri, Apr 08, 2011 at 03:38:48PM +0800, Lu Guanqun wrote: > > > > Fix the possible dead lock shown below: > > > > > > > > spin_lock > > > > sst_get_stream_status > > > > sst_period_elapsed > > > > intel_sst_interrupt > > > > handle_IRQ_event > > > > handle_fasteoi_irq > > > > do_IRQ > > > > common_interrupt > > > > spin_lock > > > > sst_set_stream_status > > > > sst_platform_pcm_trigger > > > > > > > > Signed-off-by: Lu Guanqun <guanqun.lu@intel.com> > > Sorry, I am little unsure about this yet. > > Can you send more details of the deadlock you see. > > Which scenario it is hit, would help to send the debug trace :) > > Hi Vinod, > > I don't get the actual debug trace, so I have to manually reveal the > possible deadlock... > I compile the kernel with lockdep enabled. without this patch, it will complain with the below message: [ 161.686829] ================================= [ 161.686910] [ INFO: inconsistent lock state ] [ 161.686998] --------------------------------- [ 161.687059] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. [ 161.687127] swapper/0 [HC1[1]:SC0[0]:HE0:SE1] takes: [ 161.687181] (&(&stream->status_lock)->rlock){?.+...}, at: [<c14a1b8c>] sst_period_elapsed+0x27/0x41 [ 161.687314] {HARDIRQ-ON-W} state was registered at: [ 161.687365] [<c105f017>] __lock_acquire+0x5ca/0x13b3 [ 161.687439] [<c1060334>] lock_acquire+0xfa/0x136 [ 161.687504] [<c15f963a>] _raw_spin_lock+0x25/0x34 [ 161.687574] [<c14a1d46>] sst_platform_open+0x120/0x2a2 [ 161.687644] [<c147bab4>] soc_pcm_open+0x91/0x4c3 [ 161.687713] [<c143b540>] snd_pcm_open_substream+0x46/0x96 [ 161.687746] [<c143b70a>] snd_pcm_open+0x17a/0x341 [ 161.687746] [<c143b936>] snd_pcm_playback_open+0x2f/0x35 [ 161.687746] [<c142c5ef>] snd_open+0x1fb/0x360 [ 161.687746] [<c10c60a2>] chrdev_open+0x1cf/0x208 [ 161.687746] [<c10c1501>] __dentry_open+0x1d1/0x2db [ 161.687746] [<c10c2193>] nameidata_to_filp+0x26/0x33 [ 161.687746] [<c10ccf7c>] do_last+0x3b7/0x495 [ 161.687746] [<c10cd1fc>] do_filp_open+0x1a2/0x407 [ 161.687746] [<c10c21e4>] do_sys_open+0x44/0xc5 [ 161.687746] [<c10c2283>] sys_open+0x1e/0x26 [ 161.687746] [<c15fa135>] syscall_call+0x7/0xb [ 161.687746] irq event stamp: 858548 [ 161.687746] hardirqs last enabled at (858545): [<c1007f9a>] mwait_idle+0xf6/0x12a [ 161.687746] hardirqs last disabled at (858546): [<c1002d67>] common_interrupt+0x27/0x34 [ 161.687746] softirqs last enabled at (858548): [<c103b7b4>] _local_bh_enable+0xd/0xf [ 161.687746] softirqs last disabled at (858547): [<c103bbb5>] irq_enter+0x30/0x61 [ 161.687746] [ 161.687746] other info that might help us debug this: [ 161.687746] no locks held by swapper/0. [ 161.687746] [ 161.687746] stack backtrace: [ 161.687746] Pid: 0, comm: swapper Not tainted 2.6.37.6+ #75 [ 161.687746] Call Trace: [ 161.687746] [<c15f6a1a>] ? printk+0xf/0x11 [ 161.687746] [<c105e593>] print_usage_bug+0x151/0x15d [ 161.687746] [<c105ddd8>] ? check_usage_forwards+0x0/0xa9 [ 161.687746] [<c105e83a>] mark_lock+0x29b/0x4ae [ 161.687746] [<c105efa3>] __lock_acquire+0x556/0x13b3 [ 161.687746] [<c1090941>] ? perf_pmu_enable+0x1d/0x1f [ 161.687746] [<c1051e23>] ? __run_hrtimer+0x22f/0x2c7 [ 161.687746] [<c14a1b8c>] ? sst_period_elapsed+0x27/0x41 [ 161.687746] [<c1060334>] lock_acquire+0xfa/0x136 [ 161.687746] [<c14a1b8c>] ? sst_period_elapsed+0x27/0x41 [ 161.687746] [<c15f963a>] _raw_spin_lock+0x25/0x34 [ 161.687746] [<c14a1b8c>] ? sst_period_elapsed+0x27/0x41 [ 161.687746] [<c14a1b8c>] sst_period_elapsed+0x27/0x41 [ 161.687746] [<c14232a1>] intel_sst_interrupt+0x78/0x19d [ 161.687746] [<c107516f>] handle_IRQ_event+0xb7/0x208 [ 161.687746] [<c1076c6a>] handle_fasteoi_irq+0x90/0xc7 [ 161.687746] [<c1076bda>] ? handle_fasteoi_irq+0x0/0xc7 [ 161.687746] <IRQ> [<c10042de>] ? do_IRQ+0x3e/0x97 [ 161.687746] [<c1002d6e>] ? common_interrupt+0x2e/0x34 [ 161.687746] [<c106007b>] ? lock_release+0x36/0x1f5 [ 161.687746] [<c1007fa2>] ? mwait_idle+0xfe/0x12a [ 161.687746] [<c1001529>] ? cpu_idle+0x4d/0x129 [ 161.687746] [<c15c034b>] ? rest_init+0xab/0xb0 [ 161.687746] [<c1a2576d>] ? start_kernel+0x2dd/0x2e2 [ 161.687746] [<c1a250d5>] ? i386_start_kernel+0xd5/0xdc with this patch, I don't see such messages coming out. -- guanqun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ASoC: sst_platform: Fix lock acquring 2011-04-09 10:41 ` Lu Guanqun @ 2011-04-11 19:52 ` Liam Girdwood 0 siblings, 0 replies; 7+ messages in thread From: Liam Girdwood @ 2011-04-11 19:52 UTC (permalink / raw) To: Lu Guanqun; +Cc: Koul, Vinod, Takashi Iwai, ALSA, Harsha, Priya, Mark Brown On Sat, 2011-04-09 at 18:41 +0800, Lu Guanqun wrote: > I compile the kernel with lockdep enabled. without this patch, it will > complain with the below message: Acked-by: Liam Girdwood <lrg@ti.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ASoC: sst_platform: Fix lock acquring 2011-04-08 7:38 [PATCH] ASoC: sst_platform: Fix lock acquring Lu Guanqun 2011-04-08 7:40 ` Lu Guanqun @ 2011-04-11 20:15 ` Mark Brown 1 sibling, 0 replies; 7+ messages in thread From: Mark Brown @ 2011-04-11 20:15 UTC (permalink / raw) To: Lu Guanqun; +Cc: Koul Vinod, Takashi Iwai, ALSA, Harsha Priya, Liam Girdwood On Fri, Apr 08, 2011 at 03:38:48PM +0800, Lu Guanqun wrote: > Fix the possible dead lock shown below: > > spin_lock Applied, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-04-11 20:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-08 7:38 [PATCH] ASoC: sst_platform: Fix lock acquring Lu Guanqun 2011-04-08 7:40 ` Lu Guanqun 2011-04-09 4:51 ` Koul, Vinod 2011-04-09 10:22 ` Lu Guanqun 2011-04-09 10:41 ` Lu Guanqun 2011-04-11 19:52 ` Liam Girdwood 2011-04-11 20:15 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.