| Message ID | 20190729053243.9224-4-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: X86: Some tracepoint enhancements | expand |
On Mon, Jul 29, 2019 at 01:32:43PM +0800, Peter Xu wrote: > The PLE window tracepoint triggers easily and it can be a bit > confusing too. One example line: > > kvm_ple_window: vcpu 0: ple_window 4096 (shrink 4096) > > It easily let people think of "the window now is 4096 which is > shrinked", but the truth is the value actually didn't change (4096). > > Let's only dump this message if the value really changed, and we make > the message even simpler like: > > kvm_ple_window: vcpu 4 (4096 -> 8192) This seems a bit too terse, e.g. requires a decent amount of effort to do relatively simple things like show only cases where the windows was shrunk, or grew/shrunk by a large amount. In this case, more is likely better, e.g.: kvm_ple_window_changed: vcpu 4 ple_window 8192 old 4096 grow 4096 and kvm_ple_window_changed: vcpu 4 ple_window 4096 old 8192 shrink 4096 Tangentially related, it'd be nice to settle on a standard format for printing field+val. Right now there are four different styles, e.g. "field=val", "field = val", "field: val" and "field val". > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > arch/x86/kvm/svm.c | 8 ++++---- > arch/x86/kvm/trace.h | 22 +++++++++------------- > arch/x86/kvm/vmx/vmx.c | 4 ++-- > 3 files changed, 15 insertions(+), 19 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 48c865a4e5dd..0d365b621b5a 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1268,8 +1268,8 @@ static void grow_ple_window(struct kvm_vcpu *vcpu) > if (control->pause_filter_count != old) > mark_dirty(svm->vmcb, VMCB_INTERCEPTS); > > - trace_kvm_ple_window_grow(vcpu->vcpu_id, > - control->pause_filter_count, old); > + trace_kvm_ple_window_changed(vcpu->vcpu_id, > + control->pause_filter_count, old); > } > > static void shrink_ple_window(struct kvm_vcpu *vcpu) > @@ -1286,8 +1286,8 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) > if (control->pause_filter_count != old) > mark_dirty(svm->vmcb, VMCB_INTERCEPTS); > > - trace_kvm_ple_window_shrink(vcpu->vcpu_id, > - control->pause_filter_count, old); > + trace_kvm_ple_window_changed(vcpu->vcpu_id, > + control->pause_filter_count, old); > } > > static __init int svm_hardware_setup(void) > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > index 76a39bc25b95..91c91f358b23 100644 > --- a/arch/x86/kvm/trace.h > +++ b/arch/x86/kvm/trace.h > @@ -891,34 +891,30 @@ TRACE_EVENT(kvm_pml_full, > ); > > TRACE_EVENT(kvm_ple_window, > - TP_PROTO(bool grow, unsigned int vcpu_id, int new, int old), > - TP_ARGS(grow, vcpu_id, new, old), > + TP_PROTO(unsigned int vcpu_id, int new, int old), > + TP_ARGS(vcpu_id, new, old), > > TP_STRUCT__entry( > - __field( bool, grow ) Side note, if the tracepoint is invoked only on changes the "grow" field can be removed even if the tracepoint prints grow vs. shrink, i.e. there's no ambiguity since new==old will never happen. > __field( unsigned int, vcpu_id ) > __field( int, new ) > __field( int, old ) > ), > > TP_fast_assign( > - __entry->grow = grow; > __entry->vcpu_id = vcpu_id; > __entry->new = new; > __entry->old = old; > ), > > - TP_printk("vcpu %u: ple_window %d (%s %d)", > - __entry->vcpu_id, > - __entry->new, > - __entry->grow ? "grow" : "shrink", > - __entry->old) > + TP_printk("vcpu %u (%d -> %d)", > + __entry->vcpu_id, __entry->old, __entry->new) > ); > > -#define trace_kvm_ple_window_grow(vcpu_id, new, old) \ > - trace_kvm_ple_window(true, vcpu_id, new, old) > -#define trace_kvm_ple_window_shrink(vcpu_id, new, old) \ > - trace_kvm_ple_window(false, vcpu_id, new, old) > +#define trace_kvm_ple_window_changed(vcpu, new, old) \ > + do { \ > + if (old != new) \ > + trace_kvm_ple_window(vcpu, new, old); \ > + } while (0) > > TRACE_EVENT(kvm_pvclock_update, > TP_PROTO(unsigned int vcpu_id, struct pvclock_vcpu_time_info *pvclock), > 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(). > } > > static void shrink_ple_window(struct kvm_vcpu *vcpu) > @@ -5229,7 +5229,7 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) > if (vmx->ple_window != old) > vmx->ple_window_dirty = true; > > - trace_kvm_ple_window_shrink(vcpu->vcpu_id, vmx->ple_window, old); > + trace_kvm_ple_window_changed(vcpu->vcpu_id, vmx->ple_window, old); > } > > /* > -- > 2.21.0 >
On Mon, Jul 29, 2019 at 09:23:38AM -0700, Sean Christopherson wrote: > On Mon, Jul 29, 2019 at 01:32:43PM +0800, Peter Xu wrote: > > The PLE window tracepoint triggers easily and it can be a bit > > confusing too. One example line: > > > > kvm_ple_window: vcpu 0: ple_window 4096 (shrink 4096) > > > > It easily let people think of "the window now is 4096 which is > > shrinked", but the truth is the value actually didn't change (4096). > > > > Let's only dump this message if the value really changed, and we make > > the message even simpler like: > > > > kvm_ple_window: vcpu 4 (4096 -> 8192) > > This seems a bit too terse, e.g. requires a decent amount of effort to > do relatively simple things like show only cases where the windows was > shrunk, or grew/shrunk by a large amount. In this case, more is likely > better, e.g.: > > kvm_ple_window_changed: vcpu 4 ple_window 8192 old 4096 grow 4096 > > and > > kvm_ple_window_changed: vcpu 4 ple_window 4096 old 8192 shrink 4096 How about: kvm_ple_window: vcpu 4 (4096 -> 8192, growed) Or: kvm_ple_window: vcpu 4 old 4096 new 8192 growed I would prefer the arrow which is very clear to me to show a value change, but I'd be fine to see what's your final preference or any further reviewers. Anyway I think any of them is clearer than the original version... > > > Tangentially related, it'd be nice to settle on a standard format for > printing field+val. Right now there are four different styles, e.g. > "field=val", "field = val", "field: val" and "field val". Right, I ses "field val" is used most frequently. But I didn't touch those up because they haven't yet caused any confusion to me. [...] > > TP_STRUCT__entry( > > - __field( bool, grow ) > > Side note, if the tracepoint is invoked only on changes the "grow" field > can be removed even if the tracepoint prints grow vs. shrink, i.e. there's > no ambiguity since new==old will never happen. But I do see it happen... Please see below. > > > __field( unsigned int, vcpu_id ) > > __field( int, new ) > > __field( int, old ) > > ), > > > > TP_fast_assign( > > - __entry->grow = grow; > > __entry->vcpu_id = vcpu_id; > > __entry->new = new; > > __entry->old = old; > > ), > > > > - TP_printk("vcpu %u: ple_window %d (%s %d)", > > - __entry->vcpu_id, > > - __entry->new, > > - __entry->grow ? "grow" : "shrink", > > - __entry->old) > > + TP_printk("vcpu %u (%d -> %d)", > > + __entry->vcpu_id, __entry->old, __entry->new) > > ); > > > > -#define trace_kvm_ple_window_grow(vcpu_id, new, old) \ > > - trace_kvm_ple_window(true, vcpu_id, new, old) > > -#define trace_kvm_ple_window_shrink(vcpu_id, new, old) \ > > - trace_kvm_ple_window(false, vcpu_id, new, old) > > +#define trace_kvm_ple_window_changed(vcpu, new, old) \ > > + do { \ > > + if (old != new) \ > > + trace_kvm_ple_window(vcpu, new, old); \ > > + } while (0) > > > > TRACE_EVENT(kvm_pvclock_update, > > TP_PROTO(unsigned int vcpu_id, struct pvclock_vcpu_time_info *pvclock), > > 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; It didn't return, did it? :) Thanks,
On Tue, Jul 30, 2019 at 09:43:39AM +0800, Peter Xu wrote: > On Mon, Jul 29, 2019 at 09:23:38AM -0700, Sean Christopherson wrote: > > On Mon, Jul 29, 2019 at 01:32:43PM +0800, Peter Xu wrote: > > > The PLE window tracepoint triggers easily and it can be a bit > > > confusing too. One example line: > > > > > > kvm_ple_window: vcpu 0: ple_window 4096 (shrink 4096) > > > > > > It easily let people think of "the window now is 4096 which is > > > shrinked", but the truth is the value actually didn't change (4096). > > > > > > Let's only dump this message if the value really changed, and we make > > > the message even simpler like: > > > > > > kvm_ple_window: vcpu 4 (4096 -> 8192) > > > > This seems a bit too terse, e.g. requires a decent amount of effort to > > do relatively simple things like show only cases where the windows was > > shrunk, or grew/shrunk by a large amount. In this case, more is likely > > better, e.g.: > > > > kvm_ple_window_changed: vcpu 4 ple_window 8192 old 4096 grow 4096 > > > > and > > > > kvm_ple_window_changed: vcpu 4 ple_window 4096 old 8192 shrink 4096 > > How about: > > kvm_ple_window: vcpu 4 (4096 -> 8192, growed) > > Or: > > kvm_ple_window: vcpu 4 old 4096 new 8192 growed > > I would prefer the arrow which is very clear to me to show a value > change, but I'd be fine to see what's your final preference or any > further reviewers. Anyway I think any of them is clearer than the > original version... For tracepoints, I prefer to err on the side of more info as it's easy to filter out unwanted date. But odds are I'll never use this particular tracepoint, so I'll defer to folks who are actually affected. > > > > > > Tangentially related, it'd be nice to settle on a standard format for > > printing field+val. Right now there are four different styles, e.g. > > "field=val", "field = val", "field: val" and "field val". > > Right, I ses "field val" is used most frequently. But I didn't touch > those up because they haven't yet caused any confusion to me. Ya, it was more of a general complaint :-) > [...] > > > > TP_STRUCT__entry( > > > - __field( bool, grow ) > > > > Side note, if the tracepoint is invoked only on changes the "grow" field > > can be removed even if the tracepoint prints grow vs. shrink, i.e. there's > > no ambiguity since new==old will never happen. > > But I do see it happen... Please see below. ... > > > 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); }
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. Regards,
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,
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.
On Mon, Jul 29, 2019 at 07:28:44PM -0700, Sean Christopherson wrote: > 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. Ah right. Then I assume it's pretty safe to change the symbol name here. Thanks!
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 48c865a4e5dd..0d365b621b5a 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1268,8 +1268,8 @@ static void grow_ple_window(struct kvm_vcpu *vcpu) if (control->pause_filter_count != old) mark_dirty(svm->vmcb, VMCB_INTERCEPTS); - trace_kvm_ple_window_grow(vcpu->vcpu_id, - control->pause_filter_count, old); + trace_kvm_ple_window_changed(vcpu->vcpu_id, + control->pause_filter_count, old); } static void shrink_ple_window(struct kvm_vcpu *vcpu) @@ -1286,8 +1286,8 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) if (control->pause_filter_count != old) mark_dirty(svm->vmcb, VMCB_INTERCEPTS); - trace_kvm_ple_window_shrink(vcpu->vcpu_id, - control->pause_filter_count, old); + trace_kvm_ple_window_changed(vcpu->vcpu_id, + control->pause_filter_count, old); } static __init int svm_hardware_setup(void) diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h index 76a39bc25b95..91c91f358b23 100644 --- a/arch/x86/kvm/trace.h +++ b/arch/x86/kvm/trace.h @@ -891,34 +891,30 @@ TRACE_EVENT(kvm_pml_full, ); TRACE_EVENT(kvm_ple_window, - TP_PROTO(bool grow, unsigned int vcpu_id, int new, int old), - TP_ARGS(grow, vcpu_id, new, old), + TP_PROTO(unsigned int vcpu_id, int new, int old), + TP_ARGS(vcpu_id, new, old), TP_STRUCT__entry( - __field( bool, grow ) __field( unsigned int, vcpu_id ) __field( int, new ) __field( int, old ) ), TP_fast_assign( - __entry->grow = grow; __entry->vcpu_id = vcpu_id; __entry->new = new; __entry->old = old; ), - TP_printk("vcpu %u: ple_window %d (%s %d)", - __entry->vcpu_id, - __entry->new, - __entry->grow ? "grow" : "shrink", - __entry->old) + TP_printk("vcpu %u (%d -> %d)", + __entry->vcpu_id, __entry->old, __entry->new) ); -#define trace_kvm_ple_window_grow(vcpu_id, new, old) \ - trace_kvm_ple_window(true, vcpu_id, new, old) -#define trace_kvm_ple_window_shrink(vcpu_id, new, old) \ - trace_kvm_ple_window(false, vcpu_id, new, old) +#define trace_kvm_ple_window_changed(vcpu, new, old) \ + do { \ + if (old != new) \ + trace_kvm_ple_window(vcpu, new, old); \ + } while (0) TRACE_EVENT(kvm_pvclock_update, TP_PROTO(unsigned int vcpu_id, struct pvclock_vcpu_time_info *pvclock), 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); } static void shrink_ple_window(struct kvm_vcpu *vcpu) @@ -5229,7 +5229,7 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) if (vmx->ple_window != old) vmx->ple_window_dirty = true; - trace_kvm_ple_window_shrink(vcpu->vcpu_id, vmx->ple_window, old); + trace_kvm_ple_window_changed(vcpu->vcpu_id, vmx->ple_window, old); } /*
The PLE window tracepoint triggers easily and it can be a bit confusing too. One example line: kvm_ple_window: vcpu 0: ple_window 4096 (shrink 4096) It easily let people think of "the window now is 4096 which is shrinked", but the truth is the value actually didn't change (4096). Let's only dump this message if the value really changed, and we make the message even simpler like: kvm_ple_window: vcpu 4 (4096 -> 8192) Signed-off-by: Peter Xu <peterx@redhat.com> --- arch/x86/kvm/svm.c | 8 ++++---- arch/x86/kvm/trace.h | 22 +++++++++------------- arch/x86/kvm/vmx/vmx.c | 4 ++-- 3 files changed, 15 insertions(+), 19 deletions(-)