[Xen-devel] [PATCH v3 05/10] libxl: event: Make libxl__poller_wakeup take a gc, not an egc

[Ian Jackson <ian.jackson@eu.citrix.com>]

We are going to want to call this in the following situation:

 * We have just set up an ao, which is to call back - so a
   non-synchronous one.  It ought not to call the application
   back right away, so no egc.

 * There is a libxl thread blocking somewhere but it is using
   using an out of date fd or timeout set, which does not take into
   account the ao we have just started.

 * We try to wake that thread up, but libxl__poller_wakeup fails.

In more detail:

The idea before was that these two functions take an egc, not so much
because it actually uses the egc, but to make sure it's only called in a
restricted set of conditions; and now we're relaxing those conditions.

Specifically, we need to make one exception, relating to ao's.

In the situation described above, there is no egc, but we need to call
libxl__poller_wakeup.  Introducing an egc is wrong because that would
imply that this situation might result in application callbacks, but
it shouldn't (and not having an egc prevents that).

libxl__poller_wakeup and LIBXL__EVENT_DISASTER only take an egc for
form's sake; they don't use any part of it other than the gc.  The
"form's sake" is to stop them being called from libxl entrypoints that
are not involved in event generation.

Before this patch this is enforced by the types: you can't call it in
the wrong place because it wants an egc which you don't have.

After this patch this is no longer enforced.  But the mistake
(principally, calling _DISASTER) seems unlikely.  The type enforcement
I mention above was done because it was possible and easy, not because
it was important.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
---
v3: Significantly expanded commit message based on irc comments
v2: New patch
---
 tools/libxl/libxl_event.c    | 7 +++----
 tools/libxl/libxl_internal.h | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 16e6786889..268a5da120 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1477,7 +1477,7 @@ void libxl__event_occurred(libxl__egc *egc, libxl_event *event)
         libxl__poller *poller;
         LIBXL_TAILQ_INSERT_TAIL(&CTX->occurred, event, link);
         LIBXL_LIST_FOREACH(poller, &CTX->pollers_event, entry)
-            libxl__poller_wakeup(egc, poller);
+            libxl__poller_wakeup(gc, poller);
     }
 }
 
@@ -1668,9 +1668,8 @@ void libxl__poller_put(libxl_ctx *ctx, libxl__poller *p)
     LIBXL_LIST_INSERT_HEAD(&ctx->pollers_idle, p, entry);
 }
 
-void libxl__poller_wakeup(libxl__egc *egc, libxl__poller *p)
+void libxl__poller_wakeup(libxl__gc *gc, libxl__poller *p)
 {
-    EGC_GC;
     int e = libxl__self_pipe_wakeup(p->wakeup_pipe[1]);
     if (e) LIBXL__EVENT_DISASTER(gc, "cannot poke watch pipe", e, 0);
 }
@@ -1924,7 +1923,7 @@ void libxl__ao_complete_check_progress_reports(libxl__egc *egc, libxl__ao *ao)
         assert(ao->in_initiator);
         if (!ao->constructing)
             /* don't bother with this if we're not in the event loop */
-            libxl__poller_wakeup(egc, ao->poller);
+            libxl__poller_wakeup(gc, ao->poller);
     } else if (ao->how.callback) {
         LOG(DEBUG, "ao %p: complete for callback", ao);
         LIBXL_TAILQ_INSERT_TAIL(&egc->aos_for_callback, ao, entry_for_callback);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 328ecf3e1e..b68ab218b6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1311,7 +1311,7 @@ _hidden void libxl__poller_put(libxl_ctx*, libxl__poller *p /* may be NULL */);
 
 /* Notifies whoever is polling using p that they should wake up.
  * ctx must be locked. */
-_hidden void libxl__poller_wakeup(libxl__egc *egc, libxl__poller *p);
+_hidden void libxl__poller_wakeup(libxl__gc *egc, libxl__poller *p);
 
 /* Internal to fork and child reaping machinery */
 extern const libxl_childproc_hooks libxl__childproc_default_hooks;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel