* [PATCH] ALSA: aloop - add locking to timer access
@ 2012-09-29 6:54 Omair Mohammed Abdullah
2012-10-08 8:17 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Omair Mohammed Abdullah @ 2012-09-29 6:54 UTC (permalink / raw)
To: alsa-devel; +Cc: tiwai, Vinod Koul, Omair Mohammed Abdullah
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@linux.intel.com>
Signed-off-by: Vinod Koul <vinod.koul@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ALSA: aloop - add locking to timer access
2012-09-29 6:54 [PATCH] ALSA: aloop - add locking to timer access Omair Mohammed Abdullah
@ 2012-10-08 8:17 ` Takashi Iwai
2012-10-08 8:15 ` Vinod Koul
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2012-10-08 8:17 UTC (permalink / raw)
To: Omair Mohammed Abdullah; +Cc: alsa-devel, Vinod Koul
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@linux.intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@linux.intel.com>
Thanks, applied.
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ALSA: aloop - add locking to timer access
2012-10-08 8:17 ` Takashi Iwai
@ 2012-10-08 8:15 ` Vinod Koul
2012-10-08 8:41 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Vinod Koul @ 2012-10-08 8:15 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Omair Mohammed Abdullah
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@linux.intel.com>
> > Signed-off-by: Vinod Koul <vinod.koul@linux.intel.com>
>
> Thanks, applied.
Sorry should have mentioned earlier, but can you also CC stable on this
one (if not done already :))
>
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ALSA: aloop - add locking to timer access
2012-10-08 8:15 ` Vinod Koul
@ 2012-10-08 8:41 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2012-10-08 8:41 UTC (permalink / raw)
To: Vinod Koul; +Cc: alsa-devel, Omair Mohammed Abdullah
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@linux.intel.com>
> > > Signed-off-by: Vinod Koul <vinod.koul@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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-08 8:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-29 6:54 [PATCH] ALSA: aloop - add locking to timer access Omair Mohammed Abdullah
2012-10-08 8:17 ` Takashi Iwai
2012-10-08 8:15 ` Vinod Koul
2012-10-08 8:41 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).