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

[Peter Xu <peterx@redhat.com>]

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?

Regards,

-- 
Peter Xu