Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Tue, Jul 30, 2019 at 10:25:25AM +0800, Peter Xu wrote:
> On Tue, Jul 30, 2019 at 10:12:45AM +0800, Peter Xu wrote:
> > On Mon, Jul 29, 2019 at 07:06:07PM -0700, Sean Christopherson wrote:
> > > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > > > > index d98eac371c0a..cc1f98130e6a 100644
> > > > > > --- a/arch/x86/kvm/vmx/vmx.c
> > > > > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > > > > @@ -5214,7 +5214,7 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
> > > > > >  	if (vmx->ple_window != old)
> > > > > >  		vmx->ple_window_dirty = true;
> > > > > >  
> > > > > > -	trace_kvm_ple_window_grow(vcpu->vcpu_id, vmx->ple_window, old);
> > > > > > +	trace_kvm_ple_window_changed(vcpu->vcpu_id, vmx->ple_window, old);
> > > > > 
> > > > > No need for the macro, the snippet right about already checks 'new != old'.
> > > > > Though I do like the rename, i.e. rename the trace function to
> > > > > trace_kvm_ple_window_changed().
> > > > 
> > > > Do you mean this one?
> > > > 
> > > > 	if (vmx->ple_window != old)
> > > > 		vmx->ple_window_dirty = true;
> > > 
> > > Yep.
> > > 
> > > > It didn't return, did it? :)
> > > 
> > > You lost me.  What's wrong with:
> > > 
> > > 	if (vmx->ple_window != old) {
> > > 		vmx->ple_window_dirty = true;
> > > 		trace_kvm_ple_window_update(vcpu->vcpu_id, vmx_ple->window, old);
> > > 	}
> > 
> > Yes this looks fine to me.  I'll switch.
> 
> Btw, I noticed we have this:
> 
>   EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_ple_window);
> 
> Is that trying to expose the tracepoints to the outter world?  Is that
> whole chunk of EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_*) really needed?

It's needed to invoke tracepoints from VMX/SVM as the implementations live
in kvm.ko.  Same reason functions in x86.c and company need to be exported
if they're called by VMX/SVM code.