kvm mmu tracing quirks

kvm mmu tracing quirks

[Jan Kiszka]

Hi Xiao,

while tracing a guest with all events enabled, I notices some issues
with kvm mmu instrumentations. A critical one:

TRACE_EVENT(
	kvm_mmu_audit,
	TP_PROTO(struct kvm_vcpu *vcpu, int audit_point),
	TP_ARGS(vcpu, audit_point),

	TP_STRUCT__entry(
		__field(struct kvm_vcpu *, vcpu)
		__field(int, audit_point)
	),

	TP_fast_assign(
		__entry->vcpu = vcpu;
		__entry->audit_point = audit_point;
	),

	TP_printk("vcpu:%d %s", __entry->vcpu->cpu,
		  audit_point_name[__entry->audit_point])
);

Saving the vcpu reference to the trace buffer can break on dump if the
vcpu was destroyed in the meantime. I was about to fix that by saving
vcpu->cpu instead, but then I wondered what kind of information you
actually need here. The triggering host cpu is automatically recorded by
ftrace anyway. Can you comment on this / clean it up? Also, it would be
nice to use __print_symbolic for translating audit_point to a string as
that would also work for trace-cmd/kernelshark.

And then we have tracepoints for kvm_mmu_sync_page, kvm_mmu_unsync_page
and kvm_mmu_prepare_zap_page. All take a kvm_mmu_page as argument but do
nothing with it. On the other hand, the kvm plugin of trace-cmd expects
values like gfn, role, or unsync from those. Maybe you can have a look.

The latter topic is just for the sake of cleanness, but the former one
is actually problematic even if you are not interested in all mmu
details but only enable those events by chance.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: kvm mmu tracing quirks

[Avi Kivity]

On 04/13/2011 05:09 PM, Jan Kiszka wrote:
> Hi Xiao,
>
> while tracing a guest with all events enabled, I notices some issues
> with kvm mmu instrumentations. A critical one:
>
> TRACE_EVENT(
> 	kvm_mmu_audit,
> 	TP_PROTO(struct kvm_vcpu *vcpu, int audit_point),
> 	TP_ARGS(vcpu, audit_point),
>
> 	TP_STRUCT__entry(
> 		__field(struct kvm_vcpu *, vcpu)
> 		__field(int, audit_point)
> 	),
>
> 	TP_fast_assign(
> 		__entry->vcpu = vcpu;
> 		__entry->audit_point = audit_point;
> 	),
>
> 	TP_printk("vcpu:%d %s", __entry->vcpu->cpu,
> 		  audit_point_name[__entry->audit_point])
> );
>
> Saving the vcpu reference to the trace buffer can break on dump if the
> vcpu was destroyed in the meantime. I was about to fix that by saving
> vcpu->cpu instead, but then I wondered what kind of information you
> actually need here. The triggering host cpu is automatically recorded by
> ftrace anyway. Can you comment on this / clean it up? Also, it would be
> nice to use __print_symbolic for translating audit_point to a string as
> that would also work for trace-cmd/kernelshark.
>

IIRC the audit trace is only used to get a low cost invocation for the 
audit machinery; it's not actually useful as a user tracepoint.

We should probably switch to static_branch() instead 
(https://lwn.net/Articles/429447/).

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

Re: kvm mmu tracing quirks

[Xiao Guangrong]

On 04/14/2011 12:12 AM, Avi Kivity wrote:
> On 04/13/2011 05:09 PM, Jan Kiszka wrote:
>> Hi Xiao,
>>
>> while tracing a guest with all events enabled, I notices some issues
>> with kvm mmu instrumentations. A critical one:
>>
>> TRACE_EVENT(
>>     kvm_mmu_audit,
>>     TP_PROTO(struct kvm_vcpu *vcpu, int audit_point),
>>     TP_ARGS(vcpu, audit_point),
>>
>>     TP_STRUCT__entry(
>>         __field(struct kvm_vcpu *, vcpu)
>>         __field(int, audit_point)
>>     ),
>>
>>     TP_fast_assign(
>>         __entry->vcpu = vcpu;
>>         __entry->audit_point = audit_point;
>>     ),
>>
>>     TP_printk("vcpu:%d %s", __entry->vcpu->cpu,
>>           audit_point_name[__entry->audit_point])
>> );
>>
>> Saving the vcpu reference to the trace buffer can break on dump if the
>> vcpu was destroyed in the meantime. I was about to fix that by saving
>> vcpu->cpu instead, but then I wondered what kind of information you
>> actually need here. The triggering host cpu is automatically recorded by
>> ftrace anyway. Can you comment on this / clean it up? Also, it would be
>> nice to use __print_symbolic for translating audit_point to a string as
>> that would also work for trace-cmd/kernelshark.
>>

Thanks you for pointing it out, Jan! The fix is good for me!

> 
> IIRC the audit trace is only used to get a low cost invocation for the audit machinery; it's not actually useful as a user tracepoint.
> 

Yes, it is.

> We should probably switch to static_branch() instead (https://lwn.net/Articles/429447/).
> 

It's a good idea, i'll do that after static_branch merged!