All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jim Fehlig <jfehlig@suse.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: LibVir <libvir-list@redhat.com>,
	xen-devel@lists.xensource.com,
	Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH 00/12] libxl: fork: SIGCHLD flexibility
Date: Thu, 30 Jan 2014 09:56:16 -0700	[thread overview]
Message-ID: <52EA8430.7010800@suse.com> (raw)
In-Reply-To: <21226.32158.954708.954668@mariner.uk.xensource.com>

Ian Jackson wrote:
> Jim Fehlig writes ("Re: [Xen-devel] [PATCH 00/12] libxl: fork: SIGCHLD flexibility"):
>   
>> Looking at the libvirt code again, it seems a single thread services the
>> event loop. See virNetServerRun() in src/util/virnetserver.c. Indeed, I
>> see the same thread ID in all the timer and fd callbacks. One of the
>> libvirt core devs can correct me if I'm wrong.
>>     
>
> OK.  So just to recap where we stand:
>
>  * I think libxl needs the SIGCHLD flexibility series.  I'll repost
>    that (v3) but it's had hardly any changes.
>   

Ok, thanks.  I'm currently testing on your git branch referenced earlier
in this thread

git://xenbits.xen.org/people/iwj/xen.git#wip.enumerate-pids-v2.1

>  * You need to fix the timer deregistration arrangements in the
>    libvirt/libxl driver to avoid the crash you identified the other day.
>   

Yes, I'm testing a fix now.

>  * Something needs to be done about the 20ms slop in the libvirt event
>    loop (as it could cause libxl to lock up).  If you can't get rid of
>    it in the libvirt core, then adding 20ms to the every requested
>    callback time in the libvirt/libxl driver would work for now.
>   

The commit msg adding the fuzz says

    Fix event test timer checks on kernels with HZ=100
   
    On kernels with HZ=100, the resolution of sleeps in poll() is
    quite bad. Doing a precise check on the expiry time vs the
    current time will thus often thing the timer has not expired
    even though we're within 10ms of the expected expiry time. This
    then causes another pointless sleep in poll() for <10ms. Timers
    do not need to have such precise expiration, so we treat a timer
    as expired if it is within 20ms of the expected expiry time. This
    also fixes the eventtest.c test suite on kernels with HZ=100
   
    * daemon/event.c: Add 20ms fuzz when checking for timer expiry


I could handle this in the libxl driver as you say, but doing so makes
me a bit nervous.  Potentially locking up libxl makes me nervous too :).

>  * I think we can get away with not doing anything about the fd
>    deregistration race in libvirt because both Linux and FreeBSD have
>    behaviours that are tolerable.
>
>  * libxl should have the fd deregistration race fixed in Xen 4.5.
>
> Have you managed to fix the timer deregistration crash, and retest ?
>   

Yes.  I've been running my tests for about 24 hours now with no problems
noted.  The tests include starting/stopping a persistent VM,
creating/stopping a transient VM, rebooting a persistent VM,
saving/restoring a transient VM, and getting info on all of these VMs.

I should probably add saving/restoring a persistent VM to the mix since
the associated libxl_ctx is never freed.  Only when a persistent VM is
undefined is the libxl_ctx freed.

Regards,
Jim

  reply	other threads:[~2014-01-30 16:56 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-17 16:23 [PATCH 00/12] libxl: fork: SIGCHLD flexibility Ian Jackson
2014-01-17 16:23 ` [PATCH 01/12] libxl: fork: Break out checked_waitpid Ian Jackson
2014-01-17 16:23 ` [PATCH 02/12] libxl: fork: Break out childproc_reaped_ours Ian Jackson
2014-01-17 16:23 ` [PATCH 03/12] libxl: fork: Clarify docs for libxl_sigchld_owner Ian Jackson
2014-01-17 16:23 ` [PATCH 04/12] libxl: fork: Document libxl_sigchld_owner_libxl better Ian Jackson
2014-01-17 16:28   ` Ian Campbell
2014-01-17 16:23 ` [PATCH 05/12] libxl: fork: assert that chldmode is right Ian Jackson
2014-01-17 16:23 ` [PATCH 06/12] libxl: fork: Provide libxl_childproc_sigchld_occurred Ian Jackson
2014-01-17 16:24 ` [PATCH 07/12] libxl: fork: Provide ..._always_selective_reap Ian Jackson
2014-01-17 22:17   ` Jim Fehlig
2014-01-17 16:24 ` [PATCH 08/12] libxl: fork: Provide LIBXL_HAVE_SIGCHLD_SELECTIVE_REAP Ian Jackson
2014-01-17 16:24 ` [PATCH 09/12] libxl: fork: Rename sigchld handler functions Ian Jackson
2014-01-20  9:59   ` Ian Campbell
2014-01-17 16:24 ` [PATCH 10/12] libxl: fork: Break out sigchld_installhandler_core Ian Jackson
2014-01-20  9:59   ` Ian Campbell
2014-01-17 16:24 ` [PATCH 11/12] libxl: fork: Break out sigchld_sethandler_raw Ian Jackson
2014-01-20  9:58   ` Ian Campbell
2014-01-20 17:57     ` Ian Jackson
2014-01-17 16:24 ` [PATCH 12/12] libxl: fork: Share SIGCHLD handler amongst ctxs Ian Jackson
2014-01-17 18:13   ` Ian Jackson
2014-01-20  9:56     ` Ian Campbell
2014-01-21 14:40       ` Ian Jackson
2014-01-21 14:53         ` Ian Campbell
2014-01-21 15:09           ` Ian Jackson
2014-01-17 16:37 ` [PATCH 00/12] libxl: fork: SIGCHLD flexibility Ian Jackson
2014-01-17 22:29 ` Jim Fehlig
2014-01-20 18:14   ` Jim Fehlig
2014-01-21 14:46     ` Ian Jackson
2014-01-21 15:11       ` [PATCH 13/12] libxl: events: Break out libxl__pipe_nonblock, _close Ian Jackson
2014-01-21 15:11         ` [PATCH 14/12] libxl: fork: Make SIGCHLD self-pipe nonblocking Ian Jackson
2014-01-21 15:32           ` Ian Campbell
2014-01-21 15:48             ` Ian Jackson
2014-01-21 15:27         ` [PATCH 13/12] libxl: events: Break out libxl__pipe_nonblock, _close Ian Campbell
2014-01-21 15:31           ` Ian Jackson
2014-01-21 15:28     ` [PATCH 00/12] libxl: fork: SIGCHLD flexibility Ian Jackson
2014-01-22  5:32       ` Jim Fehlig
2014-01-23  4:05         ` Jim Fehlig
2014-01-23 10:56           ` Ian Jackson
2014-01-23 21:36             ` Jim Fehlig
2014-01-24  4:27             ` Jim Fehlig
2014-01-24 12:41               ` Ian Jackson
2014-01-24 12:52                 ` Ian Campbell
2014-01-24 15:14                   ` Ian Jackson
2014-01-24 15:18                     ` Ian Jackson
2014-01-24 16:36                     ` Ian Jackson
2014-01-24 16:57                       ` Ian Jackson
2014-01-27  5:39                   ` Jim Fehlig
2014-01-27  5:22                 ` Jim Fehlig
2014-01-27 14:48                   ` Ian Jackson
2014-01-28  1:39                 ` [libvirt] [Xen-devel] " Jim Fehlig
2014-01-28 10:06                   ` Daniel P. Berrange
2014-01-29 16:23                     ` [libvirt] " Ian Jackson
2014-01-30 12:18                   ` [libvirt] [Xen-devel] " Daniel P. Berrange
2014-01-30 16:14                     ` Jim Fehlig
2014-01-30 16:17                       ` Daniel P. Berrange
2014-01-30 16:28                   ` Ian Jackson
2014-01-30 16:56                     ` Jim Fehlig [this message]
2014-01-30 17:12                       ` 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=52EA8430.7010800@suse.com \
    --to=jfehlig@suse.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=libvir-list@redhat.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 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.