Re: [PATCH] add trace points in irqchip code

[Avi Kivity <avi@redhat.com>]

On 07/07/2009 03:44 PM, Gleb Natapov wrote:
> On Tue, Jul 07, 2009 at 01:06:00PM +0300, Avi Kivity wrote:
>    
>> 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.
>>      
> This will move knowledge how gsi is mapped to pic chips into tracing
> code. This will break if we will add emulation to some fantastic HW with
> number of pics different from 2.
>    

Right, that doesn't work out well in this case.  Something to keep in 
mind in general.

(Could still be handled by defining inline functions or macros to do the 
decoding and using them in TP_fast_assign, but probably not worth it in 
this case)

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