From nobody Wed Apr 24 09:21:14 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=citrix.com ARC-Seal: i=1; a=rsa-sha256; t=1578566869; cv=none; d=zohomail.com; s=zohoarc; b=emqNZv8KZzJ5VKCKEsxaHY+Jwk6JxDkfsjvRnr/u2MzVQdhm6S16JLRi/c5JMELwh47Ex+zVs2d3pUrUIuVAUK+ozkRor9aw3SdYlNzSsk0GnqujuNc74WTUt45Sep1ZT57nq0h023be19k7xkxWsPwyI+fg2NalOqU7o8jFhVA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1578566869; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=7KzpYnk3SgtAM67ADwj/NjO+d5IoiSBnopsXG42I+S8=; b=fhktMzEq8QFw4uNFecvjjmb4uKhCiOm33pnI4IRNIpuS1uQImT9xWnnKg/4wgUhfPAnECOUfCZySbhxRACy0cVYJoVlnTmhrK/WG7ZuMeV69hXOfHlrKbvuDha0dsFjkoe6JqutTrA+q6Wo53BKKlHKZjxtyV5Xrkmx05VwtA7o= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1578566869425112.63870680400123; Thu, 9 Jan 2020 02:47:49 -0800 (PST) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ipVLJ-00032S-FP; Thu, 09 Jan 2020 10:47:09 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ipVLH-00032M-UW for xen-devel@lists.xenproject.org; Thu, 09 Jan 2020 10:47:07 +0000 Received: from esa5.hc3370-68.iphmx.com (unknown [216.71.155.168]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 5c285b8e-32cd-11ea-a985-bc764e2007e4; Thu, 09 Jan 2020 10:46:58 +0000 (UTC) X-Inumbo-ID: 5c285b8e-32cd-11ea-a985-bc764e2007e4 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1578566818; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=WxPZNqjUb6YsNmhHfebMYWTqHAOgpVsdhtFL90kC53g=; b=co/2pvkDMdGgkLzmfhngsc8t+gYr3A7jTq5TlLmqH9nZ2hyftV9GhwUs TQxcfgIBFAN7TktaQy9KAaUQYPr/qy9sbWripqRr5ygrTMUcMC6q5ugj4 D1Ba7ilo6WT61SFXv9S8ku9af+f+59MrrtWU1hcO/60rJqPy8F+HZyEOI M=; Authentication-Results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=george.dunlap@citrix.com; spf=Pass smtp.mailfrom=George.Dunlap@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Received-SPF: None (esa5.hc3370-68.iphmx.com: no sender authenticity information available from domain of george.dunlap@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa5.hc3370-68.iphmx.com; envelope-from="George.Dunlap@citrix.com"; x-sender="george.dunlap@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa5.hc3370-68.iphmx.com: domain of George.Dunlap@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa5.hc3370-68.iphmx.com; envelope-from="George.Dunlap@citrix.com"; x-sender="George.Dunlap@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa5.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa5.hc3370-68.iphmx.com; envelope-from="George.Dunlap@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: yWAWD5uHRHdkFV83/I7FFaLvpFmITWqdf2cXoHn2l1q94FFgZlfwVNGxr4YDpDmWt1HHXSJp0A NqmV5kR4muyfTdjB5wKb8Dz9cChm4Ekn7w9JqjBn9zyW+GZ+1TH0zjXO0zBhfBYd9bFc6+SlBP DbZZNY0tuI1mNcgui9HXO0eEXn9sLAPclUThjPyXMUPfTICV655513TqmsF2rapmbbY68K4am3 VvbS4XtdlgMG1dIVL05dpiQQGNbGwU5l2YqOMuyfpd3jzUPaPDHnqmSWvvDNYEZf5EYv8qx6ao yGA= X-SBRS: 2.7 X-MesageID: 11046600 X-Ironport-Server: esa5.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.69,413,1571716800"; d="scan'208";a="11046600" From: George Dunlap To: Date: Thu, 9 Jan 2020 10:46:53 +0000 Message-ID: <20200109104653.866503-1-george.dunlap@citrix.com> X-Mailer: git-send-email 2.24.1 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH] libxl: Add new "notify-only" childproc mode X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Anthony Perard , Ian Jackson , George Dunlap , Wei Liu , Nick Rosbrook Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) libxl needs to be able to know when processes it forks have completed. At the moment, libxl has two basic mode (with some variations). In one mode -- libxl_sigchld_owner_libxl* -- libxl sets up its own SIGCHLD signal handler, and also handles the event loop that allows libxl to safely block until the child in question is finished (using a self-pipe for the SIGCHLD handler to notify the waiters that it's time to look for reaped children). In the other mode, libxl does not set up the SIGCHLD handler, nor does it do anything with processing the event loop; it expects the library caller to handle the event loop itself. The golang runtime manages its own processes, and thus must use SIGCHLD itself; and it has an easy way for other users to get SIGCHLD notifications. However, because its event loop is hidden away behind abstractions, it's not easy to hook into; and there's no need -- the golang runtime assumes that C function calls may block, and handles everything behind the scenes. Introduce a new mode, libxl_sigchld_owner_notify, in which libxl sets up the SIGCHLD event handling machinery, but relies on the caller to tell it when a SIGCHLD happens. Call these two actions "notify" (for the self-pipe notification machinery) and "handler" (for the actual SIGCHLD handler). Move the actual notification handler into its own function. Have sigchld_handler() call this function, and provide a new external function, libxl_childproc_sigchld_notify(), for library users to call when a SIGCHLD happens. Rename chldmode_ours() to chldmode_notify(), and use it to determine whether to set up the notification chain. When setting up the notification chain, do everything except setting up the signal handler in "notify-only" mode. defer_sigchld() and release_sigchld() do two things: they modify the signal handler, and grab and release locks. Refactor these so that they grab and release the locks correctly in "notify-only" mode, but don't tweak the signal handler unless it's been set up. With the golang bindings ported to use this change, domain creation works. Signed-off-by: George Dunlap --- CC: Ian Jackson CC: Wei Liu CC: Anthony Perard CC: Nick Rosbrook --- tools/libxl/libxl_event.h | 32 +++++++++++++++++- tools/libxl/libxl_fork.c | 64 +++++++++++++++++++++++++----------- tools/libxl/libxl_internal.h | 4 +-- 3 files changed, 78 insertions(+), 22 deletions(-) diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h index d1517f7456..8001571cf1 100644 --- a/tools/libxl/libxl_event.h +++ b/tools/libxl/libxl_event.h @@ -441,7 +441,7 @@ void libxl_osevent_occurred_timeout(libxl_ctx *ctx, voi= d *for_libxl) * * A program which does this must call libxl_childproc_setmode. * There are three options: - *=20 + * * libxl_sigchld_owner_libxl: * * While any libxl operation which might use child processes @@ -466,6 +466,14 @@ void libxl_osevent_occurred_timeout(libxl_ctx *ctx, vo= id *for_libxl) * has multiple libxl ctx's, it must call libxl_childproc_exited * on each ctx.) * + * libxl_sigchld_owner_mainloop_notify: + * + * The application must install a SIGCHLD handler, but will + * notify libxl when a sigchld happened by calling + * libxl_childproc_sigchld_notify. libxl will arrange to reap + * only its own children. This needs to be called only once, + * even for applications which have multiple libxl ctx's. + * * libxl_sigchld_owner_libxl_always: * * The application expects this libxl ctx to reap all of the @@ -494,6 +502,12 @@ typedef enum { * arrange to (un)block or catch SIGCHLD. */ libxl_sigchld_owner_mainloop, =20 + /* Application promises to discover when SIGCHLD occurs and call + * libxl_childproc_sigchld_notify (perhaps from within a signal + * handler). libxl will not itself arrange to (un)block or catch + * SIGCHLD. */ + libxl_sigchld_owner_mainloop_notify, + /* libxl owns SIGCHLD all the time, and the application is * relying on libxl's event loop for reaping its children too. */ libxl_sigchld_owner_libxl_always, @@ -590,6 +604,22 @@ int libxl_childproc_reaped(libxl_ctx *ctx, pid_t, int = status) void libxl_childproc_sigchld_occurred(libxl_ctx *ctx) LIBXL_EXTERNAL_CALLERS_ONLY; =20 +/* + * This function is for an application which owns SIGCHLD but still + * expects libxl to handle its own event loops. + * + * Such an the application must notify libxl, by calling this + * function, that a SIGCHLD occurred. libxl will then notify all + * appropriate event loops to check for reaped children.. + * + * May be called only by an application which has called setmode with + * chldowner =3D=3D libxl_sigchld_owner_mainloop_notify. + * + * MAY be called from within a signal handler. + */ +void libxl_childproc_sigchld_notify(void) + LIBXL_EXTERNAL_CALLERS_ONLY; + =20 /* * An application which initialises a libxl_ctx in a parent process diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c index 0f1b6b518c..f350246576 100644 --- a/tools/libxl/libxl_fork.c +++ b/tools/libxl/libxl_fork.c @@ -78,7 +78,7 @@ static void atfork_unlock(void) int libxl__atfork_init(libxl_ctx *ctx) { int r, rc; - =20 + atfork_lock(); if (atfork_registered) { rc =3D 0; goto out; } =20 @@ -227,12 +227,9 @@ static pid_t checked_waitpid(libxl__egc *egc, pid_t wa= nt, int *status) static void sigchld_selfpipe_handler(libxl__egc *egc, libxl__ev_fd *ev, int fd, short events, short revents); =20 -static void sigchld_handler(int signo) +static void sigchld_notify(void) { - /* This function has to be reentrant! Luckily it is. */ - libxl_ctx *notify; - int esave =3D errno; =20 int r =3D pthread_mutex_lock(&sigchld_defer_mutex); assert(!r); @@ -244,10 +241,28 @@ static void sigchld_handler(int signo) =20 r =3D pthread_mutex_unlock(&sigchld_defer_mutex); assert(!r); +} + +static void sigchld_handler(int signo) +{ + /* This function has to be reentrant! Luckily it is. */ + + int esave =3D errno; + + sigchld_notify(); =20 errno =3D esave; } =20 +void libxl_childproc_sigchld_notify(void) +{ + /* + * XXX: We don't need a ctx here for functionality sake; should we + * take one just so we can make sure we're in the right mode? + */ + sigchld_notify(); +} + static void sigchld_sethandler_raw(void (*handler)(int), struct sigaction = *old) { struct sigaction ours; @@ -288,9 +303,8 @@ static void sigchld_handler_when_deferred(int signo) =20 static void defer_sigchld(void) { - assert(sigchld_installed); - - sigchld_sethandler_raw(sigchld_handler_when_deferred, 0); + if ( sigchld_installed ) + sigchld_sethandler_raw(sigchld_handler_when_deferred, 0); =20 /* Now _this thread_ cannot any longer be interrupted by the * signal, so we can take the mutex without risk of deadlock. If @@ -303,12 +317,12 @@ static void defer_sigchld(void) =20 static void release_sigchld(void) { - assert(sigchld_installed); - int r =3D pthread_mutex_unlock(&sigchld_defer_mutex); assert(!r); =20 - sigchld_sethandler_raw(sigchld_handler, 0); + if ( sigchld_installed ) + sigchld_sethandler_raw(sigchld_handler, 0); + if (sigchld_occurred_while_deferred) { sigchld_occurred_while_deferred =3D 0; /* We might get another SIGCHLD here, in which case @@ -326,7 +340,7 @@ static void sigchld_removehandler_core(void) /* idempot= ent */ { struct sigaction was; int r; - =20 + if (!sigchld_installed) return; =20 @@ -375,6 +389,11 @@ static void sigchld_user_remove(libxl_ctx *ctx) /* ide= mpotent */ =20 void libxl__sigchld_notneeded(libxl__gc *gc) /* non-reentrant, idempotent = */ { + /* + * NB that we don't need to special-case + * libxl_sigchld_owner_mainloop_notify; sigchld_removehandler_core + * will DTRT if no signal handler has been set up. + */ sigchld_user_remove(CTX); libxl__ev_fd_deregister(gc, &CTX->sigchld_selfpipe_efd); } @@ -399,7 +418,9 @@ int libxl__sigchld_needed(libxl__gc *gc) /* non-reentra= nt, idempotent */ if (!CTX->sigchld_user_registered) { atfork_lock(); =20 - sigchld_installhandler_core(); + if (CTX->childproc_hooks->chldowner !=3D libxl_sigchld_owner_mainl= oop_notify) { + sigchld_installhandler_core(); + } =20 defer_sigchld(); =20 @@ -416,13 +437,15 @@ int libxl__sigchld_needed(libxl__gc *gc) /* non-reent= rant, idempotent */ return rc; } =20 -static bool chldmode_ours(libxl_ctx *ctx, bool creating) +/* Do we need the sigchld notification machinery? */ +static bool chldmode_notify(libxl_ctx *ctx, bool creating) { switch (ctx->childproc_hooks->chldowner) { case libxl_sigchld_owner_libxl: return creating || !LIBXL_LIST_EMPTY(&ctx->children); case libxl_sigchld_owner_mainloop: return 0; + case libxl_sigchld_owner_mainloop_notify: case libxl_sigchld_owner_libxl_always: case libxl_sigchld_owner_libxl_always_selective_reap: return 1; @@ -432,7 +455,7 @@ static bool chldmode_ours(libxl_ctx *ctx, bool creating) =20 static void perhaps_sigchld_notneeded(libxl__gc *gc) { - if (!chldmode_ours(CTX, 0)) + if (!chldmode_notify(CTX, 0)) libxl__sigchld_notneeded(gc); } =20 @@ -440,7 +463,7 @@ static int perhaps_sigchld_needed(libxl__gc *gc, bool c= reating) { int rc; =20 - if (chldmode_ours(CTX, creating)) { + if (chldmode_notify(CTX, creating)) { rc =3D libxl__sigchld_needed(gc); if (rc) return rc; } @@ -556,13 +579,16 @@ static void sigchld_selfpipe_handler(libxl__egc *egc,= libxl__ev_fd *ev, int e =3D libxl__self_pipe_eatall(selfpipe); if (e) LIBXL__EVENT_DISASTER(egc, "read sigchld pipe", e, 0); =20 - if (CTX->childproc_hooks->chldowner - =3D=3D libxl_sigchld_owner_libxl_always_selective_reap) { + switch (CTX->childproc_hooks->chldowner) { + case libxl_sigchld_owner_libxl_always_selective_reap: + case libxl_sigchld_owner_mainloop_notify: childproc_checkall(egc); return; + default: + ; } =20 - while (chldmode_ours(CTX, 0) /* in case the app changes the mode */) { + while (chldmode_notify(CTX, 0) /* in case the app changes the mode */)= { int status; pid_t pid =3D checked_waitpid(egc, -1, &status); =20 diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index b5adbfe4b7..bfbee9451e 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -161,7 +161,7 @@ #endif /* all of these macros preserve errno (saving and restoring) */ =20 -/*=20 +/* * A macro to help retain the first failure in "do as much as you can" * situations. Note the hard-coded use of the variable name `rc`. */ @@ -1367,7 +1367,7 @@ typedef struct { const char *shim_cmdline; const char *pv_cmdline; =20 - /*=20 + /* * dm_runas: If set, pass qemu the `-runas` parameter with this * string as an argument * dm_kill_uid: If set, the devicemodel should be killed by --=20 2.24.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel