kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] KVM: X86: Some tracepoint enhancements
@ 2019-07-29  5:32 Peter Xu
  2019-07-29  5:32 ` [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit Peter Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Peter Xu @ 2019-07-29  5:32 UTC (permalink / raw)
  To: kvm; +Cc: Paolo Bonzini, peterx

Each small patch explains itself.  I noticed them when I'm tracing
some IRQ paths and I found them helpful at least to me.

Please have a look.  Thanks,

Peter Xu (3):
  KVM: X86: Trace vcpu_id for vmexit
  KVM: X86: Remove tailing newline for tracepoints
  KVM: X86: Tune PLE Window tracepoint

 arch/x86/kvm/svm.c     |  8 ++++----
 arch/x86/kvm/trace.h   | 33 ++++++++++++++++-----------------
 arch/x86/kvm/vmx/vmx.c |  4 ++--
 3 files changed, 22 insertions(+), 23 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit
  2019-07-29  5:32 [PATCH 0/3] KVM: X86: Some tracepoint enhancements Peter Xu
@ 2019-07-29  5:32 ` Peter Xu
  2019-07-29 16:28   ` Sean Christopherson
  2019-07-31 21:49   ` Krish Sadhukhan
  2019-07-29  5:32 ` [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints Peter Xu
  2019-07-29  5:32 ` [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint Peter Xu
  2 siblings, 2 replies; 17+ messages in thread
From: Peter Xu @ 2019-07-29  5:32 UTC (permalink / raw)
  To: kvm; +Cc: Paolo Bonzini, peterx

It helps to pair vmenters and vmexis with multi-core systems.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/x86/kvm/trace.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 4d47a2631d1f..26423d2e45df 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -232,17 +232,20 @@ TRACE_EVENT(kvm_exit,
 		__field(	u32,	        isa             )
 		__field(	u64,	        info1           )
 		__field(	u64,	        info2           )
+		__field(	int,	        vcpu_id         )
 	),
 
 	TP_fast_assign(
 		__entry->exit_reason	= exit_reason;
 		__entry->guest_rip	= kvm_rip_read(vcpu);
 		__entry->isa            = isa;
+		__entry->vcpu_id        = vcpu->vcpu_id;
 		kvm_x86_ops->get_exit_info(vcpu, &__entry->info1,
 					   &__entry->info2);
 	),
 
-	TP_printk("reason %s rip 0x%lx info %llx %llx",
+	TP_printk("vcpu %d reason %s rip 0x%lx info %llx %llx",
+		  __entry->vcpu_id,
 		 (__entry->isa == KVM_ISA_VMX) ?
 		 __print_symbolic(__entry->exit_reason, VMX_EXIT_REASONS) :
 		 __print_symbolic(__entry->exit_reason, SVM_EXIT_REASONS),
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints
  2019-07-29  5:32 [PATCH 0/3] KVM: X86: Some tracepoint enhancements Peter Xu
  2019-07-29  5:32 ` [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit Peter Xu
@ 2019-07-29  5:32 ` Peter Xu
  2019-08-01  0:19   ` Krish Sadhukhan
  2019-08-01  3:39   ` Wanpeng Li
  2019-07-29  5:32 ` [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint Peter Xu
  2 siblings, 2 replies; 17+ messages in thread
From: Peter Xu @ 2019-07-29  5:32 UTC (permalink / raw)
  To: kvm; +Cc: Paolo Bonzini, peterx

It's done by TP_printk() already.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/x86/kvm/trace.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 26423d2e45df..76a39bc25b95 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1323,7 +1323,7 @@ TRACE_EVENT(kvm_avic_incomplete_ipi,
 		__entry->index = index;
 	),
 
-	TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
+	TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u",
 		  __entry->vcpu, __entry->icrh, __entry->icrl,
 		  __entry->id, __entry->index)
 );
@@ -1348,7 +1348,7 @@ TRACE_EVENT(kvm_avic_unaccelerated_access,
 		__entry->vec = vec;
 	),
 
-	TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x\n",
+	TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x",
 		  __entry->vcpu,
 		  __entry->offset,
 		  __print_symbolic(__entry->offset, kvm_trace_symbol_apic),
@@ -1368,7 +1368,7 @@ TRACE_EVENT(kvm_hv_timer_state,
 			__entry->vcpu_id = vcpu_id;
 			__entry->hv_timer_in_use = hv_timer_in_use;
 			),
-		TP_printk("vcpu_id %x hv_timer %x\n",
+		TP_printk("vcpu_id %x hv_timer %x",
 			__entry->vcpu_id,
 			__entry->hv_timer_in_use)
 );
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-29  5:32 [PATCH 0/3] KVM: X86: Some tracepoint enhancements Peter Xu
  2019-07-29  5:32 ` [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit Peter Xu
  2019-07-29  5:32 ` [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints Peter Xu
@ 2019-07-29  5:32 ` Peter Xu
  2019-07-29 16:23   ` Sean Christopherson
  2 siblings, 1 reply; 17+ messages in thread
From: Peter Xu @ 2019-07-29  5:32 UTC (permalink / raw)
  To: kvm; +Cc: Paolo Bonzini, peterx

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(-)

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);
 }
 
 /*
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-29  5:32 ` [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint Peter Xu
@ 2019-07-29 16:23   ` Sean Christopherson
  2019-07-30  1:43     ` Peter Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2019-07-29 16:23 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, Paolo Bonzini, peterx

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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit
  2019-07-29  5:32 ` [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit Peter Xu
@ 2019-07-29 16:28   ` Sean Christopherson
  2019-07-30  1:49     ` Peter Xu
  2019-07-31 21:49   ` Krish Sadhukhan
  1 sibling, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2019-07-29 16:28 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, Paolo Bonzini, peterx

On Mon, Jul 29, 2019 at 01:32:41PM +0800, Peter Xu wrote:
> It helps to pair vmenters and vmexis with multi-core systems.

Typo "vmexis".  The wording is also a bit funky.  How about:

Tracing the ID helps to pair vmenters and vmexits for guests with
multiple vCPUs.

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  arch/x86/kvm/trace.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 4d47a2631d1f..26423d2e45df 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -232,17 +232,20 @@ TRACE_EVENT(kvm_exit,
>  		__field(	u32,	        isa             )
>  		__field(	u64,	        info1           )
>  		__field(	u64,	        info2           )
> +		__field(	int,	        vcpu_id         )
>  	),
>  
>  	TP_fast_assign(
>  		__entry->exit_reason	= exit_reason;
>  		__entry->guest_rip	= kvm_rip_read(vcpu);
>  		__entry->isa            = isa;
> +		__entry->vcpu_id        = vcpu->vcpu_id;
>  		kvm_x86_ops->get_exit_info(vcpu, &__entry->info1,
>  					   &__entry->info2);
>  	),
>  
> -	TP_printk("reason %s rip 0x%lx info %llx %llx",
> +	TP_printk("vcpu %d reason %s rip 0x%lx info %llx %llx",
> +		  __entry->vcpu_id,
>  		 (__entry->isa == KVM_ISA_VMX) ?
>  		 __print_symbolic(__entry->exit_reason, VMX_EXIT_REASONS) :
>  		 __print_symbolic(__entry->exit_reason, SVM_EXIT_REASONS),
> -- 
> 2.21.0
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-29 16:23   ` Sean Christopherson
@ 2019-07-30  1:43     ` Peter Xu
  2019-07-30  2:06       ` Sean Christopherson
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Xu @ 2019-07-30  1:43 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Peter Xu, kvm, Paolo Bonzini

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,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit
  2019-07-29 16:28   ` Sean Christopherson
@ 2019-07-30  1:49     ` Peter Xu
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Xu @ 2019-07-30  1:49 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Peter Xu, kvm, Paolo Bonzini

On Mon, Jul 29, 2019 at 09:28:15AM -0700, Sean Christopherson wrote:
> On Mon, Jul 29, 2019 at 01:32:41PM +0800, Peter Xu wrote:
> > It helps to pair vmenters and vmexis with multi-core systems.
> 
> Typo "vmexis".  The wording is also a bit funky.  How about:
> 
> Tracing the ID helps to pair vmenters and vmexits for guests with
> multiple vCPUs.

Sure thing.  Thanks,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-30  1:43     ` Peter Xu
@ 2019-07-30  2:06       ` Sean Christopherson
  2019-07-30  2:12         ` Peter Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2019-07-30  2:06 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, Paolo Bonzini

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);
	}

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-30  2:06       ` Sean Christopherson
@ 2019-07-30  2:12         ` Peter Xu
  2019-07-30  2:25           ` Peter Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Xu @ 2019-07-30  2:12 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Peter Xu, kvm, Paolo Bonzini

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,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-30  2:12         ` Peter Xu
@ 2019-07-30  2:25           ` Peter Xu
  2019-07-30  2:28             ` Sean Christopherson
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Xu @ 2019-07-30  2:25 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Peter Xu, kvm, Paolo Bonzini

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-30  2:25           ` Peter Xu
@ 2019-07-30  2:28             ` Sean Christopherson
  2019-07-30  2:39               ` Peter Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2019-07-30  2:28 UTC (permalink / raw)
  To: Peter Xu; +Cc: Peter Xu, kvm, Paolo Bonzini

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint
  2019-07-30  2:28             ` Sean Christopherson
@ 2019-07-30  2:39               ` Peter Xu
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Xu @ 2019-07-30  2:39 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Peter Xu, kvm, Paolo Bonzini

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!

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit
  2019-07-29  5:32 ` [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit Peter Xu
  2019-07-29 16:28   ` Sean Christopherson
@ 2019-07-31 21:49   ` Krish Sadhukhan
  1 sibling, 0 replies; 17+ messages in thread
From: Krish Sadhukhan @ 2019-07-31 21:49 UTC (permalink / raw)
  To: Peter Xu, kvm; +Cc: Paolo Bonzini, peterx



On 07/28/2019 10:32 PM, Peter Xu wrote:
> It helps to pair vmenters and vmexis with multi-core systems.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   arch/x86/kvm/trace.h | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 4d47a2631d1f..26423d2e45df 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -232,17 +232,20 @@ TRACE_EVENT(kvm_exit,
>   		__field(	u32,	        isa             )
>   		__field(	u64,	        info1           )
>   		__field(	u64,	        info2           )
> +		__field(	int,	        vcpu_id         )
>   	),
>   
>   	TP_fast_assign(
>   		__entry->exit_reason	= exit_reason;
>   		__entry->guest_rip	= kvm_rip_read(vcpu);
>   		__entry->isa            = isa;
> +		__entry->vcpu_id        = vcpu->vcpu_id;
>   		kvm_x86_ops->get_exit_info(vcpu, &__entry->info1,
>   					   &__entry->info2);
>   	),
>   
> -	TP_printk("reason %s rip 0x%lx info %llx %llx",
> +	TP_printk("vcpu %d reason %s rip 0x%lx info %llx %llx",
> +		  __entry->vcpu_id,
>   		 (__entry->isa == KVM_ISA_VMX) ?
>   		 __print_symbolic(__entry->exit_reason, VMX_EXIT_REASONS) :
>   		 __print_symbolic(__entry->exit_reason, SVM_EXIT_REASONS),

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints
  2019-07-29  5:32 ` [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints Peter Xu
@ 2019-08-01  0:19   ` Krish Sadhukhan
  2019-08-01  3:39   ` Wanpeng Li
  1 sibling, 0 replies; 17+ messages in thread
From: Krish Sadhukhan @ 2019-08-01  0:19 UTC (permalink / raw)
  To: Peter Xu, kvm; +Cc: Paolo Bonzini, peterx



On 07/28/2019 10:32 PM, Peter Xu wrote:
> It's done by TP_printk() already.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   arch/x86/kvm/trace.h | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 26423d2e45df..76a39bc25b95 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1323,7 +1323,7 @@ TRACE_EVENT(kvm_avic_incomplete_ipi,
>   		__entry->index = index;
>   	),
>   
> -	TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
> +	TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u",
>   		  __entry->vcpu, __entry->icrh, __entry->icrl,
>   		  __entry->id, __entry->index)
>   );
> @@ -1348,7 +1348,7 @@ TRACE_EVENT(kvm_avic_unaccelerated_access,
>   		__entry->vec = vec;
>   	),
>   
> -	TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x\n",
> +	TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x",
>   		  __entry->vcpu,
>   		  __entry->offset,
>   		  __print_symbolic(__entry->offset, kvm_trace_symbol_apic),
> @@ -1368,7 +1368,7 @@ TRACE_EVENT(kvm_hv_timer_state,
>   			__entry->vcpu_id = vcpu_id;
>   			__entry->hv_timer_in_use = hv_timer_in_use;
>   			),
> -		TP_printk("vcpu_id %x hv_timer %x\n",
> +		TP_printk("vcpu_id %x hv_timer %x",
>   			__entry->vcpu_id,
>   			__entry->hv_timer_in_use)
>   );

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints
  2019-07-29  5:32 ` [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints Peter Xu
  2019-08-01  0:19   ` Krish Sadhukhan
@ 2019-08-01  3:39   ` Wanpeng Li
  2019-08-13 16:43     ` Peter Xu
  1 sibling, 1 reply; 17+ messages in thread
From: Wanpeng Li @ 2019-08-01  3:39 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, Paolo Bonzini, Peter Xu

On Mon, 29 Jul 2019 at 13:35, Peter Xu <zhexu@redhat.com> wrote:
>
> It's done by TP_printk() already.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  arch/x86/kvm/trace.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 26423d2e45df..76a39bc25b95 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1323,7 +1323,7 @@ TRACE_EVENT(kvm_avic_incomplete_ipi,
>                 __entry->index = index;
>         ),
>
> -       TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
> +       TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u",
>                   __entry->vcpu, __entry->icrh, __entry->icrl,
>                   __entry->id, __entry->index)
>  );
> @@ -1348,7 +1348,7 @@ TRACE_EVENT(kvm_avic_unaccelerated_access,
>                 __entry->vec = vec;
>         ),
>
> -       TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x\n",
> +       TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x",
>                   __entry->vcpu,
>                   __entry->offset,
>                   __print_symbolic(__entry->offset, kvm_trace_symbol_apic),
> @@ -1368,7 +1368,7 @@ TRACE_EVENT(kvm_hv_timer_state,
>                         __entry->vcpu_id = vcpu_id;
>                         __entry->hv_timer_in_use = hv_timer_in_use;
>                         ),
> -               TP_printk("vcpu_id %x hv_timer %x\n",
> +               TP_printk("vcpu_id %x hv_timer %x",
>                         __entry->vcpu_id,
>                         __entry->hv_timer_in_use)

The last one is handled by commit 7be373b6de503 .

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints
  2019-08-01  3:39   ` Wanpeng Li
@ 2019-08-13 16:43     ` Peter Xu
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Xu @ 2019-08-13 16:43 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Peter Xu, kvm, Paolo Bonzini

On Thu, Aug 01, 2019 at 11:39:04AM +0800, Wanpeng Li wrote:
> On Mon, 29 Jul 2019 at 13:35, Peter Xu <zhexu@redhat.com> wrote:
> >
> > It's done by TP_printk() already.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  arch/x86/kvm/trace.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > index 26423d2e45df..76a39bc25b95 100644
> > --- a/arch/x86/kvm/trace.h
> > +++ b/arch/x86/kvm/trace.h
> > @@ -1323,7 +1323,7 @@ TRACE_EVENT(kvm_avic_incomplete_ipi,
> >                 __entry->index = index;
> >         ),
> >
> > -       TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
> > +       TP_printk("vcpu=%u, icrh:icrl=%#010x:%08x, id=%u, index=%u",
> >                   __entry->vcpu, __entry->icrh, __entry->icrl,
> >                   __entry->id, __entry->index)
> >  );
> > @@ -1348,7 +1348,7 @@ TRACE_EVENT(kvm_avic_unaccelerated_access,
> >                 __entry->vec = vec;
> >         ),
> >
> > -       TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x\n",
> > +       TP_printk("vcpu=%u, offset=%#x(%s), %s, %s, vec=%#x",
> >                   __entry->vcpu,
> >                   __entry->offset,
> >                   __print_symbolic(__entry->offset, kvm_trace_symbol_apic),
> > @@ -1368,7 +1368,7 @@ TRACE_EVENT(kvm_hv_timer_state,
> >                         __entry->vcpu_id = vcpu_id;
> >                         __entry->hv_timer_in_use = hv_timer_in_use;
> >                         ),
> > -               TP_printk("vcpu_id %x hv_timer %x\n",
> > +               TP_printk("vcpu_id %x hv_timer %x",
> >                         __entry->vcpu_id,
> >                         __entry->hv_timer_in_use)
> 
> The last one is handled by commit 7be373b6de503 .

Right, I'll rebase. Thanks.

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-08-13 16:43 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-29  5:32 [PATCH 0/3] KVM: X86: Some tracepoint enhancements Peter Xu
2019-07-29  5:32 ` [PATCH 1/3] KVM: X86: Trace vcpu_id for vmexit Peter Xu
2019-07-29 16:28   ` Sean Christopherson
2019-07-30  1:49     ` Peter Xu
2019-07-31 21:49   ` Krish Sadhukhan
2019-07-29  5:32 ` [PATCH 2/3] KVM: X86: Remove tailing newline for tracepoints Peter Xu
2019-08-01  0:19   ` Krish Sadhukhan
2019-08-01  3:39   ` Wanpeng Li
2019-08-13 16:43     ` Peter Xu
2019-07-29  5:32 ` [PATCH 3/3] KVM: X86: Tune PLE Window tracepoint Peter Xu
2019-07-29 16:23   ` Sean Christopherson
2019-07-30  1:43     ` Peter Xu
2019-07-30  2:06       ` Sean Christopherson
2019-07-30  2:12         ` Peter Xu
2019-07-30  2:25           ` Peter Xu
2019-07-30  2:28             ` Sean Christopherson
2019-07-30  2:39               ` Peter Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).