From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: kvm mmu tracing quirks Date: Thu, 14 Apr 2011 11:47:38 +0800 Message-ID: <4DA66E5A.7070501@cn.fujitsu.com> References: <4DA5AEAB.6020203@siemens.com> <4DA5CB73.3050202@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Jan Kiszka , kvm To: Avi Kivity Return-path: Received: from cn.fujitsu.com ([222.73.24.84]:50371 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1752089Ab1DNDrM (ORCPT ); Wed, 13 Apr 2011 23:47:12 -0400 In-Reply-To: <4DA5CB73.3050202@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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!