Commit c037239c authored by Andrew Gabbasov's avatar Andrew Gabbasov Committed by Takashi Iwai

ALSA: aloop: Remove redundant locking in timer open function

loopback_parse_timer_id() uses snd_card_ref(), that can lock on mutex,
also snd_timer_instance_new() uses non-atomic allocation, that can sleep.
So, both functions can not be called from loopback_snd_timer_open()
with cable->lock spinlock locked.

Moreover, most part of loopback_snd_timer_open() function body works
when the opposite stream of the same cable does not yet exist, and
the current stream is not yet completely open and can't be running,
so existing locking of loopback->cable_lock mutex is enough to protect
from conflicts with simultaneous opening or closing.
Locking of cable->lock spinlock is not needed in this case.

Fixes: 26c53379 ("ALSA: aloop: Support selection of snd_timer instead of jiffies")
Signed-off-by: default avatarAndrew Gabbasov <andrew_gabbasov@mentor.com>
Link: https://lore.kernel.org/r/20191122175218.17187-1-andrew_gabbasov@mentor.comSigned-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 1e5ddb6b
...@@ -1107,20 +1107,18 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm) ...@@ -1107,20 +1107,18 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm)
struct snd_timer_instance *timeri; struct snd_timer_instance *timeri;
struct loopback_cable *cable = dpcm->cable; struct loopback_cable *cable = dpcm->cable;
spin_lock_irq(&cable->lock);
/* check if timer was already opened. It is only opened once /* check if timer was already opened. It is only opened once
* per playback and capture subdevice (aka cable). * per playback and capture subdevice (aka cable).
*/ */
if (cable->snd_timer.instance) if (cable->snd_timer.instance)
goto unlock; goto exit;
err = loopback_parse_timer_id(dpcm->loopback->timer_source, &tid); err = loopback_parse_timer_id(dpcm->loopback->timer_source, &tid);
if (err < 0) { if (err < 0) {
pcm_err(dpcm->substream->pcm, pcm_err(dpcm->substream->pcm,
"Parsing timer source \'%s\' failed with %d", "Parsing timer source \'%s\' failed with %d",
dpcm->loopback->timer_source, err); dpcm->loopback->timer_source, err);
goto unlock; goto exit;
} }
cable->snd_timer.stream = dpcm->substream->stream; cable->snd_timer.stream = dpcm->substream->stream;
...@@ -1129,7 +1127,7 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm) ...@@ -1129,7 +1127,7 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm)
timeri = snd_timer_instance_new(dpcm->loopback->card->id); timeri = snd_timer_instance_new(dpcm->loopback->card->id);
if (!timeri) { if (!timeri) {
err = -ENOMEM; err = -ENOMEM;
goto unlock; goto exit;
} }
/* The callback has to be called from another tasklet. If /* The callback has to be called from another tasklet. If
* SNDRV_TIMER_IFLG_FAST is specified it will be called from the * SNDRV_TIMER_IFLG_FAST is specified it will be called from the
...@@ -1148,10 +1146,9 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm) ...@@ -1148,10 +1146,9 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm)
tasklet_init(&cable->snd_timer.event_tasklet, tasklet_init(&cable->snd_timer.event_tasklet,
loopback_snd_timer_tasklet, (unsigned long)timeri); loopback_snd_timer_tasklet, (unsigned long)timeri);
/* snd_timer_close() and snd_timer_open() should not be called with /* The mutex loopback->cable_lock is kept locked.
* locked spinlock because both functions can block on a mutex. The * Therefore snd_timer_open() cannot be called a second time
* mutex loopback->cable_lock is kept locked. Therefore snd_timer_open() * by the other device of the same cable.
* cannot be called a second time by the other device of the same cable.
* Therefore the following issue cannot happen: * Therefore the following issue cannot happen:
* [proc1] Call loopback_timer_open() -> * [proc1] Call loopback_timer_open() ->
* Unlock cable->lock for snd_timer_close/open() call * Unlock cable->lock for snd_timer_close/open() call
...@@ -1160,9 +1157,7 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm) ...@@ -1160,9 +1157,7 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm)
* [proc1] Call snd_timer_open() and overwrite running timer * [proc1] Call snd_timer_open() and overwrite running timer
* instance * instance
*/ */
spin_unlock_irq(&cable->lock);
err = snd_timer_open(timeri, &cable->snd_timer.id, current->pid); err = snd_timer_open(timeri, &cable->snd_timer.id, current->pid);
spin_lock_irq(&cable->lock);
if (err < 0) { if (err < 0) {
pcm_err(dpcm->substream->pcm, pcm_err(dpcm->substream->pcm,
"snd_timer_open (%d,%d,%d) failed with %d", "snd_timer_open (%d,%d,%d) failed with %d",
...@@ -1171,14 +1166,12 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm) ...@@ -1171,14 +1166,12 @@ static int loopback_snd_timer_open(struct loopback_pcm *dpcm)
cable->snd_timer.id.subdevice, cable->snd_timer.id.subdevice,
err); err);
snd_timer_instance_free(timeri); snd_timer_instance_free(timeri);
goto unlock; goto exit;
} }
cable->snd_timer.instance = timeri; cable->snd_timer.instance = timeri;
unlock: exit:
spin_unlock_irq(&cable->lock);
return err; return err;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment