[ovs-dev] [threads 05/28] timeval: Always block SIGALRM in time_poll().

Ben Pfaff blp at nicira.com
Wed Jul 10 23:03:47 UTC 2013


Until now, time_poll() has blocked SIGALRM lazily, only after waking up
once due to SIGALRM.  The rationale was that many calls to poll() wake up
quickly, before any SIGALRM would arrive, so that by only blocking it
lazily we saved two system calls (one to block SIGALRM, one to unblock it)
on most calls.

Adding multithreading to OVS seems to me to shift the balance a little
bit.  I expect that most of the new threads will often be idle.  It
seems counterintuitive to have them wake up after sleeping for a while.

Therefore, this commit changes time_poll() to block SIGALRM every time, not
just after a wakeup due to SIGALRM.  We use ppoll() instead of poll() to
reduce the overhead of blocking SIGALRM from two system calls to one.
(The usual reason for ppoll() is to ensure atomicity of changing the signal
mask and blocking, but time_poll() does not need atomicity.  Therefore,
even a userspace emulated ppoll() would work here in a race-free manner, if
the kernel does not support ppoll().)

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/timeval.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/lib/timeval.c b/lib/timeval.c
index 0ccfa42..87dab37 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -327,8 +327,8 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
 {
     static long long int last_wakeup = 0;
     long long int start;
-    sigset_t oldsigs;
-    bool blocked;
+    sigset_t *psigmask;
+    sigset_t sigmask;
     int retval;
 
     time_refresh();
@@ -337,23 +337,41 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
     }
     coverage_clear();
     start = time_msec();
-    blocked = false;
+    psigmask = NULL;
 
     timeout_when = MIN(timeout_when, deadline);
 
     for (;;) {
         long long int now = time_msec();
-        int time_left;
+        struct timespec time_left;
 
         if (now >= timeout_when) {
-            time_left = 0;
-        } else if ((unsigned long long int) timeout_when - now > INT_MAX) {
-            time_left = INT_MAX;
+            time_left.tv_sec = 0;
+            time_left.tv_nsec = 0;
         } else {
-            time_left = timeout_when - now;
+            unsigned long long int ms_timeout;
+            unsigned long long int s_timeout;
+            int ms_remainder;
+
+            ms_timeout = (unsigned long long int) timeout_when - now;
+            s_timeout = ms_timeout / 1000;
+            ms_remainder = ms_timeout % 1000;
+            if (s_timeout <= TIME_MAX) {
+                time_left.tv_sec = s_timeout;
+                time_left.tv_nsec = ms_remainder * 1000 * 1000;
+            } else {
+                time_left.tv_sec = TIME_MAX;
+                time_left.tv_nsec = 0;
+            }
+
+            if (CACHE_TIME && !psigmask) {
+                xpthread_sigmask(SIG_BLOCK, NULL, &sigmask);
+                sigaddset(&sigmask, SIGALRM);
+                psigmask = &sigmask;
+            }
         }
 
-        retval = poll(pollfds, n_pollfds, time_left);
+        retval = ppoll(pollfds, n_pollfds, &time_left, psigmask);
         if (retval < 0) {
             retval = -errno;
         }
@@ -370,14 +388,6 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
         if (retval != -EINTR) {
             break;
         }
-
-        if (!blocked && CACHE_TIME) {
-            block_sigalrm(&oldsigs);
-            blocked = true;
-        }
-    }
-    if (blocked) {
-        unblock_sigalrm(&oldsigs);
     }
     last_wakeup = time_msec();
     refresh_rusage();
-- 
1.7.2.5




More information about the dev mailing list