From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54635) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WS4QF-0006wd-CJ for qemu-devel@nongnu.org; Mon, 24 Mar 2014 08:56:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WS4Q9-00030Z-Ao for qemu-devel@nongnu.org; Mon, 24 Mar 2014 08:56:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44430) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WS4Q9-00030R-3b for qemu-devel@nongnu.org; Mon, 24 Mar 2014 08:56:05 -0400 Date: Mon, 24 Mar 2014 13:55:59 +0100 From: Stefan Hajnoczi Message-ID: <20140324125559.GA4384@stefanha-thinkpad.redhat.com> References: <1395415704-17528-1-git-send-email-stefanha@redhat.com> <20140321163954.GA4107@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140321163954.GA4107@redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.0 v2] configure: add workaround for SystemTap PR13296 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Frank Ch. Eigler" Cc: Paolo Bonzini , qemu-devel@nongnu.org, Peter Maydell On Fri, Mar 21, 2014 at 12:39:54PM -0400, Frank Ch. Eigler wrote: > Hi - > > On Fri, Mar 21, 2014 at 04:28:24PM +0100, Stefan Hajnoczi wrote: > > SystemTap sdt.h sometimes results in compiled probes without sufficient > > information to extract arguments. See code comment for details on this > > workaround. > > > > This patch fixes the apic_reset_irq_delivered() trace event on Fedora 20 > > with gcc-4.8.2-7.fc20 and systemtap-sdt-devel-2.4-2.fc20 on x86_64. > > An alternate solution would focus on this particular trace site. > (It's peculiar because the surrounding code doesn't read the value > being passed to the tracers at all, so the value is not already > available in a register.) > > The benefit of this solution is that the hundreds of other trace sites > are not affected at all. 230 of them (on my qemu-system-X86 builds) > use register-indirect operand expressions currently, and your patch > would force all of those to be turned into actual loads -- i.e., slow > down qemu. > > Please consider whether that performance is worth it, over a > patch as dreadful :-) as the following. > > > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c > index c623fcc6d813..e7bbac1be48b 100644 > --- a/hw/intc/apic_common.c > +++ b/hw/intc/apic_common.c > @@ -117,7 +117,10 @@ void apic_report_irq_delivered(int delivered) > > void apic_reset_irq_delivered(void) > { > - trace_apic_reset_irq_delivered(apic_irq_delivered); > + /* Copy this into a local variable to encourage gcc to emit a plain > + register for a sys/sdt.h marker. */ > + volatile int a_i_d = apic_irq_delivered; > + trace_apic_reset_irq_delivered(a_i_d); > > apic_irq_delivered = 0; > } This is not too bad either. Can I add your Signed-off-by:? Stefan