From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aganN-0000N3-Gr for qemu-devel@nongnu.org; Thu, 17 Mar 2016 12:29:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aganK-0005Pf-A4 for qemu-devel@nongnu.org; Thu, 17 Mar 2016 12:29:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59912) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aganK-0005Oy-2G for qemu-devel@nongnu.org; Thu, 17 Mar 2016 12:29:06 -0400 Date: Thu, 17 Mar 2016 16:29:01 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20160317162900.GK5966@work-vm> References: <87wpp4m6n1.fsf@blackfin.pond.sub.org> <20160315133916.GM27203@stefanha-x1.localdomain> <20160315135647.GB11728@work-vm> <20160316182343.GE2012@stefanha-x1.localdomain> <20160316182748.GG2246@work-vm> <20160317112508.GG14062@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160317112508.GG14062@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: =?iso-8859-1?Q?Llu=EDs?= Vilanova , Peter Maydell , Markus Armbruster , qemu-devel@nongnu.org * Stefan Hajnoczi (stefanha@redhat.com) wrote: > On Wed, Mar 16, 2016 at 06:27:48PM +0000, Dr. David Alan Gilbert wrote: > > * Stefan Hajnoczi (stefanha@redhat.com) wrote: > > > On Tue, Mar 15, 2016 at 01:56:47PM +0000, Dr. David Alan Gilbert wrote: > > > > > This would put trace_foo() in generated-tracers-virtio-blk.h and > > > > > trace_bar() in generated-tracers-memory.h. Source files using tracing > > > > > would need to include headers for relevant sections. > > > > > > > > > > This way we can narrow the scope of tracing headers and prevent global > > > > > recompilation. > > > > > > > > > > The fly in the ointment is that trace/control.h defines enum > > > > > TraceEventID, a global numbering of all trace events. The enum is used > > > > > in trace/contro.h APIs and also in the simpletrace file format. > > > > > > > > > > If ./trace-event is modified the numbering of trace events could change. > > > > > This would require global recompilation :(. > > > > > > > > > > So in order to avoid global recompilation we need to eliminate enum > > > > > TraceEventID. Perhaps it's possible to use TraceEvent* instead of > > > > > TraceEventID. For the simpletrace backend we would continue to use a > > > > > global ordering but that only affects generated-tracers.c and not header > > > > > files (thankfully!). > > > > > > > > I think the hardest part is going to be understanding all of the different > > > > tracers and the things they need to know; like that thing about simpletrace > > > > needing the single ordering; I don't know what the other backends need > > > > but I bet they've each got their own surprise. > > > > > > > > (And yes, I'd love to split them up - I tend to use traceing quite a bit > > > > in migration now and trying to do a bisect over a big migration patch > > > > series takes ages mostly because of the trace.h changes) > > > > > > Yes, this is the hard part. The simpletrace.py pretty-printing script > > > takes a trace-events file as input. I suppose it could alphabetically > > > sort the trace-events filenames from the command-line to produce a > > > global ordering of trace event IDs. Of course that doesn't work > > > properly if the script is invoked with relative paths from a > > > sub-directory... > > > > > > Perhaps we need a new script during the QEMU build process that produces > > > a trace-event-ids.csv file. Then simpletrace.py could take that instead > > > of processing ./trace-events. > > > > Well that sounds easy - but is it easy to avoid using those fixed IDs > > in any of the trace.h functions that are included everywhere (without > > making them more expensive). > > How about this: > > Instead of using TraceEventID in tracing APIs, pass the TraceEvent > pointer directly. The tracetool.py code generator should produce > "extern TraceEvent trace_event_foo;" definitions in headers so that the > users have direct access to the TraceEvents. OK, so I see TraceEvent has a TraceEventID field; so yes that works easily; it turns out to be a little more expensive though since what was a: trace_events_dstate[id] is now trace_events_dstate[te->id] But hang on, what's the 'sstate' in TraceEvent; do we actually need two state fields if we're passing a TraceEvent pointer around? Dave > Stefan -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK