From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34646 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PFUAk-0003Kw-3r for qemu-devel@nongnu.org; Mon, 08 Nov 2010 11:02:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PFUAi-0003Yy-Dq for qemu-devel@nongnu.org; Mon, 08 Nov 2010 11:02:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:25929) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PFUAi-0003Yg-6M for qemu-devel@nongnu.org; Mon, 08 Nov 2010 11:02:16 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oA8G2FUF008820 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 8 Nov 2010 11:02:15 -0500 Received: from file.fab.redhat.com (file.fab.redhat.com [10.33.63.6]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id oA8G2DmT025089 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 8 Nov 2010 11:02:14 -0500 Received: from file.fab.redhat.com (localhost.localdomain [127.0.0.1]) by file.fab.redhat.com (8.13.1/8.13.1) with ESMTP id oA8G2Dwu022037 for ; Mon, 8 Nov 2010 16:02:13 GMT Received: (from berrange@localhost) by file.fab.redhat.com (8.13.1/8.13.1/Submit) id oA8G2DwL022033 for qemu-devel@nongnu.org; Mon, 8 Nov 2010 16:02:13 GMT Date: Mon, 8 Nov 2010 16:02:13 +0000 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] [RFC] tracing: consistent usage of "disable" in "trace-events" Message-ID: <20101108160212.GX26714@redhat.com> References: <8762w7512w.fsf@ginnungagap.bsc.es> <20101108152556.GU26714@redhat.com> <877hgn3j4x.fsf@ginnungagap.bsc.es> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <877hgn3j4x.fsf@ginnungagap.bsc.es> Content-Transfer-Encoding: quoted-printable Reply-To: "Daniel P. Berrange" List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Mon, Nov 08, 2010 at 04:55:10PM +0100, Llu=C3=ADs wrote: > Daniel P Berrange writes: >=20 > > On Mon, Nov 08, 2010 at 03:42:15PM +0100, Llu=C3=ADs wrote: > >> On the current implementation, the "disable" keyword in "trace-event= s" > >> has different semantics, depending on the backend: > >>=20 > >> * nop : ignored (not a problem) > >> * simple : enables tracing, but sets dynamic state to disable > >> * ust : disables tracing (uses nop backend) > >> * dtrace : same as simple > >>=20 > >> Would it be possible to just use nop whenever the event is disabled = in > >> trace-events? If you agree I can cook the patch, as it's pretty simp= le. >=20 > > I don't particularly see the point of the 'disable' keyword existing = at > > all, unless there are performance implications for a particular trace= =20 > > backend. For the DTrace backend I strip & ignore the disable keyword > > because probes that are compiled in, reduce to a inline conditional=20 > > check that has no serious overhead when no trace client is active. >=20 > I think the same is applicable to the UST backend. >=20 > But it is not true when tracing guest events (e.g., memory > accesses). Not because of the backend, but because of the frequency of > appearance of such events. >=20 > In this case, the auto-generated code is on the lines of (I haven't yet > posted the patch series producing this): >=20 > [called during TCG code generation -- e.g., translate.c] >=20 > #define TRACE_CURR_CPU_STATE_SET (cpu_single_env->trace_state_set) > #define trace_guest_vmem_cpu_event 0 // number of per-CPU trace event > =20 > static inline void trace_gen_guest_vmem (TCGv_i64 addr, uint32_t size= , uint32_t write) > { > if (TRACE_CURR_CPU_STATE_SET & (1 << trace_guest_vmem_cpu_event)) = { > gen_helper_proxy_guest_vmem(addr, tcg_const_i32(size), tcg_cons= t_i32(write)); > } > } >=20 > [extra helper functions -- declared at helper.h] > =20 > void helper_proxy_guest_vmem (uint64_t addr, uint32_t size, uint32_t = write) > { > trace_guest_vmem(addr, size, write); > } >=20 > (*) A state set is a bitset with the events that have been declared wit= h > the "gen" keyword in "trace-events". >=20 > This code has indeed a performance cost, so I opted to follow the > approach taken by the UST backend ("disable" produces a trace event wit= h > "nop"). When I opted for this, only simple and ust where in 'tracetool'. >=20 > In any case, there might appear other events that could have performanc= e > implications, although I understand the ease of usage of having all > trace events available by default. Ok, I agree that if we have tracepoints in the kind of places in TCG you describe, then this could have measurable performance impact. > That's why I would rather declare all trace-events without the "disable= " > keyword, and leave it only on those events that are known to have a hig= h > frequency, as no backend should have so poor performance as to force > events to "disappear". This sounds like a reasonable plan to me.=20 Regards, Daniel --=20 |: Red Hat, Engineering, London -o- http://people.redhat.com/berrang= e/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.o= rg :| |: http://autobuild.org -o- http://search.cpan.org/~danber= r/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 95= 05 :|