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

Gurucharan Shetty shettyg at nicira.com
Mon Jun 30 15:49:00 UTC 2014


On Fri, Jun 27, 2014 at 5:40 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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.
Thank you. I added your suggested incremental and pushed it.
>
> 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