* [PATCH 0/10] snd_pcm_stop() lock fixes
@ 2013-07-15 13:06 Takashi Iwai
2013-07-15 13:06 ` [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call Takashi Iwai
` (10 more replies)
0 siblings, 11 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
Hi,
while reviewing another patch series, I stumbled on the snd_pcm_lock()
call in an unlocked context. Then I started looking through the whole
tree, and found a bunch of drivers doing it wrong, too. So here is a
patch series to fix them.
[PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call
[PATCH 02/10] ALSA: atiixp: Fix unlocked snd_pcm_stop() call
[PATCH 03/10] ALSA: 6fire: Fix unlocked snd_pcm_stop() call
[PATCH 04/10] ALSA: ua101: Fix unlocked snd_pcm_stop() call
[PATCH 05/10] ALSA: usx2y: Fix unlocked snd_pcm_stop() call
[PATCH 06/10] ALSA: pxa2xx: Fix unlocked snd_pcm_stop() call
[PATCH 07/10] ASoC: atmel: Fix unlocked snd_pcm_stop() call
[PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
[PATCH 09/10] [media] saa7134: Fix unlocked snd_pcm_stop() call
[PATCH 10/10] staging: line6: Fix unlocked snd_pcm_stop() call
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 13:06 ` [PATCH 02/10] ALSA: atiixp: " Takashi Iwai
` (9 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/pci/asihpi/asihpi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c
index 185d54a..dc632cd 100644
--- a/sound/pci/asihpi/asihpi.c
+++ b/sound/pci/asihpi/asihpi.c
@@ -769,7 +769,10 @@ static void snd_card_asihpi_timer_function(unsigned long data)
s->number);
ds->drained_count++;
if (ds->drained_count > 20) {
+ unsigned long flags;
+ snd_pcm_stream_lock_irqsave(s, flags);
snd_pcm_stop(s, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock_irqrestore(s, flags);
continue;
}
} else {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 02/10] ALSA: atiixp: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
2013-07-15 13:06 ` [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 13:06 ` [PATCH 03/10] ALSA: 6fire: " Takashi Iwai
` (8 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/pci/atiixp.c | 2 ++
sound/pci/atiixp_modem.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
index fe4c61b..f6dec3e 100644
--- a/sound/pci/atiixp.c
+++ b/sound/pci/atiixp.c
@@ -689,7 +689,9 @@ static void snd_atiixp_xrun_dma(struct atiixp *chip, struct atiixp_dma *dma)
if (! dma->substream || ! dma->running)
return;
snd_printdd("atiixp: XRUN detected (DMA %d)\n", dma->ops->type);
+ snd_pcm_stream_lock(dma->substream);
snd_pcm_stop(dma->substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock(dma->substream);
}
/*
diff --git a/sound/pci/atiixp_modem.c b/sound/pci/atiixp_modem.c
index cf29b9a..289563e 100644
--- a/sound/pci/atiixp_modem.c
+++ b/sound/pci/atiixp_modem.c
@@ -638,7 +638,9 @@ static void snd_atiixp_xrun_dma(struct atiixp_modem *chip,
if (! dma->substream || ! dma->running)
return;
snd_printdd("atiixp-modem: XRUN detected (DMA %d)\n", dma->ops->type);
+ snd_pcm_stream_lock(dma->substream);
snd_pcm_stop(dma->substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock(dma->substream);
}
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 03/10] ALSA: 6fire: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
2013-07-15 13:06 ` [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call Takashi Iwai
2013-07-15 13:06 ` [PATCH 02/10] ALSA: atiixp: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 13:06 ` [PATCH 04/10] ALSA: ua101: " Takashi Iwai
` (7 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/usb/6fire/pcm.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/usb/6fire/pcm.c b/sound/usb/6fire/pcm.c
index c5b9cac..2aa4e13 100644
--- a/sound/usb/6fire/pcm.c
+++ b/sound/usb/6fire/pcm.c
@@ -639,17 +639,25 @@ int usb6fire_pcm_init(struct sfire_chip *chip)
void usb6fire_pcm_abort(struct sfire_chip *chip)
{
struct pcm_runtime *rt = chip->pcm;
+ unsigned long flags;
int i;
if (rt) {
rt->panic = true;
- if (rt->playback.instance)
+ if (rt->playback.instance) {
+ snd_pcm_stream_lock_irqsave(rt->playback.instance, flags);
snd_pcm_stop(rt->playback.instance,
SNDRV_PCM_STATE_XRUN);
- if (rt->capture.instance)
+ snd_pcm_stream_unlock_irqrestore(rt->playback.instance, flags);
+ }
+
+ if (rt->capture.instance) {
+ snd_pcm_stream_lock_irqsave(rt->capture.instance, flags);
snd_pcm_stop(rt->capture.instance,
SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock_irqrestore(rt->capture.instance, flags);
+ }
for (i = 0; i < PCM_N_URBS; i++) {
usb_poison_urb(&rt->in_urbs[i].instance);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 04/10] ALSA: ua101: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (2 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 03/10] ALSA: 6fire: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 19:01 ` Clemens Ladisch
2013-07-15 13:06 ` [PATCH 05/10] ALSA: usx2y: " Takashi Iwai
` (6 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/usb/misc/ua101.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/sound/usb/misc/ua101.c b/sound/usb/misc/ua101.c
index 8b5d2c5..5093159 100644
--- a/sound/usb/misc/ua101.c
+++ b/sound/usb/misc/ua101.c
@@ -613,14 +613,24 @@ static int start_usb_playback(struct ua101 *ua)
static void abort_alsa_capture(struct ua101 *ua)
{
- if (test_bit(ALSA_CAPTURE_RUNNING, &ua->states))
+ unsigned long flags;
+
+ if (test_bit(ALSA_CAPTURE_RUNNING, &ua->states)) {
+ snd_pcm_stream_lock_irqsave(ua->capture.substream, flags);
snd_pcm_stop(ua->capture.substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock_irqrestore(ua->capture.substream, flags);
+ }
}
static void abort_alsa_playback(struct ua101 *ua)
{
- if (test_bit(ALSA_PLAYBACK_RUNNING, &ua->states))
+ unsigned long flags;
+
+ if (test_bit(ALSA_PLAYBACK_RUNNING, &ua->states)) {
+ snd_pcm_stream_lock_irqsave(ua->playback.substream, flags);
snd_pcm_stop(ua->playback.substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock_irqrestore(ua->playback.substream, flags);
+ }
}
static int set_stream_hw(struct ua101 *ua, struct snd_pcm_substream *substream,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 05/10] ALSA: usx2y: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (3 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 04/10] ALSA: ua101: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 13:06 ` [PATCH 06/10] ALSA: pxa2xx: " Takashi Iwai
` (5 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/usb/usx2y/usbusx2yaudio.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
index 4967fe9..63fb521 100644
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -273,7 +273,11 @@ static void usX2Y_clients_stop(struct usX2Ydev *usX2Y)
struct snd_usX2Y_substream *subs = usX2Y->subs[s];
if (subs) {
if (atomic_read(&subs->state) >= state_PRERUNNING) {
+ unsigned long flags;
+
+ snd_pcm_stream_lock_irqsave(subs->pcm_substream, flags);
snd_pcm_stop(subs->pcm_substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock_irqrestore(subs->pcm_substream, flags);
}
for (u = 0; u < NRURBS; u++) {
struct urb *urb = subs->urb[u];
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 06/10] ALSA: pxa2xx: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (4 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 05/10] ALSA: usx2y: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 16:00 ` Mark Brown
2013-07-15 13:06 ` [PATCH 07/10] ASoC: atmel: " Takashi Iwai
` (4 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/arm/pxa2xx-pcm-lib.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/arm/pxa2xx-pcm-lib.c b/sound/arm/pxa2xx-pcm-lib.c
index 76e0d56..823359e 100644
--- a/sound/arm/pxa2xx-pcm-lib.c
+++ b/sound/arm/pxa2xx-pcm-lib.c
@@ -166,7 +166,9 @@ void pxa2xx_pcm_dma_irq(int dma_ch, void *dev_id)
} else {
printk(KERN_ERR "%s: DMA error on channel %d (DCSR=%#x)\n",
rtd->params->name, dma_ch, dcsr);
+ snd_pcm_stream_lock(substream);
snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock(substream);
}
}
EXPORT_SYMBOL(pxa2xx_pcm_dma_irq);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 07/10] ASoC: atmel: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (5 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 06/10] ALSA: pxa2xx: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 16:00 ` Mark Brown
2013-07-15 13:06 ` [PATCH 08/10] ASoC: s6000: " Takashi Iwai
` (3 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/atmel/atmel-pcm-dma.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c
index 1d38fd0..d128265 100644
--- a/sound/soc/atmel/atmel-pcm-dma.c
+++ b/sound/soc/atmel/atmel-pcm-dma.c
@@ -81,7 +81,9 @@ static void atmel_pcm_dma_irq(u32 ssc_sr,
/* stop RX and capture: will be enabled again at restart */
ssc_writex(prtd->ssc->regs, SSC_CR, prtd->mask->ssc_disable);
+ snd_pcm_stream_lock(substream);
snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock(substream);
/* now drain RHR and read status to remove xrun condition */
ssc_readx(prtd->ssc->regs, SSC_RHR);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (6 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 07/10] ASoC: atmel: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 16:00 ` Mark Brown
2013-07-15 16:02 ` Daniel Glöckner
2013-07-15 13:06 ` [PATCH 09/10] [media] saa7134: " Takashi Iwai
` (2 subsequent siblings)
10 siblings, 2 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/s6000/s6000-pcm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/s6000/s6000-pcm.c b/sound/soc/s6000/s6000-pcm.c
index 1358c7d..3aaf6f6 100644
--- a/sound/soc/s6000/s6000-pcm.c
+++ b/sound/soc/s6000/s6000-pcm.c
@@ -128,7 +128,9 @@ static irqreturn_t s6000_pcm_irq(int irq, void *data)
substream->runtime &&
snd_pcm_running(substream)) {
dev_dbg(pcm->dev, "xrun\n");
+ snd_pcm_stream_lock(substream);
snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+ snd_pcm_substream_unlock(substream);
ret = IRQ_HANDLED;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 09/10] [media] saa7134: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (7 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 08/10] ASoC: s6000: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 13:06 ` [PATCH 10/10] staging: line6: " Takashi Iwai
2013-07-15 13:12 ` [PATCH 0/10] snd_pcm_stop() lock fixes Jaroslav Kysela
10 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/media/pci/saa7134/saa7134-alsa.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c b/drivers/media/pci/saa7134/saa7134-alsa.c
index 10460fd..dbcdfbf 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -172,7 +172,9 @@ static void saa7134_irq_alsa_done(struct saa7134_dev *dev,
dprintk("irq: overrun [full=%d/%d] - Blocks in %d\n",dev->dmasound.read_count,
dev->dmasound.bufsize, dev->dmasound.blocks);
spin_unlock(&dev->slock);
+ snd_pcm_stream_lock(dev->dmasound.substream);
snd_pcm_stop(dev->dmasound.substream,SNDRV_PCM_STATE_XRUN);
+ snd_pcm_stream_unlock(dev->dmasound.substream);
return;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 10/10] staging: line6: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (8 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 09/10] [media] saa7134: " Takashi Iwai
@ 2013-07-15 13:06 ` Takashi Iwai
2013-07-15 13:12 ` [PATCH 0/10] snd_pcm_stop() lock fixes Jaroslav Kysela
10 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:06 UTC (permalink / raw)
To: alsa-devel
snd_pcm_stop() must be called in the PCM substream lock context.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/staging/line6/pcm.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/line6/pcm.c b/drivers/staging/line6/pcm.c
index 02f77d7..a7856ba 100644
--- a/drivers/staging/line6/pcm.c
+++ b/drivers/staging/line6/pcm.c
@@ -385,8 +385,11 @@ static int snd_line6_pcm_free(struct snd_device *device)
*/
static void pcm_disconnect_substream(struct snd_pcm_substream *substream)
{
- if (substream->runtime && snd_pcm_running(substream))
+ if (substream->runtime && snd_pcm_running(substream)) {
+ snd_pcm_stream_lock_irq(substream);
snd_pcm_stop(substream, SNDRV_PCM_STATE_DISCONNECTED);
+ snd_pcm_stream_unlock_irq(substream);
+ }
}
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 0/10] snd_pcm_stop() lock fixes
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
` (9 preceding siblings ...)
2013-07-15 13:06 ` [PATCH 10/10] staging: line6: " Takashi Iwai
@ 2013-07-15 13:12 ` Jaroslav Kysela
2013-07-15 13:17 ` Takashi Iwai
10 siblings, 1 reply; 23+ messages in thread
From: Jaroslav Kysela @ 2013-07-15 13:12 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
Date 15.7.2013 15:06, Takashi Iwai wrote:
> Hi,
>
> while reviewing another patch series, I stumbled on the snd_pcm_lock()
> call in an unlocked context. Then I started looking through the whole
> tree, and found a bunch of drivers doing it wrong, too. So here is a
> patch series to fix them.
>
> [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call
> [PATCH 02/10] ALSA: atiixp: Fix unlocked snd_pcm_stop() call
> [PATCH 03/10] ALSA: 6fire: Fix unlocked snd_pcm_stop() call
> [PATCH 04/10] ALSA: ua101: Fix unlocked snd_pcm_stop() call
> [PATCH 05/10] ALSA: usx2y: Fix unlocked snd_pcm_stop() call
> [PATCH 06/10] ALSA: pxa2xx: Fix unlocked snd_pcm_stop() call
> [PATCH 07/10] ASoC: atmel: Fix unlocked snd_pcm_stop() call
> [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
> [PATCH 09/10] [media] saa7134: Fix unlocked snd_pcm_stop() call
> [PATCH 10/10] staging: line6: Fix unlocked snd_pcm_stop() call
Probably, it may be better to introduce 'snd_pcm_stop_dolock()' helper
(inline?) in include/sound/pcm.h .
Jaroslav
--
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/10] snd_pcm_stop() lock fixes
2013-07-15 13:12 ` [PATCH 0/10] snd_pcm_stop() lock fixes Jaroslav Kysela
@ 2013-07-15 13:17 ` Takashi Iwai
2013-07-16 14:48 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 13:17 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: alsa-devel
At Mon, 15 Jul 2013 15:12:07 +0200,
Jaroslav Kysela wrote:
>
> Date 15.7.2013 15:06, Takashi Iwai wrote:
> > Hi,
> >
> > while reviewing another patch series, I stumbled on the snd_pcm_lock()
> > call in an unlocked context. Then I started looking through the whole
> > tree, and found a bunch of drivers doing it wrong, too. So here is a
> > patch series to fix them.
> >
> > [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call
> > [PATCH 02/10] ALSA: atiixp: Fix unlocked snd_pcm_stop() call
> > [PATCH 03/10] ALSA: 6fire: Fix unlocked snd_pcm_stop() call
> > [PATCH 04/10] ALSA: ua101: Fix unlocked snd_pcm_stop() call
> > [PATCH 05/10] ALSA: usx2y: Fix unlocked snd_pcm_stop() call
> > [PATCH 06/10] ALSA: pxa2xx: Fix unlocked snd_pcm_stop() call
> > [PATCH 07/10] ASoC: atmel: Fix unlocked snd_pcm_stop() call
> > [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
> > [PATCH 09/10] [media] saa7134: Fix unlocked snd_pcm_stop() call
> > [PATCH 10/10] staging: line6: Fix unlocked snd_pcm_stop() call
>
> Probably, it may be better to introduce 'snd_pcm_stop_dolock()' helper
> (inline?) in include/sound/pcm.h .
Yes, it'll be the next step. These patches are done in open-coded way
so that they can be applied easily to each stable kernel.
thanks,
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/10] ALSA: pxa2xx: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 ` [PATCH 06/10] ALSA: pxa2xx: " Takashi Iwai
@ 2013-07-15 16:00 ` Mark Brown
0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2013-07-15 16:00 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 259 bytes --]
On Mon, Jul 15, 2013 at 03:06:18PM +0200, Takashi Iwai wrote:
> snd_pcm_stop() must be called in the PCM substream lock context.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Mark Brown <broonie@linaro.org>
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 07/10] ASoC: atmel: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 ` [PATCH 07/10] ASoC: atmel: " Takashi Iwai
@ 2013-07-15 16:00 ` Mark Brown
0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2013-07-15 16:00 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 259 bytes --]
On Mon, Jul 15, 2013 at 03:06:19PM +0200, Takashi Iwai wrote:
> snd_pcm_stop() must be called in the PCM substream lock context.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Mark Brown <broonie@linaro.org>
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 ` [PATCH 08/10] ASoC: s6000: " Takashi Iwai
@ 2013-07-15 16:00 ` Mark Brown
2013-07-15 16:02 ` Daniel Glöckner
1 sibling, 0 replies; 23+ messages in thread
From: Mark Brown @ 2013-07-15 16:00 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 259 bytes --]
On Mon, Jul 15, 2013 at 03:06:20PM +0200, Takashi Iwai wrote:
> snd_pcm_stop() must be called in the PCM substream lock context.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Mark Brown <broonie@linaro.org>
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 ` [PATCH 08/10] ASoC: s6000: " Takashi Iwai
2013-07-15 16:00 ` Mark Brown
@ 2013-07-15 16:02 ` Daniel Glöckner
2013-07-15 16:05 ` Takashi Iwai
1 sibling, 1 reply; 23+ messages in thread
From: Daniel Glöckner @ 2013-07-15 16:02 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
Nack
On Mon, Jul 15, 2013 at 03:06:20PM +0200, Takashi Iwai wrote:
> --- a/sound/soc/s6000/s6000-pcm.c
> +++ b/sound/soc/s6000/s6000-pcm.c
> @@ -128,7 +128,9 @@ static irqreturn_t s6000_pcm_irq(int irq, void *data)
> substream->runtime &&
> snd_pcm_running(substream)) {
> dev_dbg(pcm->dev, "xrun\n");
> + snd_pcm_stream_lock(substream);
> snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
> + snd_pcm_substream_unlock(substream);
I think this is supposed to be a call to snd_pcm_stream_unlock.
Daniel
--
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Bertha-von-Suttner-Straße 9, 37085 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Dr. Uwe Kracke, Ust-IdNr.: DE 205 198 055
emlix - your embedded linux partner
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
2013-07-15 16:02 ` Daniel Glöckner
@ 2013-07-15 16:05 ` Takashi Iwai
0 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2013-07-15 16:05 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: alsa-devel
At Mon, 15 Jul 2013 18:02:48 +0200,
Daniel Glöckner wrote:
>
> Nack
>
> On Mon, Jul 15, 2013 at 03:06:20PM +0200, Takashi Iwai wrote:
> > --- a/sound/soc/s6000/s6000-pcm.c
> > +++ b/sound/soc/s6000/s6000-pcm.c
> > @@ -128,7 +128,9 @@ static irqreturn_t s6000_pcm_irq(int irq, void *data)
> > substream->runtime &&
> > snd_pcm_running(substream)) {
> > dev_dbg(pcm->dev, "xrun\n");
> > + snd_pcm_stream_lock(substream);
> > snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
> > + snd_pcm_substream_unlock(substream);
>
> I think this is supposed to be a call to snd_pcm_stream_unlock.
Bah, thanks, fixed now.
This didn't go through my build test, obviously...
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 04/10] ALSA: ua101: Fix unlocked snd_pcm_stop() call
2013-07-15 13:06 ` [PATCH 04/10] ALSA: ua101: " Takashi Iwai
@ 2013-07-15 19:01 ` Clemens Ladisch
0 siblings, 0 replies; 23+ messages in thread
From: Clemens Ladisch @ 2013-07-15 19:01 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
Takashi Iwai wrote:
> snd_pcm_stop() must be called in the PCM substream lock context.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/10] snd_pcm_stop() lock fixes
2013-07-15 13:17 ` Takashi Iwai
@ 2013-07-16 14:48 ` Takashi Iwai
2013-07-16 14:54 ` Lars-Peter Clausen
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2013-07-16 14:48 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: alsa-devel
At Mon, 15 Jul 2013 15:17:58 +0200,
Takashi Iwai wrote:
>
> At Mon, 15 Jul 2013 15:12:07 +0200,
> Jaroslav Kysela wrote:
> >
> > Date 15.7.2013 15:06, Takashi Iwai wrote:
> > > Hi,
> > >
> > > while reviewing another patch series, I stumbled on the snd_pcm_lock()
> > > call in an unlocked context. Then I started looking through the whole
> > > tree, and found a bunch of drivers doing it wrong, too. So here is a
> > > patch series to fix them.
> > >
> > > [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call
> > > [PATCH 02/10] ALSA: atiixp: Fix unlocked snd_pcm_stop() call
> > > [PATCH 03/10] ALSA: 6fire: Fix unlocked snd_pcm_stop() call
> > > [PATCH 04/10] ALSA: ua101: Fix unlocked snd_pcm_stop() call
> > > [PATCH 05/10] ALSA: usx2y: Fix unlocked snd_pcm_stop() call
> > > [PATCH 06/10] ALSA: pxa2xx: Fix unlocked snd_pcm_stop() call
> > > [PATCH 07/10] ASoC: atmel: Fix unlocked snd_pcm_stop() call
> > > [PATCH 08/10] ASoC: s6000: Fix unlocked snd_pcm_stop() call
> > > [PATCH 09/10] [media] saa7134: Fix unlocked snd_pcm_stop() call
> > > [PATCH 10/10] staging: line6: Fix unlocked snd_pcm_stop() call
> >
> > Probably, it may be better to introduce 'snd_pcm_stop_dolock()' helper
> > (inline?) in include/sound/pcm.h .
>
> Yes, it'll be the next step. These patches are done in open-coded way
> so that they can be applied easily to each stable kernel.
BTW the conversion I thought of is a patch like below.
Takashi
---
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 84b10f9..96eda75 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -502,6 +502,7 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
struct snd_pcm_status *status);
int snd_pcm_start(struct snd_pcm_substream *substream);
int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
+int snd_pcm_lock_and_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
int snd_pcm_drain_done(struct snd_pcm_substream *substream);
#ifdef CONFIG_PM
int snd_pcm_suspend(struct snd_pcm_substream *substream);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index f928181..56582ac 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -809,9 +809,10 @@ static int snd_pcm_action_lock_irq(struct action_ops *ops,
struct snd_pcm_substream *substream,
int state)
{
+ unsigned long flags;
int res;
- read_lock_irq(&snd_pcm_link_rwlock);
+ read_lock_irqsave(&snd_pcm_link_rwlock, flags);
if (snd_pcm_stream_linked(substream)) {
spin_lock(&substream->group->lock);
spin_lock(&substream->self_group.lock);
@@ -823,7 +824,7 @@ static int snd_pcm_action_lock_irq(struct action_ops *ops,
res = snd_pcm_action_single(ops, substream, state);
spin_unlock(&substream->self_group.lock);
}
- read_unlock_irq(&snd_pcm_link_rwlock);
+ read_unlock_irqrestore(&snd_pcm_link_rwlock, flags);
return res;
}
@@ -900,6 +901,8 @@ static struct action_ops snd_pcm_action_start = {
* @substream: the PCM substream instance
*
* Return: Zero if successful, or a negative error code.
+ *
+ * Note that this function must be called in substream lock context.
*/
int snd_pcm_start(struct snd_pcm_substream *substream)
{
@@ -955,6 +958,8 @@ static struct action_ops snd_pcm_action_stop = {
* The state of each stream is then changed to the given state unconditionally.
*
* Return: Zero if succesful, or a negative error code.
+ *
+ * Note that this function must be called in substream lock context.
*/
int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
{
@@ -964,6 +969,20 @@ int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
EXPORT_SYMBOL(snd_pcm_stop);
/**
+ * snd_pcm_lock_and_stop - lock substream and call snd_pcm_stop()
+ * @substream: the PCM substream instance
+ * @state: PCM state after stopping the stream
+ *
+ * Call this function instead of snd_pcm_stop() when need to stop a
+ * PCM stream outside the stream lock context.
+ */
+int snd_pcm_lock_and_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
+{
+ return snd_pcm_action_lock_irq(&snd_pcm_action_stop, substream, state);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_lock_and_stop);
+
+/**
* snd_pcm_drain_done - stop the DMA only when the given stream is playback
* @substream: the PCM substream
*
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 0/10] snd_pcm_stop() lock fixes
2013-07-16 14:48 ` Takashi Iwai
@ 2013-07-16 14:54 ` Lars-Peter Clausen
2013-07-16 15:10 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-07-16 14:54 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On 07/16/2013 04:48 PM, Takashi Iwai wrote:
[...]
> +int snd_pcm_lock_and_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
The name makes it sound as if the lock will still be held after the function
call has completed. How about snd_pcm_stop_locked()?
- Lars
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/10] snd_pcm_stop() lock fixes
2013-07-16 14:54 ` Lars-Peter Clausen
@ 2013-07-16 15:10 ` Takashi Iwai
2013-07-16 15:57 ` Lars-Peter Clausen
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2013-07-16 15:10 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: alsa-devel
At Tue, 16 Jul 2013 16:54:14 +0200,
Lars-Peter Clausen wrote:
>
> On 07/16/2013 04:48 PM, Takashi Iwai wrote:
> [...]
> > +int snd_pcm_lock_and_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
>
> The name makes it sound as if the lock will still be held after the function
> call has completed.
Yeah, I agree with my bad sense of naming...
> How about snd_pcm_stop_locked()?
Hrm, it's also unclear whether it's called in a locked context or it
does lock by itself.
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/10] snd_pcm_stop() lock fixes
2013-07-16 15:10 ` Takashi Iwai
@ 2013-07-16 15:57 ` Lars-Peter Clausen
0 siblings, 0 replies; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-07-16 15:57 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On 07/16/2013 05:10 PM, Takashi Iwai wrote:
> At Tue, 16 Jul 2013 16:54:14 +0200,
> Lars-Peter Clausen wrote:
>>
>> On 07/16/2013 04:48 PM, Takashi Iwai wrote:
>> [...]
>>> +int snd_pcm_lock_and_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
>>
>> The name makes it sound as if the lock will still be held after the function
>> call has completed.
>
> Yeah, I agree with my bad sense of naming...
>
>> How about snd_pcm_stop_locked()?
>
> Hrm, it's also unclear whether it's called in a locked context or it
> does lock by itself.
Hm, I always thought that ..._locked() was mainly used for functions that
take the lock, but a quick grep revealed that both meanings are used and
it's split about 50/50 :/
- Lars
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-07-16 15:56 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-15 13:06 [PATCH 0/10] snd_pcm_stop() lock fixes Takashi Iwai
2013-07-15 13:06 ` [PATCH 01/10] ALSA: asihpi: Fix unlocked snd_pcm_stop() call Takashi Iwai
2013-07-15 13:06 ` [PATCH 02/10] ALSA: atiixp: " Takashi Iwai
2013-07-15 13:06 ` [PATCH 03/10] ALSA: 6fire: " Takashi Iwai
2013-07-15 13:06 ` [PATCH 04/10] ALSA: ua101: " Takashi Iwai
2013-07-15 19:01 ` Clemens Ladisch
2013-07-15 13:06 ` [PATCH 05/10] ALSA: usx2y: " Takashi Iwai
2013-07-15 13:06 ` [PATCH 06/10] ALSA: pxa2xx: " Takashi Iwai
2013-07-15 16:00 ` Mark Brown
2013-07-15 13:06 ` [PATCH 07/10] ASoC: atmel: " Takashi Iwai
2013-07-15 16:00 ` Mark Brown
2013-07-15 13:06 ` [PATCH 08/10] ASoC: s6000: " Takashi Iwai
2013-07-15 16:00 ` Mark Brown
2013-07-15 16:02 ` Daniel Glöckner
2013-07-15 16:05 ` Takashi Iwai
2013-07-15 13:06 ` [PATCH 09/10] [media] saa7134: " Takashi Iwai
2013-07-15 13:06 ` [PATCH 10/10] staging: line6: " Takashi Iwai
2013-07-15 13:12 ` [PATCH 0/10] snd_pcm_stop() lock fixes Jaroslav Kysela
2013-07-15 13:17 ` Takashi Iwai
2013-07-16 14:48 ` Takashi Iwai
2013-07-16 14:54 ` Lars-Peter Clausen
2013-07-16 15:10 ` Takashi Iwai
2013-07-16 15:57 ` Lars-Peter Clausen
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.