From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53093) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WBRLG-0002Td-G2 for qemu-devel@nongnu.org; Thu, 06 Feb 2014 10:58:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WBRL6-0001Wu-Sm for qemu-devel@nongnu.org; Thu, 06 Feb 2014 10:58:18 -0500 Received: from mail-qa0-x233.google.com ([2607:f8b0:400d:c00::233]:59214) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WBRL6-0001Wf-Oe for qemu-devel@nongnu.org; Thu, 06 Feb 2014 10:58:08 -0500 Received: by mail-qa0-f51.google.com with SMTP id f11so3023800qae.38 for ; Thu, 06 Feb 2014 07:58:07 -0800 (PST) Sender: Richard Henderson Message-ID: <52F3B105.80908@twiddle.net> Date: Thu, 06 Feb 2014 07:57:57 -0800 From: Richard Henderson MIME-Version: 1.0 References: <20140131160902.32741.2680.stgit@fimbulvetr.bsc.es> <52F0FFBD.8050801@twiddle.net> <87k3davcs3.fsf@fimbulvetr.bsc.es> In-Reply-To: <87k3davcs3.fsf@fimbulvetr.bsc.es> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, Stefan Hajnoczi On 02/04/2014 12:33 PM, Lluís Vilanova wrote: > Richard Henderson writes: > >> On 01/31/2014 08:09 AM, Lluís Vilanova wrote: >>> Adds the base ability to specify which events in the "trace-events" file may be >>> used to trace guest activity in the TCG code (using the "tcg" event propery). >>> >>> Such events generate an extra set of tracing functions that can be called during >>> TCG code generation and will automatically redirect a call to the appropriate >>> backend-dependent tracing functions when the guest code is executed. >>> >>> Files generating guest code (TCG) must include "trace-tcg.h". Files declaring >>> per-target helpers ("${target}/helper.h") must include >>> "trace/generated-helpers.h". >>> >>> The flow of the generated routines is: >>> >>> >>> [At translation time] >>> >>> * trace_${name}_tcg(bool, TCGv) >>> Declared: "trace/generated-tcg-tracers.h" >>> Defined : "trace/generated-tcg-tracers.h" >>> >>> * gen_helper_trace_${name}_tcg(bool, TCGv) >>> Declared: "trace/generated-helpers.h" >>> Defined : "trace/generated-helpers.h" >>> >>> Automatically transforms all the arguments and allocates them into the >>> appropriate TCG temporary values (which are also freed). Provides a more >>> streamlined interface by allowing events in "trace-events" to take a mix of >>> tracing-supported types and TCG types. >>> >>> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv) >>> Declared: "trace/generated-helpers.h" >>> Defined : "trace/generated-helpers.h" (using helper machinery) >>> >>> The actual TCG helper function, created using QEMU's TCG helper machinery. > >> I suppose I have no major objection to the feature, although frankly it's >> not especially exciting. I can't really imagine ever wanting to bulk trace >> all of the helpers. Tracing specific helpers on a target-by-target basis, >> sure. But that can be done just as easily as adding tracing code to any >> other bit of C. > > I'm not sure I understand this comment. The patch does not add helper tracing > capabilities, but generates a "trace_foo_tcg" routine that can be called during > TCG code generation, and will call "trace_foo" when that TCG code is > executed. Ah, see, I told you I was probably reading the patches wrong. With all the helpers.h changes, I thought this was somehow related to tracing the existing helpers. But the existance of trace_foo_tcg is triggered by the trace-events file? >> I would strongly suggest this is backward. One should perform the check for >> the tracepoint being enabled at translation time before emitting the call to >> the helper in the first place. > > Right, the thing is that dynamically enabling/disabling such events will not > immediately show up in the trace if I do the check at translation time > (trace_foo_tcg), since QEMU will execute the cached TCG translations. I see the > following solutions to ensure traces are accurate: > > * Delay the check until execution time. > > * Check at translation time; flush translation cache when the tracing state of > a TCG event changes. > > * Check at translation time; use multiple translation caches, one for each > possible tracing state of all the TCG-enabled events. > > This series implements the first approach, since it's correct and much > simpler. > > Other patches I did not send implement the third approach, which is quite > efficient if one is dynamically switching the tracing state while executing > mostly-cached code (e.g., profiling the accesses). How often do events change state? My guess is exceedingly rarely. And by "rare" I mean something well under once per minute. ;-) At which point, option 2 would be the best bet, I think. > I can wait for a later series to send the third option, or even implement the > second, but I just wanted to keep this one as simple as possible. Also, > implementing any of these two last approaches would provide minimal overheads on > builds that have such events enabled at compile time. Fair enough. r~