* [PATCH 0/4] Internal PCM fixes / cleanups
@ 2015-02-20 16:23 Takashi Iwai
2015-02-20 16:23 ` [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream() Takashi Iwai
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 16:23 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Vinod Koul
Hi,
while reading the internal PCM (BE) handling code again, I noticed a
few potential bugs. Here are fix and cleanup patches.
Takashi
===
Takashi Iwai (4):
ALSA: pcm: Minor refactoring in snd_pcm_attach_substream()
ALSA: pcm: Don't add internal PCMs to PCM device list
ALSA: pcm: Don't notify internal PCMs
ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect()
sound/core/pcm.c | 92 +++++++++++++++++++++++++-------------------------------
1 file changed, 41 insertions(+), 51 deletions(-)
--
2.3.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream()
2015-02-20 16:23 [PATCH 0/4] Internal PCM fixes / cleanups Takashi Iwai
@ 2015-02-20 16:23 ` Takashi Iwai
2015-02-20 18:52 ` Takashi Iwai
2015-02-20 16:23 ` [PATCH 2/4] ALSA: pcm: Don't add internal PCMs to PCM device list Takashi Iwai
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 16:23 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Vinod Koul
No functional changes at all.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/core/pcm.c | 41 +++++++++++++++--------------------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 0345e53a340c..89206e9c3578 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -919,6 +919,9 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
if (snd_BUG_ON(!pcm || !rsubstream))
return -ENXIO;
+ if (snd_BUG_ON(stream != SNDRV_PCM_STREAM_PLAYBACK ||
+ stream != SNDRV_PCM_STREAM_CAPTURE))
+ return -EINVAL;
*rsubstream = NULL;
pstr = &pcm->streams[stream];
if (pstr->substream == NULL || pstr->substream_count == 0)
@@ -927,25 +930,14 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
card = pcm->card;
prefer_subdevice = snd_ctl_get_preferred_subdevice(card, SND_CTL_SUBDEV_PCM);
- switch (stream) {
- case SNDRV_PCM_STREAM_PLAYBACK:
- if (pcm->info_flags & SNDRV_PCM_INFO_HALF_DUPLEX) {
- for (substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream; substream; substream = substream->next) {
- if (SUBSTREAM_BUSY(substream))
- return -EAGAIN;
- }
- }
- break;
- case SNDRV_PCM_STREAM_CAPTURE:
- if (pcm->info_flags & SNDRV_PCM_INFO_HALF_DUPLEX) {
- for (substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream; substream; substream = substream->next) {
- if (SUBSTREAM_BUSY(substream))
- return -EAGAIN;
- }
+ if (pcm->info_flags & SNDRV_PCM_INFO_HALF_DUPLEX) {
+ int opposite = !stream;
+
+ for (substream = pcm->streams[opposite].substream; substream;
+ substream = substream->next) {
+ if (SUBSTREAM_BUSY(substream))
+ return -EAGAIN;
}
- break;
- default:
- return -EINVAL;
}
if (file->f_flags & O_APPEND) {
@@ -968,15 +960,12 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
return 0;
}
- if (prefer_subdevice >= 0) {
- for (substream = pstr->substream; substream; substream = substream->next)
- if (!SUBSTREAM_BUSY(substream) && substream->number == prefer_subdevice)
- goto __ok;
- }
- for (substream = pstr->substream; substream; substream = substream->next)
- if (!SUBSTREAM_BUSY(substream))
+ for (substream = pstr->substream; substream; substream = substream->next) {
+ if (!SUBSTREAM_BUSY(substream) &&
+ (prefer_subdevice == -1 ||
+ substream->number == prefer_subdevice))
break;
- __ok:
+ }
if (substream == NULL)
return -EAGAIN;
--
2.3.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] ALSA: pcm: Don't add internal PCMs to PCM device list
2015-02-20 16:23 [PATCH 0/4] Internal PCM fixes / cleanups Takashi Iwai
2015-02-20 16:23 ` [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream() Takashi Iwai
@ 2015-02-20 16:23 ` Takashi Iwai
2015-02-20 16:23 ` [PATCH 3/4] ALSA: pcm: Don't notify internal PCMs Takashi Iwai
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 16:23 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Vinod Koul
An internal PCM object shouldn't be added to the PCM device list, as
it's never accessed directly from the user-space, and it has no proc
or any similar accesses. Currently, it's excluded in snd_pcm_get()
and snd_pcm_next(), but it's easier not to add such an object to the
list.
Actually, the whole snd_pcm_dev_register() can be skipped for an
internal PCM. So this patch changes the code there, but also
addresses the uninitialized list_head access.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/core/pcm.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 89206e9c3578..92bbb5143b83 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -49,8 +49,6 @@ static struct snd_pcm *snd_pcm_get(struct snd_card *card, int device)
struct snd_pcm *pcm;
list_for_each_entry(pcm, &snd_pcm_devices, list) {
- if (pcm->internal)
- continue;
if (pcm->card == card && pcm->device == device)
return pcm;
}
@@ -62,8 +60,6 @@ static int snd_pcm_next(struct snd_card *card, int device)
struct snd_pcm *pcm;
list_for_each_entry(pcm, &snd_pcm_devices, list) {
- if (pcm->internal)
- continue;
if (pcm->card == card && pcm->device > device)
return pcm->device;
else if (pcm->card->number > card->number)
@@ -76,6 +72,9 @@ static int snd_pcm_add(struct snd_pcm *newpcm)
{
struct snd_pcm *pcm;
+ if (newpcm->internal)
+ return 0;
+
list_for_each_entry(pcm, &snd_pcm_devices, list) {
if (pcm->card == newpcm->card && pcm->device == newpcm->device)
return -EBUSY;
@@ -782,6 +781,9 @@ static int _snd_pcm_new(struct snd_card *card, const char *id, int device,
pcm->card = card;
pcm->device = device;
pcm->internal = internal;
+ mutex_init(&pcm->open_mutex);
+ init_waitqueue_head(&pcm->open_wait);
+ INIT_LIST_HEAD(&pcm->list);
if (id)
strlcpy(pcm->id, id, sizeof(pcm->id));
if ((err = snd_pcm_new_stream(pcm, SNDRV_PCM_STREAM_PLAYBACK, playback_count)) < 0) {
@@ -792,8 +794,6 @@ static int _snd_pcm_new(struct snd_card *card, const char *id, int device,
snd_pcm_free(pcm);
return err;
}
- mutex_init(&pcm->open_mutex);
- init_waitqueue_head(&pcm->open_wait);
if ((err = snd_device_new(card, SNDRV_DEV_PCM, pcm, &ops)) < 0) {
snd_pcm_free(pcm);
return err;
@@ -1075,15 +1075,16 @@ static int snd_pcm_dev_register(struct snd_device *device)
if (snd_BUG_ON(!device || !device->device_data))
return -ENXIO;
pcm = device->device_data;
+ if (pcm->internal)
+ return 0;
+
mutex_lock(®ister_mutex);
err = snd_pcm_add(pcm);
- if (err) {
- mutex_unlock(®ister_mutex);
- return err;
- }
+ if (err)
+ goto unlock;
for (cidx = 0; cidx < 2; cidx++) {
int devtype = -1;
- if (pcm->streams[cidx].substream == NULL || pcm->internal)
+ if (pcm->streams[cidx].substream == NULL)
continue;
switch (cidx) {
case SNDRV_PCM_STREAM_PLAYBACK:
@@ -1098,9 +1099,8 @@ static int snd_pcm_dev_register(struct snd_device *device)
&snd_pcm_f_ops[cidx], pcm,
&pcm->streams[cidx].dev);
if (err < 0) {
- list_del(&pcm->list);
- mutex_unlock(®ister_mutex);
- return err;
+ list_del_init(&pcm->list);
+ goto unlock;
}
for (substream = pcm->streams[cidx].substream; substream; substream = substream->next)
@@ -1110,8 +1110,9 @@ static int snd_pcm_dev_register(struct snd_device *device)
list_for_each_entry(notify, &snd_pcm_notify_list, list)
notify->n_register(pcm);
+ unlock:
mutex_unlock(®ister_mutex);
- return 0;
+ return err;
}
static int snd_pcm_dev_disconnect(struct snd_device *device)
--
2.3.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] ALSA: pcm: Don't notify internal PCMs
2015-02-20 16:23 [PATCH 0/4] Internal PCM fixes / cleanups Takashi Iwai
2015-02-20 16:23 ` [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream() Takashi Iwai
2015-02-20 16:23 ` [PATCH 2/4] ALSA: pcm: Don't add internal PCMs to PCM device list Takashi Iwai
@ 2015-02-20 16:23 ` Takashi Iwai
2015-02-20 16:23 ` [PATCH 4/4] ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect() Takashi Iwai
2015-02-20 18:42 ` [PATCH 0/4] Internal PCM fixes / cleanups Liam Girdwood
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 16:23 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Vinod Koul
Notifier shouldn't listen to the changes of internal PCMs.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/core/pcm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 92bbb5143b83..3cf345426171 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -888,8 +888,9 @@ static int snd_pcm_free(struct snd_pcm *pcm)
if (!pcm)
return 0;
- list_for_each_entry(notify, &snd_pcm_notify_list, list) {
- notify->n_unregister(pcm);
+ if (!pcm->internal) {
+ list_for_each_entry(notify, &snd_pcm_notify_list, list)
+ notify->n_unregister(pcm);
}
if (pcm->private_free)
pcm->private_free(pcm);
@@ -1129,7 +1130,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
mutex_lock(&pcm->open_mutex);
wake_up(&pcm->open_wait);
list_del_init(&pcm->list);
- for (cidx = 0; cidx < 2; cidx++)
+ for (cidx = 0; cidx < 2; cidx++) {
for (substream = pcm->streams[cidx].substream; substream; substream = substream->next) {
snd_pcm_stream_lock_irq(substream);
if (substream->runtime) {
@@ -1139,8 +1140,10 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
}
snd_pcm_stream_unlock_irq(substream);
}
- list_for_each_entry(notify, &snd_pcm_notify_list, list) {
- notify->n_disconnect(pcm);
+ }
+ if (!pcm->internal) {
+ list_for_each_entry(notify, &snd_pcm_notify_list, list)
+ notify->n_disconnect(pcm);
}
for (cidx = 0; cidx < 2; cidx++) {
snd_unregister_device(&pcm->streams[cidx].dev);
--
2.3.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect()
2015-02-20 16:23 [PATCH 0/4] Internal PCM fixes / cleanups Takashi Iwai
` (2 preceding siblings ...)
2015-02-20 16:23 ` [PATCH 3/4] ALSA: pcm: Don't notify internal PCMs Takashi Iwai
@ 2015-02-20 16:23 ` Takashi Iwai
2015-02-20 18:42 ` [PATCH 0/4] Internal PCM fixes / cleanups Liam Girdwood
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 16:23 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Vinod Koul
Some codes in snd_pcm_dev_disconnect() are still valid even for
internal PCMs, but they are skipped because of the check of
list_empty(&pcm->list) at the beginning. Remove this check and put
pcm->internal checks appropriately for internal PCM object to process
through this function.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/core/pcm.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 3cf345426171..64e726658ca1 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -1124,9 +1124,6 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
int cidx;
mutex_lock(®ister_mutex);
- if (list_empty(&pcm->list))
- goto unlock;
-
mutex_lock(&pcm->open_mutex);
wake_up(&pcm->open_wait);
list_del_init(&pcm->list);
@@ -1146,14 +1143,14 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
notify->n_disconnect(pcm);
}
for (cidx = 0; cidx < 2; cidx++) {
- snd_unregister_device(&pcm->streams[cidx].dev);
+ if (!pcm->internal)
+ snd_unregister_device(&pcm->streams[cidx].dev);
if (pcm->streams[cidx].chmap_kctl) {
snd_ctl_remove(pcm->card, pcm->streams[cidx].chmap_kctl);
pcm->streams[cidx].chmap_kctl = NULL;
}
}
mutex_unlock(&pcm->open_mutex);
- unlock:
mutex_unlock(®ister_mutex);
return 0;
}
--
2.3.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] Internal PCM fixes / cleanups
2015-02-20 16:23 [PATCH 0/4] Internal PCM fixes / cleanups Takashi Iwai
` (3 preceding siblings ...)
2015-02-20 16:23 ` [PATCH 4/4] ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect() Takashi Iwai
@ 2015-02-20 18:42 ` Liam Girdwood
2015-02-20 18:53 ` Takashi Iwai
4 siblings, 1 reply; 8+ messages in thread
From: Liam Girdwood @ 2015-02-20 18:42 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Vinod Koul, alsa-devel
On Fri, 2015-02-20 at 17:23 +0100, Takashi Iwai wrote:
> Hi,
>
> while reading the internal PCM (BE) handling code again, I noticed a
> few potential bugs. Here are fix and cleanup patches.
>
>
> Takashi
>
> ===
>
> Takashi Iwai (4):
> ALSA: pcm: Minor refactoring in snd_pcm_attach_substream()
> ALSA: pcm: Don't add internal PCMs to PCM device list
> ALSA: pcm: Don't notify internal PCMs
> ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect()
>
> sound/core/pcm.c | 92 +++++++++++++++++++++++++-------------------------------
> 1 file changed, 41 insertions(+), 51 deletions(-)
>
All
Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream()
2015-02-20 16:23 ` [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream() Takashi Iwai
@ 2015-02-20 18:52 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 18:52 UTC (permalink / raw)
To: alsa-devel; +Cc: Liam Girdwood, Vinod Koul
At Fri, 20 Feb 2015 17:23:25 +0100,
Takashi Iwai wrote:
>
> No functional changes at all.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> sound/core/pcm.c | 41 +++++++++++++++--------------------------
> 1 file changed, 15 insertions(+), 26 deletions(-)
>
> diff --git a/sound/core/pcm.c b/sound/core/pcm.c
> index 0345e53a340c..89206e9c3578 100644
> --- a/sound/core/pcm.c
> +++ b/sound/core/pcm.c
> @@ -919,6 +919,9 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
>
> if (snd_BUG_ON(!pcm || !rsubstream))
> return -ENXIO;
> + if (snd_BUG_ON(stream != SNDRV_PCM_STREAM_PLAYBACK ||
> + stream != SNDRV_PCM_STREAM_CAPTURE))
Gah, this must be &&. Fixed in the topic/pcm-internal branch.
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] Internal PCM fixes / cleanups
2015-02-20 18:42 ` [PATCH 0/4] Internal PCM fixes / cleanups Liam Girdwood
@ 2015-02-20 18:53 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2015-02-20 18:53 UTC (permalink / raw)
To: Liam Girdwood; +Cc: Vinod Koul, alsa-devel
At Fri, 20 Feb 2015 18:42:23 +0000,
Liam Girdwood wrote:
>
> On Fri, 2015-02-20 at 17:23 +0100, Takashi Iwai wrote:
> > Hi,
> >
> > while reading the internal PCM (BE) handling code again, I noticed a
> > few potential bugs. Here are fix and cleanup patches.
> >
> >
> > Takashi
> >
> > ===
> >
> > Takashi Iwai (4):
> > ALSA: pcm: Minor refactoring in snd_pcm_attach_substream()
> > ALSA: pcm: Don't add internal PCMs to PCM device list
> > ALSA: pcm: Don't notify internal PCMs
> > ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect()
> >
> > sound/core/pcm.c | 92 +++++++++++++++++++++++++-------------------------------
> > 1 file changed, 41 insertions(+), 51 deletions(-)
> >
>
> All
>
> Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Thanks, added your ack to topic/pcm-internal branch.
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-02-20 18:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-20 16:23 [PATCH 0/4] Internal PCM fixes / cleanups Takashi Iwai
2015-02-20 16:23 ` [PATCH 1/4] ALSA: pcm: Minor refactoring in snd_pcm_attach_substream() Takashi Iwai
2015-02-20 18:52 ` Takashi Iwai
2015-02-20 16:23 ` [PATCH 2/4] ALSA: pcm: Don't add internal PCMs to PCM device list Takashi Iwai
2015-02-20 16:23 ` [PATCH 3/4] ALSA: pcm: Don't notify internal PCMs Takashi Iwai
2015-02-20 16:23 ` [PATCH 4/4] ALSA: pcm: Don't ignore internal PCMs in snd_pcm_dev_disconnect() Takashi Iwai
2015-02-20 18:42 ` [PATCH 0/4] Internal PCM fixes / cleanups Liam Girdwood
2015-02-20 18:53 ` Takashi Iwai
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.