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
