All of lore.kernel.org
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@citrix.com>
To: <xen-devel@lists.xenproject.org>
Cc: Anthony Perard <anthony.perard@citrix.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	Nick Rosbrook <rosbrookn@ainfosec.com>, Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH v2 2/2] libxl: Add new "notify-only" childproc mode
Date: Fri, 10 Jan 2020 17:58:24 +0000	[thread overview]
Message-ID: <53f13d57-de57-cb29-512a-67d8312fa29a@citrix.com> (raw)
In-Reply-To: <20200109121227.949665-2-george.dunlap@citrix.com>

On 1/9/20 12:12 PM, George Dunlap wrote:
> 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).
> 
> Provide a new external function, libxl_childproc_sigchld_notify(), for
> library users to call when a SIGCHLD happens.  Have this call
> sigchld_handler().
> 
> 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.
> 
> NB an alternate approach would be to make libxl_sigchld_owner_mainloop
> *always* set up and tear down the self-pipe notification mechanisms,
> and then simply expose libxl_childproc_sigchld_notify().  However,
> this would entail grabbing a libxl-wide global lock (across all libxl
> ctx's) twice on every fork.  This should be avoided for callers which
> don't need it.
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

FAOD, with the fixes in your other series, I consider this patch to now
be moot.

 - George

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

  parent reply	other threads:[~2020-01-10 17:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-09 12:12 [Xen-devel] [PATCH v2 1/2] libxl: Get rid of some trailing whitespace George Dunlap
2020-01-09 12:12 ` [Xen-devel] [PATCH v2 2/2] libxl: Add new "notify-only" childproc mode George Dunlap
2020-01-09 17:07   ` George Dunlap
2020-01-10 17:58   ` George Dunlap [this message]
2020-01-13 14:20     ` Ian Jackson
2020-01-10 17:57 ` [Xen-devel] [PATCH v2 1/2] libxl: Get rid of some trailing whitespace 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=53f13d57-de57-cb29-512a-67d8312fa29a@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=rosbrookn@ainfosec.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.