Skip to content

L
linux
Commit f65e0d29 authored by Takashi Iwai's avatar Takashi Iwai
Browse files
Options

ALSA: timer: Call notifier in the same spinlock 

snd_timer_notify1() is called outside the spinlock and it retakes the
lock after the unlock.  This is rather racy, and it's safer to move
snd_timer_notify() call inside the main spinlock.

The patch also contains a slight refactoring / cleanup of the code.
Now all start/stop/continue/pause look more symmetric and a bit better
readable.
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 9984d1b5
Hide whitespace changes
Inline Side-by-side
Showing with 102 additions and 118 deletions
sound/core/timer.c
View file @ f65e0d29
...@@ -305,8 +305,6 @@ int snd_timer_open(struct snd_timer_instance **ti, ...@@ -305,8 +305,6 @@ int snd_timer_open(struct snd_timer_instance **ti,
return 0; return 0;
} }
static int _snd_timer_stop(struct snd_timer_instance *timeri, int event);
/* /*
* close a timer instance * close a timer instance
*/ */
...@@ -389,7 +387,6 @@ unsigned long snd_timer_resolution(struct snd_timer_instance *timeri) ...@@ -389,7 +387,6 @@ unsigned long snd_timer_resolution(struct snd_timer_instance *timeri)
static void snd_timer_notify1(struct snd_timer_instance *ti, int event) static void snd_timer_notify1(struct snd_timer_instance *ti, int event)
{ {
struct snd_timer *timer; struct snd_timer *timer;
unsigned long flags;
unsigned long resolution = 0; unsigned long resolution = 0;
struct snd_timer_instance *ts; struct snd_timer_instance *ts;
struct timespec tstamp; struct timespec tstamp;
...@@ -413,34 +410,66 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event) ...@@ -413,34 +410,66 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event)
return; return;
if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE) if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE)
return; return;
spin_lock_irqsave(&timer->lock, flags);
list_for_each_entry(ts, &ti->slave_active_head, active_list) list_for_each_entry(ts, &ti->slave_active_head, active_list)
if (ts->ccallback) if (ts->ccallback)
ts->ccallback(ts, event + 100, &tstamp, resolution); ts->ccallback(ts, event + 100, &tstamp, resolution);
spin_unlock_irqrestore(&timer->lock, flags);
} }
static int snd_timer_start1(struct snd_timer *timer, struct snd_timer_instance *timeri, /* start/continue a master timer */
unsigned long sticks) static int snd_timer_start1(struct snd_timer_instance *timeri,
bool start, unsigned long ticks)
{ {
struct snd_timer *timer;
int result;
unsigned long flags;
timer = timeri->timer;
if (!timer)
return -EINVAL;
spin_lock_irqsave(&timer->lock, flags);
if (timer->card && timer->card->shutdown) {
result = -ENODEV;
goto unlock;
}
if (timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |
SNDRV_TIMER_IFLG_START)) {
result = -EBUSY;
goto unlock;
}
if (start)
timeri->ticks = timeri->cticks = ticks;
else if (!timeri->cticks)
timeri->cticks = 1;
timeri->pticks = 0;
list_move_tail(&timeri->active_list, &timer->active_list_head); list_move_tail(&timeri->active_list, &timer->active_list_head);
if (timer->running) { if (timer->running) {
if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE) if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE)
goto __start_now; goto __start_now;
timer->flags |= SNDRV_TIMER_FLG_RESCHED; timer->flags |= SNDRV_TIMER_FLG_RESCHED;
timeri->flags |= SNDRV_TIMER_IFLG_START; timeri->flags |= SNDRV_TIMER_IFLG_START;
return 1; /* delayed start */ result = 1; /* delayed start */
} else { } else {
timer->sticks = sticks; if (start)
timer->sticks = ticks;
timer->hw.start(timer); timer->hw.start(timer);
__start_now: __start_now:
timer->running++; timer->running++;
timeri->flags |= SNDRV_TIMER_IFLG_RUNNING; timeri->flags |= SNDRV_TIMER_IFLG_RUNNING;
return 0; result = 0;
} }
snd_timer_notify1(timeri, start ? SNDRV_TIMER_EVENT_START :
SNDRV_TIMER_EVENT_CONTINUE);
unlock:
spin_unlock_irqrestore(&timer->lock, flags);
return result;
} }
static int snd_timer_start_slave(struct snd_timer_instance *timeri) /* start/continue a slave timer */
static int snd_timer_start_slave(struct snd_timer_instance *timeri,
bool start)
{ {
unsigned long flags; unsigned long flags;
...@@ -454,88 +483,37 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri) ...@@ -454,88 +483,37 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri)
spin_lock(&timeri->timer->lock); spin_lock(&timeri->timer->lock);
list_add_tail(&timeri->active_list, list_add_tail(&timeri->active_list,
&timeri->master->slave_active_head); &timeri->master->slave_active_head);
snd_timer_notify1(timeri, start ? SNDRV_TIMER_EVENT_START :
SNDRV_TIMER_EVENT_CONTINUE);
spin_unlock(&timeri->timer->lock); spin_unlock(&timeri->timer->lock);
} }
spin_unlock_irqrestore(&slave_active_lock, flags); spin_unlock_irqrestore(&slave_active_lock, flags);
return 1; /* delayed start */ return 1; /* delayed start */
} }
/* /* stop/pause a master timer */
* start the timer instance static int snd_timer_stop1(struct snd_timer_instance *timeri, bool stop)
*/
int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
{
struct snd_timer *timer;
int result = -EINVAL;
unsigned long flags;
if (timeri == NULL || ticks < 1)
return -EINVAL;
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
result = snd_timer_start_slave(timeri);
if (result >= 0)
snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
return result;
}
timer = timeri->timer;
if (timer == NULL)
return -EINVAL;
if (timer->card && timer->card->shutdown)
return -ENODEV;
spin_lock_irqsave(&timer->lock, flags);
if (timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |
SNDRV_TIMER_IFLG_START)) {
result = -EBUSY;
goto unlock;
}
timeri->ticks = timeri->cticks = ticks;
timeri->pticks = 0;
result = snd_timer_start1(timer, timeri, ticks);
unlock:
spin_unlock_irqrestore(&timer->lock, flags);
if (result >= 0)
snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
return result;
}
static int _snd_timer_stop(struct snd_timer_instance *timeri, int event)
{ {
struct snd_timer *timer; struct snd_timer *timer;
int result = 0;
unsigned long flags; unsigned long flags;
if (snd_BUG_ON(!timeri))
return -ENXIO;
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
spin_lock_irqsave(&slave_active_lock, flags);
if (!(timeri->flags & SNDRV_TIMER_IFLG_RUNNING)) {
spin_unlock_irqrestore(&slave_active_lock, flags);
return -EBUSY;
}
if (timeri->timer)
spin_lock(&timeri->timer->lock);
timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
list_del_init(&timeri->ack_list);
list_del_init(&timeri->active_list);
if (timeri->timer)
spin_unlock(&timeri->timer->lock);
spin_unlock_irqrestore(&slave_active_lock, flags);
goto __end;
}
timer = timeri->timer; timer = timeri->timer;
if (!timer) if (!timer)
return -EINVAL; return -EINVAL;
spin_lock_irqsave(&timer->lock, flags); spin_lock_irqsave(&timer->lock, flags);
if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING | if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |
SNDRV_TIMER_IFLG_START))) { SNDRV_TIMER_IFLG_START))) {
spin_unlock_irqrestore(&timer->lock, flags); result = -EBUSY;
return -EBUSY; goto unlock;
} }
list_del_init(&timeri->ack_list); list_del_init(&timeri->ack_list);
list_del_init(&timeri->active_list); list_del_init(&timeri->active_list);
if (timer->card && timer->card->shutdown) { if (timer->card && timer->card->shutdown)
spin_unlock_irqrestore(&timer->lock, flags); goto unlock;
return 0; if (stop) {
timeri->cticks = timeri->ticks;
timeri->pticks = 0;
} }
if ((timeri->flags & SNDRV_TIMER_IFLG_RUNNING) && if ((timeri->flags & SNDRV_TIMER_IFLG_RUNNING) &&
!(--timer->running)) { !(--timer->running)) {
...@@ -550,13 +528,49 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) ...@@ -550,13 +528,49 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event)
} }
} }
timeri->flags &= ~(SNDRV_TIMER_IFLG_RUNNING | SNDRV_TIMER_IFLG_START); timeri->flags &= ~(SNDRV_TIMER_IFLG_RUNNING | SNDRV_TIMER_IFLG_START);
snd_timer_notify1(timeri, stop ? SNDRV_TIMER_EVENT_STOP :
SNDRV_TIMER_EVENT_CONTINUE);
unlock:
spin_unlock_irqrestore(&timer->lock, flags); spin_unlock_irqrestore(&timer->lock, flags);
__end: return result;
if (event != SNDRV_TIMER_EVENT_RESOLUTION) }
snd_timer_notify1(timeri, event);
/* stop/pause a slave timer */
static int snd_timer_stop_slave(struct snd_timer_instance *timeri, bool stop)
{
unsigned long flags;
spin_lock_irqsave(&slave_active_lock, flags);
if (!(timeri->flags & SNDRV_TIMER_IFLG_RUNNING)) {
spin_unlock_irqrestore(&slave_active_lock, flags);
return -EBUSY;
}
timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
if (timeri->timer) {
spin_lock(&timeri->timer->lock);
list_del_init(&timeri->ack_list);
list_del_init(&timeri->active_list);
snd_timer_notify1(timeri, stop ? SNDRV_TIMER_EVENT_STOP :
SNDRV_TIMER_EVENT_CONTINUE);
spin_unlock(&timeri->timer->lock);
}
spin_unlock_irqrestore(&slave_active_lock, flags);
return 0; return 0;
} }
/*
* start the timer instance
*/
int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
{
if (timeri == NULL || ticks < 1)
return -EINVAL;
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE)
return snd_timer_start_slave(timeri, true);
else
return snd_timer_start1(timeri, true, ticks);
}
/* /*
* stop the timer instance. * stop the timer instance.
* *
...@@ -564,21 +578,10 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) ...@@ -564,21 +578,10 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event)
*/ */
int snd_timer_stop(struct snd_timer_instance *timeri) int snd_timer_stop(struct snd_timer_instance *timeri)
{ {
struct snd_timer *timer; if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE)
unsigned long flags; return snd_timer_stop_slave(timeri, true);
int err; else
return snd_timer_stop1(timeri, true);
err = _snd_timer_stop(timeri, SNDRV_TIMER_EVENT_STOP);
if (err < 0)
return err;
timer = timeri->timer;
if (!timer)
return -EINVAL;
spin_lock_irqsave(&timer->lock, flags);
timeri->cticks = timeri->ticks;
timeri->pticks = 0;
spin_unlock_irqrestore(&timer->lock, flags);
return 0;
} }
/* /*
...@@ -586,32 +589,10 @@ int snd_timer_stop(struct snd_timer_instance *timeri) ...@@ -586,32 +589,10 @@ int snd_timer_stop(struct snd_timer_instance *timeri)
*/ */
int snd_timer_continue(struct snd_timer_instance *timeri) int snd_timer_continue(struct snd_timer_instance *timeri)
{ {
struct snd_timer *timer;
int result = -EINVAL;
unsigned long flags;
if (timeri == NULL)
return result;
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE)
return snd_timer_start_slave(timeri); return snd_timer_start_slave(timeri, false);
timer = timeri->timer; else
if (! timer) return snd_timer_start1(timeri, false, 0);
return -EINVAL;
if (timer->card && timer->card->shutdown)
return -ENODEV;
spin_lock_irqsave(&timer->lock, flags);
if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) {
result = -EBUSY;
goto unlock;
}
if (!timeri->cticks)
timeri->cticks = 1;
timeri->pticks = 0;
result = snd_timer_start1(timer, timeri, timer->sticks);
unlock:
spin_unlock_irqrestore(&timer->lock, flags);
snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE);
return result;
} }
/* /*
...@@ -619,7 +600,10 @@ int snd_timer_continue(struct snd_timer_instance *timeri) ...@@ -619,7 +600,10 @@ int snd_timer_continue(struct snd_timer_instance *timeri)
*/ */
int snd_timer_pause(struct snd_timer_instance * timeri) int snd_timer_pause(struct snd_timer_instance * timeri)
{ {
return _snd_timer_stop(timeri, SNDRV_TIMER_EVENT_PAUSE); if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE)
return snd_timer_stop_slave(timeri, false);
else
return snd_timer_stop1(timeri, false);
} }
/* /*
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7