| Message ID | 20190614103801.22619-5-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | libxl: New slow lock + fix libxl_cdrom_insert with QEMU depriv | expand |
Anthony PERARD writes ("[PATCH v2 4/9] libxl: Add optimisation to ev_lock"):
> It will often be the case that the lock is free to grab. So we first
> try to grab it before we have to fork. Even though in this case the
> locks are grabbed in the wrong order in the lock hierarchy (ev_lock
> should be outside of CTX_LOCK), it is fine to try without blocking. If
> that failed, we will release CTX_LOCK and try to grab both lock again
> in the right order.
>
> That optimisation is only enabled in releases (debug=n) so the more
> complicated code with fork is actually exercised.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 6fdcbbddd6..4587a6fc9c 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -35,6 +35,9 @@ ifeq ($(CONFIG_LIBNL),y) CFLAGS_LIBXL += $(LIBNL3_CFLAGS) endif CFLAGS_LIBXL += -Wshadow +ifeq ($(debug),y) +CFLAGS_LIBXL += -DCONFIG_DEBUG +endif LIBXL_LIBS-$(CONFIG_ARM) += -lfdt diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index 3906a0512d..fb1b9bfe52 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -620,6 +620,25 @@ static void ev_lock_prepare_fork(libxl__egc *egc, libxl__ev_lock *lock) } fd = lock->fd; + /* Enable this optimisation only in releases, so the fork code is + * exercised while libxl is built with debug=y. */ +#ifndef CONFIG_DEBUG + /* + * We try to grab the lock before forking as it is likely to be free. + * Even though we are supposed to CTX_UNLOCK before attempting to grab + * the ev_lock, it is fine to do a non-blocking request now with the + * CTX_LOCK held as if that fails we'll try again in a fork (CTX_UNLOCK + * will be called in libxl), that will avoid deadlocks. + */ + int r = flock(fd, LOCK_EX | LOCK_NB); + if (!r) { + libxl_fd_set_cloexec(CTX, fd, 1); + /* We held a lock, no need to fork but we need to check it. */ + ev_lock_child_callback(egc, &lock->child, 0, 0); + return; + } +#endif + pid = libxl__ev_child_fork(gc, &lock->child, ev_lock_child_callback); if (pid < 0) goto out;
It will often be the case that the lock is free to grab. So we first try to grab it before we have to fork. Even though in this case the locks are grabbed in the wrong order in the lock hierarchy (ev_lock should be outside of CTX_LOCK), it is fine to try without blocking. If that failed, we will release CTX_LOCK and try to grab both lock again in the right order. That optimisation is only enabled in releases (debug=n) so the more complicated code with fork is actually exercised. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v2: - new patch tools/libxl/Makefile | 3 +++ tools/libxl/libxl_internal.c | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+)