pcm: direct: fix race on clearing timer events

snd_timer handling is racy: plugins clear timer queue if avail_min
is not reached to force a sleep on timer. The race can happen if
the expected event arrives in between the avail check and the
clearing of pending events. If this race happens, the user will
unnecessarily wait for one more timer event. On low latency/realtime
streams this can lead to xruns and must be avoided.

As a fix we recheck avail after having cleared poll events.

Signed-off-by: Andreas Pape <apape@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Mounesh Sutar <sutar.mounesh@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 364191b20bab1e89a7b54005bea7dd064cd5d9b5..8a75c421dd7bda7939295a48bf93b5275c867836 100644 (file)
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -515,10 +515,12 @@ int snd_pcm_direct_async(snd_pcm_t *pcm, int sig, pid_t pid)
 }
 
 /* empty the timer read queue */
-void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
+int snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
 {
+       int changed = 0;
        if (dmix->timer_need_poll) {
                while (poll(&dmix->timer_fd, 1, 0) > 0) {
+                       changed++;
                        /* we don't need the value */
                        if (dmix->tread) {
                                snd_timer_tread_t rbuf[4];
@@ -533,15 +535,17 @@ void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
                        snd_timer_tread_t rbuf[4];
                        int len;
                        while ((len = snd_timer_read(dmix->timer, rbuf,
-                                                    sizeof(rbuf))) > 0 &&
+                                                    sizeof(rbuf))) > 0
+                                                    && (++changed) &&
                               len != sizeof(rbuf[0]))
                                ;
                } else {
                        snd_timer_read_t rbuf;
                        while (snd_timer_read(dmix->timer, &rbuf, sizeof(rbuf)) > 0)
-                               ;
+                               changed++;
                }
        }
+       return changed;
 }
 
 int snd_pcm_direct_timer_stop(snd_pcm_direct_t *dmix)
@@ -693,6 +697,8 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
        int empty = 0;
 
        assert(pfds && nfds == 1 && revents);
+
+timer_changed:
        events = pfds[0].revents;
        if (events & POLLIN) {
                snd_pcm_uframes_t avail;
@@ -720,7 +726,16 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
                break;
        default:
                if (empty) {
-                       snd_pcm_direct_clear_timer_queue(dmix);
+                       /* here we have a race condition:
+                        * if period event arrived after the avail_update call
+                        * above we might clear this event with the following
+                        * clear_timer_queue.
+                        * There is no way to do this in atomic manner, so we
+                        * need to recheck avail_update if we successfully
+                        * cleared a poll event.
+                        */
+                       if (snd_pcm_direct_clear_timer_queue(dmix))
+                               goto timer_changed;
                        events &= ~(POLLOUT|POLLIN);
                        /* additional check */
                        switch (__snd_pcm_state(pcm)) {

diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h
index fba55fdcb5b3696ef28162f1a9813371265413fb..24e85f096310841e538ed824df58ef1ef1e8ac02 100644 (file)
--- a/src/pcm/pcm_direct.h
+++ b/src/pcm/pcm_direct.h
@@ -322,7 +322,7 @@ int snd_pcm_direct_munmap(snd_pcm_t *pcm);
 int snd_pcm_direct_prepare(snd_pcm_t *pcm);
 int snd_pcm_direct_resume(snd_pcm_t *pcm);
 int snd_pcm_direct_timer_stop(snd_pcm_direct_t *dmix);
-void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix);
+int snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix);
 int snd_pcm_direct_set_timer_params(snd_pcm_direct_t *dmix);
 int snd_pcm_direct_open_secondary_client(snd_pcm_t **spcmp, snd_pcm_direct_t *dmix, const char *client_name);