mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	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
					
				
					 1 changed files with 102 additions and 118 deletions
				
			
		| 
						 | 
				
			
			@ -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