[alsa-devel] [PATCH - IOPLUG DRAIN 0/2]

[Takashi Iwai]

On Thu, 29 Mar 2018 08:39:36 +0200,
Wischer, Timo (ADITG/ESB) wrote:
> 
> > For the non-blocking mode, it's not supposed that drain() is called
> > multiple times.  Instead, it should do the loop of snd_pcm_wait(), and
> > status check, as ioplug_drain_vai_poll() actually does.
> 
> I fear the repeat call to snd_pcm_wait() when draining is active would end up in 100% CPU load as long as draining is not done because snd_pcm_wait() would immediate return due to the fact that avail > min_avail at the end of draining.
> 
> I have not tested it but I could also not find a line which would ignore the avail > min_avail short circuit of snd_pcm_wait().

A good point, and indeed it's broken.
We have to skip the avail_min check during draining.  It's already
done in the kernel code, but forgotten in alsa-lib side.

The fix patch is below.


Thanks!

Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] pcm: Skip avail_min check during draining

snd_pcm_wait() & co checks the current avail value and returns
immediately if it satisfies <= avail_min condition.  It's good in
general except for one situation: draining.  When the draining is
being performed in the non-blocking mode, apps are supposed to wait
via poll(), typically via snd_pcm_wait().  So this ends up with the
busy loop because of the immediate return from snd_pcm_wait().

A simple workaround is to put the PCM state check and ignore the
avail_min condition if it's DRAINING state.  The equivalent check is
found in the kernel xfer code, too.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 src/pcm/pcm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index ed47cb516c73..11aec8052135 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -2751,7 +2751,9 @@ int __snd_pcm_wait_in_lock(snd_pcm_t *pcm, int timeout)
 {
 	int err;
 
-	if (!snd_pcm_may_wait_for_avail_min(pcm, snd_pcm_mmap_avail(pcm))) {
+	/* NOTE: avail_min check can be skipped during draining */
+	if (__snd_pcm_state(pcm) != SND_PCM_STATE_DRAINING &&
+	    !snd_pcm_may_wait_for_avail_min(pcm, snd_pcm_mmap_avail(pcm))) {
 		/* check more precisely */
 		err = pcm_state_to_error(__snd_pcm_state(pcm));
 		return err < 0 ? err : 1;
-- 
2.16.2