[ovs-dev] [PATCH] poll-loop: Create Windows event handles for sockets automatically.

Ben Pfaff blp at nicira.com
Sat Jun 28 00:40:57 UTC 2014


On Fri, Jun 27, 2014 at 02:32:51PM -0700, Gurucharan Shetty wrote:
> We currently have a poll_fd_wait_event(fd, wevent, events) function that
> is used at places common to Windows and Linux where we have to wait on
> sockets.  On Linux, 'wevent' is always set as zero. On Windows, for sockets,
> when we send both 'fd' and 'wevent', we associate them with each other for
> 'events' and then wait on 'wevent'. Also on Windows, when we only send 'wevent'
> to this function, we would simply wait for all events for that 'wevent'.
> 
> There is a disadvantage with this approach.
> * Windows clients need to create a 'wevent' and then pass it along. This
> means that at a lot of places where we create sockets, we also are forced
> to create a 'wevent'.
> 
> With this commit, we pass the responsibility of creating a 'wevent' to
> poll_fd_wait() in case of sockets. That way, a client using poll_fd_wait()
> is only concerned about sockets and not about 'wevents'. There is a potential
> disadvantage with this change in that we create events more often and that
> may have a performance penalty. If that turns out to be the case, we will
> eventually need to create a pool of wevents that can be re-used.
> 
> In Windows, there are cases where we want to wait on a event (not
> associated with any sockets) and then control it using functions
> like SetEvent() etc. For that purpose, introduce a new function
> poll_wevent_wait(). For this function, the client needs to create a event
> and then pass it along as an argument.
> 
> Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>

This is very nice.  Thank you!

Here are some suggestions as an incremental diff.

Acked-by: Ben Pfaff <blp at nicira.com>

diff --git a/lib/poll-loop.c b/lib/poll-loop.c
index 641a669..9d826e2 100644
--- a/lib/poll-loop.c
+++ b/lib/poll-loop.c
@@ -126,6 +126,19 @@ poll_create_node(int fd, HANDLE wevent, short int events, const char *where)
     }
 }
 
+/* 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.
+ *
+ * On Windows, 'fd' must be a socket.
+ *
+ * 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
+ * 'where'.) */
 void
 poll_fd_wait_at(int fd, short int events, const char *where)
 {
@@ -133,6 +146,16 @@ poll_fd_wait_at(int fd, short int events, const char *where)
 }
 
 #ifdef _WIN32
+/* Registers for the next call to poll_block() to wake up when 'wevent' is
+ * signaled.
+ *
+ * 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_wevent_wait() to automatically provide the caller's source file and
+ * line number for 'where'.) */
 void
 poll_wevent_wait_at(HANDLE wevent, const char *where)
 {
diff --git a/lib/poll-loop.h b/lib/poll-loop.h
index bd19234..7dbfdc8 100644
--- a/lib/poll-loop.h
+++ b/lib/poll-loop.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 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.
@@ -54,8 +54,7 @@ void poll_fd_wait_at(int fd, short int events, const char *where);
 #define poll_fd_wait(fd, events) poll_fd_wait_at(fd, events, SOURCE_LOCATOR)
 
 #ifdef _WIN32
-#define poll_wevent_wait(wevent) \
-    poll_wevent_wait_at(wevent, SOURCE_LOCATOR)
+#define poll_wevent_wait(wevent) poll_wevent_wait_at(wevent, SOURCE_LOCATOR)
 #endif /* _WIN32 */
 
 void poll_timer_wait_at(long long int msec, const char *where);



More information about the dev mailing list