[Xen-devel] [PATCH 3/3] libxl: fd events: Suppress spurious fd events

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Always recheck with poll() right before making the callback.

All sorts of things may have happened since poll() originally signaled
the fd.  We would like the main functional libxl code not to have to
worry about spurious wakeups.

In particular, this fixes a bug in the save/restore callout: the save
helper message reader operates with the fd in blocking mode.  In a
multithreaded program one thread might have eaten all the messages out
of the fd while another one is busy returning from poll and reaquiring
the libxl lock, possibly resulting in a deadlock.

(Also, we abolish the anomalous direct caller of efd->func.)

Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Reported-by: Jim Fehlig <jfehlig@xxxxxxxx>
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Jim Fehlig <jfehlig@xxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 tools/libxl/libxl_event.c    |   16 ++++++++--------
 tools/libxl/libxl_internal.h |    3 +++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 2b2254e..9ff4e78 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1121,12 +1121,15 @@ static int afterpoll_check_fd(libxl__poller *poller,
     return revents;
 }
 
-static void fd_occurs(libxl__egc *egc, libxl__ev_fd *efd, short revents)
+static void fd_occurs(libxl__egc *egc, libxl__ev_fd *efd, short revents_ign)
 {
-    DBG("ev_fd=%p occurs fd=%d events=%x revents=%x",
-        efd, efd->fd, efd->events, revents);
+    short revents_current = libxl__fd_poll_recheck(egc, efd->fd, efd->events);
 
-    efd->func(egc, efd, efd->fd, efd->events, revents);
+    DBG("ev_fd=%p occurs fd=%d events=%x revents_ign=%x revents_current=%x",
+        efd, efd->fd, efd->events, revents_ign, revents_current);
+
+    if (revents_current)
+        efd->func(egc, efd, efd->fd, efd->events, revents_current);
 }
 
 static void afterpoll_internal(libxl__egc *egc, libxl__poller *poller,
@@ -1255,10 +1258,7 @@ void libxl_osevent_occurred_fd(libxl_ctx *ctx, void 
*for_libxl,
     if (!ev) goto out;
     if (ev->fd != fd) goto out;
 
-    short current_revents = libxl__fd_poll_recheck(egc, ev->fd, ev->events);
-
-    if (current_revents)
-        ev->func(egc, ev, fd, ev->events, current_revents);
+    fd_occurs(egc, ev, revents_ign);
 
  out:
     CTX_UNLOCK;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e177911..dd0a8df 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -177,6 +177,9 @@ typedef void libxl__ev_fd_callback(libxl__egc *egc, 
libxl__ev_fd *ev,
    *
    * It is not permitted to listen for the same or overlapping events
    * on the same fd using multiple different libxl__ev_fd's.
+   *
+   * (Spurious wakeups, and spurious bits set in revents, are
+   * suppressed by the libxl event core.)
    */
 struct libxl__ev_fd {
     /* caller should include this in their own struct */
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel