From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VO5tI-0002Ps-R3 for qemu-devel@nongnu.org; Mon, 23 Sep 2013 09:09:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VO5tA-000753-9A for qemu-devel@nongnu.org; Mon, 23 Sep 2013 09:09:28 -0400 Received: from mail-ee0-x230.google.com ([2a00:1450:4013:c00::230]:36667) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VO5tA-00074s-0z for qemu-devel@nongnu.org; Mon, 23 Sep 2013 09:09:20 -0400 Received: by mail-ee0-f48.google.com with SMTP id l10so1730375eei.35 for ; Mon, 23 Sep 2013 06:09:19 -0700 (PDT) Date: Mon, 23 Sep 2013 15:09:13 +0200 From: Stefan Hajnoczi Message-ID: <20130923130913.GB5814@stefanha-thinkpad.redhat.com> References: <1373917279-15360-1-git-send-email-borntraeger@de.ibm.com> <20130716030511.GF32278@stefanha-thinkpad.redhat.com> <20130716141728.787f6b2b@bee> <20130717020815.GC26311@stefanha-thinkpad.redhat.com> <20130906160608.2c7a64c5@bee> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130906160608.2c7a64c5@bee> Subject: Re: [Qemu-devel] [PATCH 02/12] trace+libvirt: start trace processing thread in final child process List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Mueller Cc: Christian Borntraeger , qemu-devel@nongnu.org, =?iso-8859-1?Q?Llu=EDs?= On Fri, Sep 06, 2013 at 04:06:08PM +0200, Michael Mueller wrote: > On Wed, 17 Jul 2013 10:08:15 +0800 > Stefan Hajnoczi wrote: > > > On Tue, Jul 16, 2013 at 02:17:28PM +0200, Michael Mueller wrote: > > > On Tue, 16 Jul 2013 11:05:11 +0800 > > > Stefan Hajnoczi wrote: > > > > > > > On Mon, Jul 15, 2013 at 09:41:19PM +0200, Christian Borntraeger wrote: > > > > > When running with trace backend e.g. "simple" the writer thread > > > > > needs to be implemented in the same process context as the trace > > > > > points that will be processed. Under libvirtd control, qemu gets > > > > > first started in daemonized mode to privide its capabilities. > > > > > Creating the writer thread in the initial process context then > > > > > leads to a dead lock because the thread gets termined together with > > > > > the initial parent. (-daemonize) This results in stale qemu > > > > > processes. Fix this by deferring trace initialization. > > > > > > > > I don't think this works since trace events will fill up trace_buf[] > > > > and eventually invoke flush_trace_file(). > > > > > > > > At that point we use trace_available_cond and trace_empty_cond, which > > > > may be NULL in Glib <2.31.0. > > > > > > > > Perhaps this can be made safe by checking trace_writeout_enabled. It > > > > will be false before the backend has been initialized. > > > > > > > > Stefan > > > > > > > > > > You mean something like this. I'll give it a try: > > > > > > --- > > > trace/simple.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > --- a/trace/simple.c > > > +++ b/trace/simple.c > > > @@ -53,7 +53,7 @@ static GCond *trace_empty_cond; > > > #endif > > > > > > static bool trace_available; > > > -static bool trace_writeout_enabled; > > > +static bool trace_writeout_enabled = false; > > > > static bool is automatically initialized to false. > > > > > enum { > > > TRACE_BUF_LEN = 4096 * 64, > > > @@ -427,5 +427,6 @@ bool trace_backend_init(const char *even > > > atexit(st_flush_trace_buffer); > > > trace_backend_init_events(events); > > > st_set_trace_file(file); > > > + trace_writeout_enabled = false; > > > > I was thinking along the lines of trace_record_finish() not calling > > flush_trace_file() if trace_writeout_enabled is false. > > > > Stefan > > > > I just looked into it again and think that it is save the way I suggested, because as long > trace_backend_init() isn't called, also trace_backend_init_events() hasn't registered any > events. Thus no trace records will be written and can fill up the trace buffer. Good point, Michael. Do you mind resending your latest code rebased onto qemu.git/master? It's been a while since we discussed these patches and I'd like to make sure we have a fresh email thread to review and complete the merge. Stefan