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
