Re: [Xen-devel] [PATCH 01/20] libxl: ao: allow immediate completion

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-03-16 at 16:26 +0000, Ian Jackson wrote:
> Make it possible to complete an ao during its initating function.
> 
> Previously this was not generally possible because initiators did not
> have an egc.  But there is no reason why an ao initiator should not
> have an egc, so make the standard macros provide one.
> 
> Change the internal documentation comments accordingly.  (This change,
> which means that an initiator function may call a completion callback
> directly, is already consistent with the documented external API.)
> 
> We also invent of a new state flag "constructing" which indicates
> whether we are between ao__create and ao__inprogress.  This is a
> slightly optimisation which allows ao_complete to not bother poking
> the wakeup pipe, since the logic in ao__inprogress will not run the
> event loop if the ao is complete on entry.
> 
> Also fix the wording in the libxl_internal.h comment forbidding use of
> ao_how-taking functions from within libxl.  (There are sadly currently
> some such functions.)
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>

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

> ---
>  tools/libxl/libxl_event.c    |    7 ++++++-
>  tools/libxl/libxl_internal.h |   14 ++++++++++----
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
> index 271949a..c89add8 100644
> --- a/tools/libxl/libxl_event.c
> +++ b/tools/libxl/libxl_event.c
> @@ -1225,7 +1225,9 @@ void libxl__ao_complete(libxl__egc *egc, libxl__ao *ao, 
> int rc)
>  
>      if (ao->poller) {
>          assert(ao->in_initiator);
> -        libxl__poller_wakeup(egc, ao->poller);
> +        if (!ao->constructing)
> +            /* don't bother with this if we're not in the event loop */
> +            libxl__poller_wakeup(egc, ao->poller);
>      } else if (ao->how.callback) {
>          LIBXL_TAILQ_INSERT_TAIL(&egc->aos_for_callback, ao, 
> entry_for_callback);
>      } else {
> @@ -1251,6 +1253,7 @@ libxl__ao *libxl__ao_create(libxl_ctx *ctx, uint32_t 
> domid,
>      if (!ao) goto out;
>  
>      ao->magic = LIBXL__AO_MAGIC;
> +    ao->constructing = 1;
>      ao->in_initiator = 1;
>      ao->poller = 0;
>      ao->domid = domid;
> @@ -1275,7 +1278,9 @@ int libxl__ao_inprogress(libxl__ao *ao)
>      int rc;
>  
>      assert(ao->magic == LIBXL__AO_MAGIC);
> +    assert(ao->constructing);
>      assert(ao->in_initiator);
> +    ao->constructing = 0;
>  
>      if (ao->poller) {
>          /* Caller wants it done synchronously. */
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index e0a1070..b1e0588 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -347,7 +347,7 @@ struct libxl__egc {
>  
>  struct libxl__ao {
>      uint32_t magic;
> -    unsigned in_initiator:1, complete:1, notified:1;
> +    unsigned constructing:1, in_initiator:1, complete:1, notified:1;
>      int rc;
>      libxl__gc gc;
>      libxl_asyncop_how how;
> @@ -1209,7 +1209,11 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
>   * operation ("ao") machinery.  The function should take a parameter
>   * const libxl_asyncop_how *ao_how and must start with a call to
>   * AO_INITIATOR_ENTRY.  These functions MAY NOT be called from
> - * outside libxl, because they can cause reentrancy callbacks.
> + * inside libxl, because they can cause reentrancy callbacks.
> + *
> + * For the same reason functions taking an ao_how may make themselves
> + * an egc with EGC_INIT (and they will generally want to, to be able
> + * to immediately complete an ao during its setup).
>   *
>   * Lifecycle of an ao:
>   *
> @@ -1240,8 +1244,7 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
>   *   directly or indirectly, should call libxl__ao_complete (with the
>   *   ctx locked, as it will generally already be in any event callback
>   *   function).  This must happen exactly once for each ao (and not if
> - *   the ao has been destroyed, obviously), and it may not happen
> - *   until libxl__ao_inprogress has been called on the ao.
> + *   the ao has been destroyed, obviously).
>   *
>   * - Note that during callback functions, two gcs are available:
>   *    - The one in egc, whose lifetime is only this callback
> @@ -1255,12 +1258,14 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
>      libxl__ctx_lock(ctx);                                       \
>      libxl__ao *ao = libxl__ao_create(ctx, domid, ao_how);       \
>      if (!ao) { libxl__ctx_unlock(ctx); return ERROR_NOMEM; }    \
> +    libxl__egc egc[1]; LIBXL_INIT_EGC(egc[0],ctx);              \
>      AO_GC;
>  
>  #define AO_INPROGRESS ({                                        \
>          libxl_ctx *ao__ctx = libxl__gc_owner(&ao->gc);          \
>          int ao__rc = libxl__ao_inprogress(ao);                  \
>          libxl__ctx_unlock(ao__ctx); /* gc is now invalid */     \
> +        EGC_FREE;                                               \
>          (ao__rc);                                               \
>     })
>  
> @@ -1269,6 +1274,7 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
>          assert(rc);                                             \
>          libxl__ao_abort(ao);                                    \
>          libxl__ctx_unlock(ao__ctx); /* gc is now invalid */     \
> +        EGC_FREE;                                               \
>          (rc);                                                   \
>      })
>  



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