[ovs-dev] [PATCH] poll-loop: windows poll_block implementation

Ben Pfaff blp at nicira.com
Fri Jan 17 23:41:31 UTC 2014


On Thu, Jan 09, 2014 at 04:26:12PM -0800, Linda Sun wrote:
> Use WaitForMultipleObjects for polling on windows.  This works on all kinds
>  of objects, e.g. sockets, files, especially ioctl calls to the kernel.
>  poll_fd_wait_event() is used if events need to be passed to pollfds.
> latch is signaled with event, to be waited/polled by WaitForMultipleObjects()
>  as well.
> Changed array of fds to hmap to check for duplicate fds.
> 
> Signed-off-by: Linda Sun <lsun at vmware.com>

I made some stylistic fixes to this, as follows, and will apply it to
master soon.  Thank you!

diff --git a/lib/poll-loop.c b/lib/poll-loop.c
index 815d2c2..623880f 100644
--- a/lib/poll-loop.c
+++ b/lib/poll-loop.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,9 +40,9 @@ COVERAGE_DEFINE(poll_zero_timeout);
 
 struct poll_node {
     struct hmap_node hmap_node;
-    struct pollfd poll_fd;    /* Events to pass to time_poll() */
-    HANDLE wevent;            /* events for waitformultipleobjects */
-    const char *where;        /* where each pollfd was created */
+    struct pollfd poll_fd;    /* Events to pass to time_poll(). */
+    HANDLE wevent;            /* Events for WaitForMultipleObjects(). */
+    const char *where;        /* Where poll_node was created. */
 };
 
 struct poll_loop {
@@ -57,13 +57,13 @@ struct poll_loop {
 
 static struct poll_loop *poll_loop(void);
 
-/* Look up the node with same fd and wevent */
+/* Look up the node with same fd and wevent. */
 static struct poll_node *
 poll_fd_node_find(struct poll_loop *loop, int fd, uint32_t wevent)
 {
     struct poll_node *node;
 
-    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, hash_2words(fd, wevent), 
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, hash_2words(fd, wevent),
                              &loop->poll_nodes) {
         if (node->poll_fd.fd == fd && node->wevent == wevent) {
             return node;
@@ -72,18 +72,20 @@ poll_fd_node_find(struct poll_loop *loop, int fd, uint32_t wevent)
     return NULL;
 }
 
-/* On unix based systems:
- * Registers 'fd' as waiting for the specified 'events' (which should be POLLIN
- * or POLLOUT or POLLIN | POLLOUT).  The following call to poll_block() will
- * wake up when 'fd' becomes ready for one or more of the requested events.
- * the 'fd's are given to poll() function later.
+/* On Unix based systems:
  *
- * On windows system:
- * register 'wevent' handle for the specified 'events'.  These wevents are given
- * to the handleMultipleObjects() to be polled.
- * The event registration is one-shot: only the following call to poll_block()
- * is affected.  The event will need to be re-registered after poll_block() is
- * called if it is to persist.
+ *     Registers 'fd' as waiting for the specified 'events' (which should be
+ *     POLLIN or POLLOUT or POLLIN | POLLOUT).  The following call to
+ *     poll_block() will wake up when 'fd' becomes ready for one or more of the
+ *     requested events.  the 'fd's are given to poll() function later.
+ *
+ * On Windows system:
+ *
+ *     Register 'wevent' handle for the specified 'events'.  These wevents are
+ *     given to the handleMultipleObjects() to be polled.  The event
+ *     registration is one-shot: only the following call to poll_block() is
+ *     affected.  The event will need to be re-registered after poll_block() is
+ *     called if it is to persist.
  *
  * ('where' is used in debug logging.  Commonly one would use poll_fd_wait() to
  * automatically provide the caller's source file and line number for
@@ -97,14 +99,14 @@ poll_fd_wait_at(int fd, HANDLE wevent, short int events, const char *where)
     COVERAGE_INC(poll_fd_wait);
 
 #ifdef _WIN32
-    /* null event cannot be polled */
+    /* Null event cannot be polled. */
     if (wevent == 0) {
         VLOG_ERR("No event to wait fd %d", fd);
         return;
     }
 #endif
 
-    /* check for duplicate.  If found, "or" the event */
+    /* Check for duplicate.  If found, "or" the event. */
     node = poll_fd_node_find(loop, fd, wevent);
     if (node) {
         node->poll_fd.events |= events;
@@ -112,10 +114,10 @@ poll_fd_wait_at(int fd, HANDLE wevent, short int events, const char *where)
         node = xzalloc(sizeof *node);
         node->where = where;
         node->poll_fd.fd = fd;
-        node->wevent = wevent;
         node->poll_fd.events = events;
+        node->wevent = wevent;
         hmap_insert(&loop->poll_nodes, &node->hmap_node,
-            hash_2words(fd, wevent));
+                    hash_2words(fd, wevent));
     }
 }
 
@@ -245,6 +247,17 @@ log_wakeup(const char *where, const struct pollfd *pollfd, int timeout)
     ds_destroy(&s);
 }
 
+static void
+free_poll_nodes(struct poll_loop *loop)
+{
+    struct poll_node *node, *next;
+
+    HMAP_FOR_EACH_SAFE (node, next, hmap_node, &loop->poll_nodes) {
+        hmap_remove(&loop->poll_nodes, &node->hmap_node);
+        free(node);
+    }
+}
+
 /* Blocks until one or more of the events registered with poll_fd_wait()
  * occurs, or until the minimum duration registered with poll_timer_wait()
  * elapses, or not at all if poll_immediate_wake() has been called. */
@@ -252,7 +265,7 @@ void
 poll_block(void)
 {
     struct poll_loop *loop = poll_loop();
-    struct poll_node *node, *next;
+    struct poll_node *node;
     struct pollfd *pollfds;
     HANDLE *wevents = NULL;
     int elapsed;
@@ -276,7 +289,7 @@ poll_block(void)
 
     /* populate with all the fds and events */
     HMAP_FOR_EACH (node, hmap_node, &loop->poll_nodes) {
-        memcpy(&pollfds[i], &node->poll_fd, sizeof node->poll_fd);
+        pollfds[i] = node->poll_fd;
 #ifdef _WIN32
         wevents[i] = node->wevent;
 #endif
@@ -300,11 +313,7 @@ poll_block(void)
         }
     }
 
-    HMAP_FOR_EACH_SAFE (node, next, hmap_node, &loop->poll_nodes) {
-        hmap_remove(&loop->poll_nodes, &node->hmap_node);
-        free(node);
-    }
-
+    free_poll_nodes(loop);
     loop->timeout_when = LLONG_MAX;
     loop->timeout_where = NULL;
     free(pollfds);
@@ -321,13 +330,8 @@ static void
 free_poll_loop(void *loop_)
 {
     struct poll_loop *loop = loop_;
-    struct poll_node *node, *next;
-
-    HMAP_FOR_EACH_SAFE (node, next, hmap_node, &loop->poll_nodes) {
-        hmap_remove(&loop->poll_nodes, &node->hmap_node);
-        free(node);
-    }
 
+    free_poll_loop(loop);
     hmap_destroy(&loop->poll_nodes);
     free(loop);
 }
diff --git a/lib/timeval.c b/lib/timeval.c
index 71eae46..691cf74 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -232,7 +232,7 @@ time_alarm(unsigned int secs)
  *
  * Stores the number of milliseconds elapsed during poll in '*elapsed'. */
 int
-time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles,
+time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
           long long int timeout_when, int *elapsed)
 {
     long long int *last_wakeup = last_wakeup_get();
@@ -262,9 +262,6 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles,
         }
 
 #ifndef _WIN32
-        if (handles) {
-            /* non-windows platform shouldn't really create these handles */
-        }
         retval = poll(pollfds, n_pollfds, time_left);
         if (retval < 0) {
             retval = -errno;



More information about the dev mailing list