[alsa-devel] [PATCH] ALSA: aloop - add locking to timer access

[Takashi Iwai]

At Mon, 08 Oct 2012 13:45:12 +0530,
Vinod Koul wrote:
> 
> On Mon, 2012-10-08 at 10:17 +0200, Takashi Iwai wrote:
> > At Sat, 29 Sep 2012 12:24:05 +0530,
> > Omair Mohammed Abdullah wrote:
> > > 
> > > When the loopback timer handler is running, calling del_timer() (for STOP
> > > trigger) will not wait for the handler to complete before deactivating the
> > > timer. The timer gets rescheduled in the handler as usual. Then a subsequent
> > > START trigger will try to start the timer using add_timer() with a timer pending
> > > leading to a kernel panic.
> > > 
> > > Serialize the calls to add_timer() and del_timer() using a spin lock to avoid
> > > this.
> > > 
> > > Signed-off-by: Omair Mohammed Abdullah <omair.m.abdullah at linux.intel.com>
> > > Signed-off-by: Vinod Koul <vinod.koul at linux.intel.com>
> > 
> > Thanks, applied.
> Sorry should have mentioned earlier, but can you also CC stable on this
> one (if not done already :))

Yes, I tagged already :)
Thanks for reminder, though.


Takashi


> > 
> > 
> > Takashi
> > 
> > > ---
> > >  sound/drivers/aloop.c |    6 ++++++
> > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
> > > index 5a34355..0fe6d64 100644
> > > --- a/sound/drivers/aloop.c
> > > +++ b/sound/drivers/aloop.c
> > > @@ -120,6 +120,7 @@ struct loopback_pcm {
> > >  	unsigned int last_drift;
> > >  	unsigned long last_jiffies;
> > >  	struct timer_list timer;
> > > +	spinlock_t timer_lock;
> > >  };
> > >  
> > >  static struct platform_device *devices[SNDRV_CARDS];
> > > @@ -170,6 +171,7 @@ static void loopback_timer_start(struct loopback_pcm *dpcm)
> > >  	unsigned long tick;
> > >  	unsigned int rate_shift = get_rate_shift(dpcm);
> > >  
> > > +	spin_lock(&dpcm->timer_lock);
> > >  	if (rate_shift != dpcm->pcm_rate_shift) {
> > >  		dpcm->pcm_rate_shift = rate_shift;
> > >  		dpcm->period_size_frac = frac_pos(dpcm, dpcm->pcm_period_size);
> > > @@ -182,12 +184,15 @@ static void loopback_timer_start(struct loopback_pcm *dpcm)
> > >  	tick = (tick + dpcm->pcm_bps - 1) / dpcm->pcm_bps;
> > >  	dpcm->timer.expires = jiffies + tick;
> > >  	add_timer(&dpcm->timer);
> > > +	spin_unlock(&dpcm->timer_lock);
> > >  }
> > >  
> > >  static inline void loopback_timer_stop(struct loopback_pcm *dpcm)
> > >  {
> > > +	spin_lock(&dpcm->timer_lock);
> > >  	del_timer(&dpcm->timer);
> > >  	dpcm->timer.expires = 0;
> > > +	spin_unlock(&dpcm->timer_lock);
> > >  }
> > >  
> > >  #define CABLE_VALID_PLAYBACK	(1 << SNDRV_PCM_STREAM_PLAYBACK)
> > > @@ -667,6 +672,7 @@ static int loopback_open(struct snd_pcm_substream *substream)
> > >  	dpcm->substream = substream;
> > >  	setup_timer(&dpcm->timer, loopback_timer_function,
> > >  		    (unsigned long)dpcm);
> > > +	spin_lock_init(&dpcm->timer_lock);
> > >  
> > >  	cable = loopback->cables[substream->number][dev];
> > >  	if (!cable) {
> > > -- 
> > > 1.7.4.1
> > > 
> 
> 
> -- 
> ~Vinod
>