Re: [PATCH 02/20] libxl: support multiple libxl__ev_fds for the same fd

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2012-04-13 at 19:39 +0100, Ian Jackson wrote:
> We need a slightly more sophisticated data structure to allow this,
> where we record the slot not just for each fd but also for each
> (fd,eventbit) where eventbit is POLLIN, POLLPRI, POLLOUT.

Just to be sure I'm following: By multiple you mean you can have one
libxl__ev_fds listening for e.g. POLLIN and another for POLLOUT but you
specifically exclude the case where two libxl__ev_fds both want to
listen for POLLIN? Similarly one listening for POLLIN|POLLPRI and the
other for POLLOUT|POLLPRI (overlapping) is forbidden.

> Document the new relaxed restriction.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  tools/libxl/libxl_event.c    |   62 +++++++++++++++++++++++------------------
>  tools/libxl/libxl_internal.h |   10 +++++--
>  2 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
> index 5e1a207..672e3fe 100644
> --- a/tools/libxl/libxl_event.c
> +++ b/tools/libxl/libxl_event.c
> @@ -635,10 +635,11 @@ static int beforepoll_internal(libxl__gc *gc, libxl__poller *poller,
>  
> 
>      /*
> -     * In order to be able to efficiently find the libxl__ev_fd
> -     * for a struct poll during _afterpoll, we maintain a shadow
> -     * data structure in CTX->fd_beforepolled: each slot in
> -     * the fds array corresponds to a slot in fd_beforepolled.
> +     * In order to be able to efficiently find the libxl__ev_fd for a
> +     * struct poll during _afterpoll, we maintain a shadow data
> +     * structure in CTX->fd_rindices: each fd corresponds to a slot in
> +     * fd_rindices, and each elemebnt in the rindices is three indices

                               element

> +     * into the fd array (for POLLIN, POLLPRI and POLLOUT).
>       */
>  
>      if (*nfds_io) {
> @@ -659,14 +660,16 @@ static int beforepoll_internal(libxl__gc *gc, libxl__poller *poller,
>          });
>  
>          /* make sure our array is as big as *nfds_io */
> -        if (poller->fd_rindex_allocd < maxfd) {
> -            assert(maxfd < INT_MAX / sizeof(int) / 2);
> -            int *newarray = realloc(poller->fd_rindex, sizeof(int) * maxfd);
> -            if (!newarray) { rc = ERROR_NOMEM; goto out; }
> -            memset(newarray + poller->fd_rindex_allocd, 0,
> -                   sizeof(int) * (maxfd - poller->fd_rindex_allocd));
> -            poller->fd_rindex = newarray;
> -            poller->fd_rindex_allocd = maxfd;
> +        if (poller->fd_rindices_allocd < maxfd) {
> +            assert(ARRAY_SIZE_OK(poller->fd_rindices, maxfd));
> +            poller->fd_rindices =
> +                libxl__realloc(0, poller->fd_rindices,
> +                               maxfd * sizeof(*poller->fd_rindices));
> +            memset(poller->fd_rindices + poller->fd_rindices_allocd,
> +                   0,
> +                   (maxfd - poller->fd_rindices_allocd)
> +                     * sizeof(*poller->fd_rindices));
> +            poller->fd_rindices_allocd = maxfd;
>          }
>      }
>  
> @@ -677,8 +680,10 @@ static int beforepoll_internal(libxl__gc *gc, libxl__poller *poller,
>              fds[used].fd = req_fd;
>              fds[used].events = req_events;
>              fds[used].revents = 0;
> -            assert(req_fd < poller->fd_rindex_allocd);
> -            poller->fd_rindex[req_fd] = used;
> +            assert(req_fd < poller->fd_rindices_allocd);
> +            if (req_events & POLLIN)  poller->fd_rindices[req_fd][0] = used;
> +            if (req_events & POLLPRI) poller->fd_rindices[req_fd][1] = used;
> +            if (req_events & POLLOUT) poller->fd_rindices[req_fd][2] = used;

Would it be possible to have a little struct here instead of the [3]? So
you'd get
	poller->fd_rindeices[req_fd].{in,pri,out}
?

Ah, I see this would make afterpoll_check_fd more complex which I think
would outweigh any gain here.

So, other than the typoe:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.