On Sun, 22 Oct 2017 23:40:12 +0200, Anna-Maria Gleixner wrote:

From: Thomas Gleixner tglx@linutronix.de

The tasklet is used to defer the execution of snd_pcm_period_elapsed() to the softirq context. Using the HRTIMER_MODE_SOFT mode invokes the timer callback in softirq context as well which renders the tasklet useless.

[o-takashi: avoid stall due to a call of hrtimer_cancel() on a callback of hrtimer] Signed-off-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Anna-Maria Gleixner anna-maria@linutronix.de Cc: Jaroslav Kysela perex@perex.cz Cc: Takashi Iwai tiwai@suse.com

Feel free to take my ack: Reviewed-by: Takashi Iwai tiwai@suse.de

thanks,

Takashi

Cc: Takashi Sakamoto o-takashi@sakamocchi.jp Cc: alsa-devel@alsa-project.org Link: http://lkml.kernel.org/r/20170905161820.jtysvxtfleunbbmf@breakpoint.cc

---

On 2017-09-06 01:05:43 [+0900], Takashi Sakamoto wrote:

...

As Iwai-san mentioned, in this function, .trigger can be called in two cases; XRUN occurs and draining is done. Thus, let you change the comment as 'In cases of XRUN and draining, this calls .trigger to stop PCM substream.'. I'm sorry to trouble you.

snd_pcm_period_elapsed() ->snd_pcm_update_hw_ptr0() ->snd_pcm_update_state() ->snd_pcm_drain_done() ... ->.trigger(TRIGGER_STOP) ->xrun() ->snd_pcm_stop() ... ->.trigger(TRIGGER_STOP)

I think you asked me just to update the comment. Did I do it right?

v2...v3: updated the comment as per Takashi Sakamoto's suggestion. v1...v2: merged Takashi Sakamoto fixup of the original patch into v2.

sound/drivers/dummy.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)

--- a/sound/drivers/dummy.c +++ b/sound/drivers/dummy.c @@ -376,17 +376,9 @@ struct dummy_hrtimer_pcm { ktime_t period_time; atomic_t running; struct hrtimer timer;

• struct tasklet_struct tasklet; struct snd_pcm_substream *substream;

};

-static void dummy_hrtimer_pcm_elapsed(unsigned long priv) -{

• struct dummy_hrtimer_pcm *dpcm = (struct dummy_hrtimer_pcm *)priv;
• if (atomic_read(&dpcm->running))

• snd_pcm_period_elapsed(dpcm->substream);
  

-}

static enum hrtimer_restart dummy_hrtimer_callback(struct hrtimer *timer) { struct dummy_hrtimer_pcm *dpcm; @@ -394,7 +386,14 @@ static enum hrtimer_restart dummy_hrtime dpcm = container_of(timer, struct dummy_hrtimer_pcm, timer); if (!atomic_read(&dpcm->running)) return HRTIMER_NORESTART;

• tasklet_schedule(&dpcm->tasklet);

• /*

• * In cases of XRUN and draining, this calls .trigger to stop PCM
  

• * substream.
  

• */
  

• snd_pcm_period_elapsed(dpcm->substream);
• if (!atomic_read(&dpcm->running))

• return HRTIMER_NORESTART;
  

• hrtimer_forward_now(timer, dpcm->period_time); return HRTIMER_RESTART;

} @@ -404,7 +403,7 @@ static int dummy_hrtimer_start(struct sn struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;

dpcm->base_time = hrtimer_cb_get_time(&dpcm->timer);

• hrtimer_start(&dpcm->timer, dpcm->period_time, HRTIMER_MODE_REL);

• hrtimer_start(&dpcm->timer, dpcm->period_time, HRTIMER_MODE_REL_SOFT); atomic_set(&dpcm->running, 1); return 0;

} @@ -414,14 +413,14 @@ static int dummy_hrtimer_stop(struct snd struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;

atomic_set(&dpcm->running, 0);

• hrtimer_cancel(&dpcm->timer);

• if (!hrtimer_callback_running(&dpcm->timer))

• hrtimer_cancel(&dpcm->timer);
  

  return 0;

}

static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm) { hrtimer_cancel(&dpcm->timer);

• tasklet_kill(&dpcm->tasklet);

}

static snd_pcm_uframes_t @@ -466,12 +465,10 @@ static int dummy_hrtimer_create(struct s if (!dpcm) return -ENOMEM; substream->runtime->private_data = dpcm;

• hrtimer_init(&dpcm->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);

• hrtimer_init(&dpcm->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT); dpcm->timer.function = dummy_hrtimer_callback; dpcm->substream = substream; atomic_set(&dpcm->running, 0);

• tasklet_init(&dpcm->tasklet, dummy_hrtimer_pcm_elapsed,

•      (unsigned long)dpcm);
  

  return 0;

}