Skip to contentSkip to conversation

kvm mmu tracing quirks

DISCUSSION Jan Kiszka 3 participants 3 messages Apr 13, 2011 – Apr 14, 2011 KVM

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

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

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#1 lore raw
Quoted 24 lines · 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])

);

Quoted 8 lines

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/).

Signature · 1 line
error compiling committee.c: too many arguments to function
#2 lore raw
Quoted 25 lines · Avi Kivity
Quoted 24 lines · 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])

);

Quoted 8 lines
Quoted 8 lines

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!

Quoted 3 lines

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.

Quoted 2 lines

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!