From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34602) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1REggM-0008Fs-P9 for qemu-devel@nongnu.org; Fri, 14 Oct 2011 08:16:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1REggI-0006Li-Hk for qemu-devel@nongnu.org; Fri, 14 Oct 2011 08:16:10 -0400 Received: from mail-wy0-f175.google.com ([74.125.82.175]:55084) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1REggI-0006Le-DQ for qemu-devel@nongnu.org; Fri, 14 Oct 2011 08:16:06 -0400 Received: by wyh5 with SMTP id 5so4264193wyh.34 for ; Fri, 14 Oct 2011 05:16:05 -0700 (PDT) Date: Fri, 14 Oct 2011 13:16:01 +0100 From: Stefan Hajnoczi Message-ID: <20111014121601.GA753@stefanha-thinkpad.localdomain> References: <1318439659-7525-1-git-send-email-wudxw@linux.vnet.ibm.com> <4E969D10.4060808@vnet.linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E969D10.4060808@vnet.linux.ibm.com> Subject: Re: [Qemu-devel] [PATCH 0/6] trace: Add support for trace events grouping List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Wu Cc: Mark Wu , Stefan Hajnoczi , qemu-devel@nongnu.org On Thu, Oct 13, 2011 at 04:10:56PM +0800, Mark Wu wrote: > On 10/13/2011 01:14 AM, Mark Wu wrote: > >This series add support for trace events grouping. The state of a given group > >of trace events can be queried or changed in bulk by the following monitor > >commands: > > > >* info trace-groups > > View available trace event groups and their state. State 1 means enabled, > > state 0 means disabled. > > > >* trace-group NAME on|off > > Enable/disable a given trace event group. > > > >A group of trace events can also be enabled in early running stage through > >adding its group name prefixed with "group:" to trace events list file > >which is passed to "-trace events". > > > >Mark Wu (6): > > trace: Make "tracetool" generate a group list > > trace: Add HMP monitor commands for trace events group > > trace: Add trace events group implementation in the backend "simple" > > trace: Add trace events group implementation in the backend "stderr" > > trace: Enable "-trace events" argument to control initial state of > > groups > > trace: Update doc for trace events group > > > > docs/tracing.txt | 29 ++++++++++++++-- > > hmp-commands.hx | 14 ++++++++ > > monitor.c | 22 ++++++++++++ > > scripts/tracetool | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++- > > trace-events | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ > > trace/control.c | 17 +++++++++ > > trace/control.h | 9 +++++ > > trace/default.c | 15 ++++++++ > > trace/simple.c | 30 +++++++++++++++++ > > trace/simple.h | 7 ++++ > > trace/stderr.c | 32 ++++++++++++++++++ > > trace/stderr.h | 7 ++++ > > 12 files changed, 359 insertions(+), 5 deletions(-) > > > Sorry, there're some coding style problems in the patches. I have > fixed them and will send out later in order to see if there's any > other problem coming up. :) Hi Mark, Please run scripts/checkpatch.pl on the patches if you haven't already. It will point out many of the coding style issues. I think we can get the same convenience by adding wildcard trace event support. For example, virtio-blk trace events could be enabled using: trace-event virtio_blk_* on This doesn't work for the memory allocation functions because their names do not share a common unique prefix, e.g. g_malloc and qemu_memalign. However, most related trace events do share a common unique prefix. Wildcards don't require special comments in trace-events so it eliminates the problem of people forgetting to add groups when they add trace events to QEMU. The other advantage is that wildcards can select fine-grained sets of trace events, like ecc_mem_writel_mer, ecc_mem_writel_mdr, etc. ecc_mem_writel_* selects all these related trace events (they are a subset of the hw/eccmemctl.c trace events). This is not possible with the groups patches since each trace-event can only be in one group; either we have a high-level hw/eccmemctl.c group or a lower-level ecc_mem_writel group but both is not possible. I suggest adding wildcard trace event matching instead of adding groups. The code changes for wildcards should be much smaller than for groups. What do you think? Stefan