From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46031) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHwgI-0005oU-12 for qemu-devel@nongnu.org; Fri, 06 Sep 2013 10:06:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VHwg8-0006aq-T7 for qemu-devel@nongnu.org; Fri, 06 Sep 2013 10:06:37 -0400 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:44495) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHwg8-0006aO-LS for qemu-devel@nongnu.org; Fri, 06 Sep 2013 10:06:28 -0400 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 6 Sep 2013 15:03:13 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 0FE5F2190061 for ; Fri, 6 Sep 2013 15:06:23 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r86E6BrU48037894 for ; Fri, 6 Sep 2013 14:06:11 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r86E6KmQ008611 for ; Fri, 6 Sep 2013 08:06:22 -0600 Date: Fri, 6 Sep 2013 16:06:08 +0200 From: Michael Mueller Message-ID: <20130906160608.2c7a64c5@bee> In-Reply-To: <20130717020815.GC26311@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> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Stefan Hajnoczi Cc: Christian Borntraeger , qemu-devel@nongnu.org, =?UTF-8?B?TGx1w61z?= 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. Michael