Re: [PATCH] add trace points in irqchip code

[Avi Kivity <avi@redhat.com>]

On 07/06/2009 04:17 PM, Gleb Natapov wrote:
> Add tracepoint in msi/ioapic/pic set_irq() functions,
> in IPI sending and in the point where IRQ is placed into
> apic's IRR.
>
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index 1d1bb75..e4bcbdd 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -30,6 +30,7 @@
>   #include "irq.h"
>
>   #include<linux/kvm_host.h>
> +#include "trace.h"
>
>   static void pic_lock(struct kvm_pic *s)
>   	__acquires(&s->lock)
> @@ -190,6 +191,8 @@ int kvm_pic_set_irq(void *opaque, int irq, int level)
>   	if (irq>= 0&&  irq<  PIC_NUM_PINS) {
>   		ret = pic_set_irq1(&s->pics[irq>>  3], irq&  7, level);
>   		pic_update_irq(s);
> +		trace_kvm_pic_set_irq(irq>>  3, irq&  7, s->pics[irq>>  3].elcr,
> +				      s->pics[irq>>  3].imr, ret == 0);
>    

It's better to send undecoded arguments (irq, s, ret) and do the 
shifting/masking/derefing in the TP_fast_assign.  This is because 
argument preparation is always compiled inline and executed (so I 
understand; not sure) but TP_fast_assign is out-of-line and only 
executed if the tracepoint is enabled.
> +TRACE_EVENT(kvm_pic_set_irq,
> +	    TP_PROTO(__u8 chip, __u8 pin, __u8 elcr, __u8 imr, int coalesced),
> +	    TP_ARGS(chip, pin, elcr, imr, coalesced),
> +
> +	TP_STRUCT__entry(
> +		__field(	__u8,		chip		)
> +		__field(	__u8,		pin		)
> +		__field(	__u8,		elcr		)
> +		__field(	__u8,		imr		)
> +		__field(	int,		coalesced	)
> +	),
>    

bool is slightly more descriptive.

> +
> +	TP_fast_assign(
> +		__entry->chip		= chip;
> +		__entry->pin		= pin;
> +		__entry->elcr		= elcr;
> +		__entry->imr		= imr;
> +		__entry->coalesced	= coalesced;
> +	),
> +
> +	TP_printk("chip=%u pin=%u (%s%s)%s",
> +		  __entry->chip, __entry->pin,
> +		  (__entry->elcr&  (1<<  __entry->pin)) ? "level":"edge",
> +		  (__entry->imr&  (1<<  __entry->pin)) ? "|masked":"",
> +		  __entry->coalesced ? " (coalesced)" : "")
> +);
>    

kvm style is not to use equals signs.

> +
> +	TP_printk("dst=%x vec=%u (%s|%s|%s|%s|%s)",
> +		  __entry->dest_id, (u8)__entry->icr_low,
> +		  __print_symbolic((__entry->icr_low>>  8&  0x7),
> +				   kvm_deliver_mode),
> +		  (__entry->icr_low&  (1<<11)) ? "logical":"physical",
> +		  (__entry->icr_low&  (1<<14)) ? "assert":"de-assert",
> +		  (__entry->icr_low&  (1<<15)) ? "level":"edge",
>    

Spaces around :.

> +	TP_STRUCT__entry(
> +		__field(	__u32,		apicid		)
> +		__field(	__u8,		dm		)
> +		__field(	__u8,		tm		)
> +		__field(	__u8,		vec		)
> +		__field(	__u8,		coalesced	)
> +	),
>    

bool.

> +
> +TRACE_EVENT(kvm_msi_set_irq,
> +	    TP_PROTO(__u32 address, __u64 data),
> +	    TP_ARGS(address, data),
> +
> +	TP_STRUCT__entry(
> +		__field(	__u32,		address		)
>    

Make this a u64 just in case.


-- 
error compiling committee.c: too many arguments to function