From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33241) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8TML-00059F-PQ for qemu-devel@nongnu.org; Tue, 27 Sep 2011 04:49:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R8TMK-0001XH-QW for qemu-devel@nongnu.org; Tue, 27 Sep 2011 04:49:49 -0400 Received: from mail7.hitachi.co.jp ([133.145.228.42]:51104) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8TMK-0001XC-Aq for qemu-devel@nongnu.org; Tue, 27 Sep 2011 04:49:48 -0400 Message-ID: <4E818E21.5010501@hitachi.com> Date: Tue, 27 Sep 2011 17:49:37 +0900 From: Masami Hiramatsu MIME-Version: 1.0 References: <1317110422-25005-1-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1317110422-25005-1-git-send-email-stefanha@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] tracetool: Omit useless QEMU_*_ENABLED() check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Josh Stone , qemu-devel@nongnu.org (2011/09/27 17:00), Stefan Hajnoczi wrote: > SystemTap provides a "semaphore" that can optionally be tested before > executing a trace event. The purpose of this mechanism is to skip > expensive tracing code when the trace event is disabled. > > For example, some applications may have trace events that format or > convert strings for trace events. This expensive processing should only > be done in the case where the trace event is enabled. > > Since QEMU's generated trace events never have such special-purpose > code, there is no reason to add the semaphore check. > > Signed-off-by: Stefan Hajnoczi Thanks Stefan, The systemtap sdt.h just makes a space with NOP for inserting a breakpoint. However, ENABLED() check adds another static variable checking for each tracepoint. So without any clear reason, ENABLED() should not be used. I've checked that this can remove useless conditional jumps from the code. Without this patch: 0000000000420130 : [snip] 420146: 48 89 44 24 08 mov %rax,0x8(%rsp) 42014b: 31 c0 xor %eax,%eax // Here, this check a semaphore 42014d: 66 83 3d 0f da 58 00 cmpw $0x0,0x58da0f(%rip) 420154: 00 // And a conditional jump 420155: 75 3e jne 420195 420157: 48 8b 1d 4a df 58 00 mov 0x58df4a(%rip),%rbx [snip] 420192: 41 5c pop %r12 420194: c3 retq 420195: 90 nop // jump to here 420196: eb bf jmp 420157 With this patch: 0000000000420130 : [snip] 420146: 48 89 44 24 08 mov %rax,0x8(%rsp) 42014b: 31 c0 xor %eax,%eax 42014d: 90 nop // We have just one NOP 42014e: 48 8b 1d 13 d7 58 00 mov 0x58d713(%rip),%rbx 420155: 48 85 db test %rbx,%rbx Reviewed-by: Masami Hiramatsu > --- > scripts/tracetool | 4 +--- > 1 files changed, 1 insertions(+), 3 deletions(-) > > diff --git a/scripts/tracetool b/scripts/tracetool > index 4c9951d..ffe3eba 100755 > --- a/scripts/tracetool > +++ b/scripts/tracetool > @@ -415,9 +415,7 @@ linetoh_dtrace() > # Define an empty function for the trace event > cat < static inline void trace_$name($args) { > - if (QEMU_${nameupper}_ENABLED()) { > - QEMU_${nameupper}($argnames); > - } > + QEMU_${nameupper}($argnames); > } > EOF > } -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com