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: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
parent
9984d1b583
commit
f65e0d2998
|
@ -305,8 +305,6 @@ int snd_timer_open(struct snd_timer_instance **ti,
|
|||
return 0;
|
||||
}
|
||||
|
||||
static int _snd_timer_stop(struct snd_timer_instance *timeri, int event);
|
||||
|
||||
/*
|
||||
* close a timer instance
|
||||
*/
|
||||
|
@ -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)
|
||||
{
|
||||
struct snd_timer *timer;
|
||||
unsigned long flags;
|
||||
unsigned long resolution = 0;
|
||||
struct snd_timer_instance *ts;
|
||||
struct timespec tstamp;
|
||||
|
@ -413,34 +410,66 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event)
|
|||
return;
|
||||
if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE)
|
||||
return;
|
||||
spin_lock_irqsave(&timer->lock, flags);
|
||||
list_for_each_entry(ts, &ti->slave_active_head, active_list)
|
||||
if (ts->ccallback)
|
||||
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,
|
||||
unsigned long sticks)
|
||||
/* start/continue a master timer */
|
||||
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);
|
||||
if (timer->running) {
|
||||
if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE)
|
||||
goto __start_now;
|
||||
timer->flags |= SNDRV_TIMER_FLG_RESCHED;
|
||||
timeri->flags |= SNDRV_TIMER_IFLG_START;
|
||||
return 1; /* delayed start */
|
||||
result = 1; /* delayed start */
|
||||
} else {
|
||||
timer->sticks = sticks;
|
||||
if (start)
|
||||
timer->sticks = ticks;
|
||||
timer->hw.start(timer);
|
||||
__start_now:
|
||||
timer->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;
|
||||
|
||||
|
@ -454,88 +483,37 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri)
|
|||
spin_lock(&timeri->timer->lock);
|
||||
list_add_tail(&timeri->active_list,
|
||||
&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_irqrestore(&slave_active_lock, flags);
|
||||
return 1; /* delayed start */
|
||||
}
|
||||
|
||||
/*
|
||||
* start the timer instance
|
||||
*/
|
||||
int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
|
||||
/* stop/pause a master timer */
|
||||
static int snd_timer_stop1(struct snd_timer_instance *timeri, bool stop)
|
||||
{
|
||||
struct snd_timer *timer;
|
||||
int result = -EINVAL;
|
||||
int result = 0;
|
||||
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;
|
||||
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;
|
||||
if (!timer)
|
||||
return -EINVAL;
|
||||
spin_lock_irqsave(&timer->lock, flags);
|
||||
if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |
|
||||
SNDRV_TIMER_IFLG_START))) {
|
||||
spin_unlock_irqrestore(&timer->lock, flags);
|
||||
return -EBUSY;
|
||||
result = -EBUSY;
|
||||
goto unlock;
|
||||
}
|
||||
list_del_init(&timeri->ack_list);
|
||||
list_del_init(&timeri->active_list);
|
||||
if (timer->card && timer->card->shutdown) {
|
||||
spin_unlock_irqrestore(&timer->lock, flags);
|
||||
return 0;
|
||||
if (timer->card && timer->card->shutdown)
|
||||
goto unlock;
|
||||
if (stop) {
|
||||
timeri->cticks = timeri->ticks;
|
||||
timeri->pticks = 0;
|
||||
}
|
||||
if ((timeri->flags & SNDRV_TIMER_IFLG_RUNNING) &&
|
||||
!(--timer->running)) {
|
||||
|
@ -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);
|
||||
snd_timer_notify1(timeri, stop ? SNDRV_TIMER_EVENT_STOP :
|
||||
SNDRV_TIMER_EVENT_CONTINUE);
|
||||
unlock:
|
||||
spin_unlock_irqrestore(&timer->lock, flags);
|
||||
__end:
|
||||
if (event != SNDRV_TIMER_EVENT_RESOLUTION)
|
||||
snd_timer_notify1(timeri, event);
|
||||
return result;
|
||||
}
|
||||
|
||||
/* 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;
|
||||
}
|
||||
|
||||
/*
|
||||
* 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.
|
||||
*
|
||||
|
@ -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)
|
||||
{
|
||||
struct snd_timer *timer;
|
||||
unsigned long flags;
|
||||
int err;
|
||||
|
||||
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;
|
||||
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE)
|
||||
return snd_timer_stop_slave(timeri, true);
|
||||
else
|
||||
return snd_timer_stop1(timeri, true);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -586,32 +589,10 @@ int snd_timer_stop(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)
|
||||
return snd_timer_start_slave(timeri);
|
||||
timer = timeri->timer;
|
||||
if (! timer)
|
||||
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;
|
||||
return snd_timer_start_slave(timeri, false);
|
||||
else
|
||||
return snd_timer_start1(timeri, false, 0);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -619,7 +600,10 @@ int snd_timer_continue(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);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
Loading…
Reference in a new issue