xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Subject: [PATCH 3/3] libxl: Fix eventloop_iteration over-locking
Date: Fri, 17 Feb 2012 18:43:59 +0000	[thread overview]
Message-ID: <1329504239-19999-4-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <20286.31877.938411.558236@mariner.uk.xensource.com>

eventloop_iteration's head comment says that it must be called with
the ctx locked exactly once, and this is indeed true, and it's done
correctly at both the call sites.

However, it takes out the lock an additional time itself.  This is
wrong because it prevents the unlocks around poll from being
effective.  This would mean that a multithreaded event-loop using
program might suffer from undesired blocking, as one thread trying to
enter libxl might end up stalled by another thread waiting for a slow
event.  So remove those two lock calls.

Also add a couple of comments documenting the locking behaviour of
libxl__ao_inprogress and libxl__egc_cleanup.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_event.c    |    4 ----
 tools/libxl/libxl_internal.h |    4 ++--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index dda9d6f..522bd99 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1058,8 +1058,6 @@ static int eventloop_iteration(libxl__egc *egc, libxl__poller *poller) {
     int rc;
     struct timeval now;
     
-    CTX_LOCK;
-
     rc = libxl__gettimeofday(gc, &now);
     if (rc) goto out;
 
@@ -1102,8 +1100,6 @@ static int eventloop_iteration(libxl__egc *egc, libxl__poller *poller) {
     afterpoll_internal(egc, poller,
                        poller->fd_polls_allocd, poller->fd_polls, now);
 
-    CTX_UNLOCK;
-
     rc = 0;
  out:
     return rc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 696c1ed..7b8d0c2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1165,7 +1165,7 @@ libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
 _hidden void libxl__egc_cleanup(libxl__egc *egc);
   /* Frees memory allocated within this egc's gc, and and report all
    * occurred events via callback, if applicable.  May reenter the
-   * application; see restrictions above. */
+   * application; see restrictions above.  The ctx must be UNLOCKED. */
 
 /* convenience macros: */
 
@@ -1260,7 +1260,7 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
  * libxl__ao_inprogress MUST be called with the ctx locked exactly once. */
 _hidden libxl__ao *libxl__ao_create(libxl_ctx*, uint32_t domid,
                                     const libxl_asyncop_how*);
-_hidden int libxl__ao_inprogress(libxl__ao *ao);
+_hidden int libxl__ao_inprogress(libxl__ao *ao); /* temporarily unlocks */
 _hidden void libxl__ao_abort(libxl__ao *ao);
 _hidden void libxl__ao_complete(libxl__egc *egc, libxl__ao *ao, int rc);
 
-- 
1.7.2.5

  parent reply	other threads:[~2012-02-17 18:43 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-26 17:20 [PATCH v8 00/11] libxl: New event API Ian Jackson
2012-01-26 17:20 ` [PATCH 01/11] .gitignore/.hgignore: New names for ioemu dirs, seabios Ian Jackson
2012-01-27 10:39   ` Ian Campbell
2012-01-26 17:20 ` [PATCH 02/11] xl: fix a couple of memory leaks Ian Jackson
2012-01-27 10:30   ` Ian Campbell
2012-01-26 17:20 ` [PATCH 03/11] libxl: New API for providing OS events to libxl Ian Jackson
2012-01-26 17:20 ` [PATCH 04/11] ocaml, libxl: support "private" fields Ian Jackson
2012-01-27 15:22   ` Ian Campbell
2012-01-26 17:20 ` [PATCH 05/11] libxl: New event generation API Ian Jackson
2012-01-26 17:20 ` [PATCH 06/11] libxl: introduce libxl_fd_set_nonblock, rationalise _cloexec Ian Jackson
2012-01-26 17:20 ` [PATCH 07/11] libxl: Permit multithreaded event waiting Ian Jackson
2012-01-26 17:20 ` [PATCH 08/11] libxl: Asynchronous/long-running operation infrastructure Ian Jackson
2012-02-15 12:25   ` Roger Pau Monné
2012-02-15 12:51     ` Ian Jackson
2012-02-15 13:06       ` Roger Pau Monné
2012-02-17 16:12         ` Ian Jackson
2012-02-17 18:43           ` [PATCH 0/3] libxl: Permit immediate asynchronous completion Ian Jackson
2012-02-17 18:47             ` Ian Jackson
2012-02-20 12:20               ` Roger Pau Monné
2012-02-20 13:44                 ` Ian Jackson
2012-02-17 18:43           ` [PATCH 1/3] libxl: ao: allow immediate completion Ian Jackson
2012-02-20 10:26             ` Ian Campbell
2012-02-20 13:32               ` Ian Jackson
2012-02-17 18:43           ` [PATCH 2/3] libxl: fix hang due to libxl__initiate_device_remove Ian Jackson
2012-02-17 18:43           ` Ian Jackson [this message]
2012-02-20 10:30             ` [PATCH 3/3] libxl: Fix eventloop_iteration over-locking Ian Campbell
2012-01-26 17:20 ` [PATCH 09/11] libxl: New convenience macro CONTAINER_OF Ian Jackson
2012-01-27 15:22   ` Ian Campbell
2012-01-26 17:20 ` [PATCH 10/11] libxl: Introduce libxl__ev_devstate Ian Jackson
2012-01-26 17:20 ` [PATCH 11/11] libxl: Convert to asynchronous: device removal Ian Jackson
2012-01-27 15:25 ` [PATCH v8 00/11] libxl: New event API Ian Campbell
2012-01-27 17:07   ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1329504239-19999-4-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).