[PATCHv2] trace "exit to userspace" event

[PATCHv2] trace "exit to userspace" event

[Gleb Natapov]

Add tracepoint for userspace exit.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
  ChangeLog:
  v1->v2
   log error case too.

diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
index 9c2cc6a..c86f4e8 100644
--- a/include/trace/events/kvm.h
+++ b/include/trace/events/kvm.h
@@ -6,6 +6,36 @@
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM kvm
 
+#define ERSN(x) { KVM_EXIT_##x, "KVM_EXIT_" #x }
+
+#define kvm_trace_exit_reason						\
+	ERSN(UNKNOWN), ERSN(EXCEPTION), ERSN(IO), ERSN(HYPERCALL),	\
+	ERSN(DEBUG), ERSN(HLT), ERSN(MMIO), ERSN(IRQ_WINDOW_OPEN),	\
+	ERSN(SHUTDOWN), ERSN(FAIL_ENTRY), ERSN(INTR), ERSN(SET_TPR),	\
+	ERSN(TPR_ACCESS), ERSN(S390_SIEIC), ERSN(S390_RESET), ERSN(DCR),\
+	ERSN(NMI), ERSN(INTERNAL_ERROR), ERSN(OSI)
+
+TRACE_EVENT(kvm_userspace_exit,
+	    TP_PROTO(__u32 reason, int errno),
+	    TP_ARGS(reason, errno),
+
+	TP_STRUCT__entry(
+		__field(	__u32,		reason		)
+		__field(	int,		errno		)
+	),
+
+	TP_fast_assign(
+		__entry->reason		= reason;
+		__entry->errno		= errno;
+	),
+
+	TP_printk("reason %s (%d)",
+		  __entry->errno < 0 ?
+		  (__entry->errno == -EINTR ? "restart" : "error") :
+		  __print_symbolic(__entry->reason, kvm_trace_exit_reason),
+		  __entry->errno < 0 ? -__entry->errno : __entry->reason)
+);
+
 #if defined(__KVM_HAVE_IOAPIC)
 TRACE_EVENT(kvm_set_irq,
 	TP_PROTO(unsigned int gsi, int level, int irq_source_id),
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 361ad1a..88c42ae 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1537,6 +1537,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
 		if (arg)
 			goto out;
 		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
+		trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
 		break;
 	case KVM_GET_REGS: {
 		struct kvm_regs *kvm_regs;
--
			Gleb.

Re: [PATCHv2] trace "exit to userspace" event

[Avi Kivity]

  On 10/24/2010 04:49 PM, Gleb Natapov wrote:
> Add tracepoint for userspace exit.
>
>
> +
> +TRACE_EVENT(kvm_userspace_exit,
> +	    TP_PROTO(__u32 reason, int errno),
> +	    TP_ARGS(reason, errno),
> +
> +	TP_STRUCT__entry(
> +		__field(	__u32,		reason		)
> +		__field(	int,		errno		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->reason		= reason;
> +		__entry->errno		= errno;

Shouldn't this be -errno?

> +	),
> +
> +	TP_printk("reason %s (%d)",
> +		  __entry->errno<  0 ?
> +		  (__entry->errno == -EINTR ? "restart" : "error") :
> +		  __print_symbolic(__entry->reason, kvm_trace_exit_reason),
> +		  __entry->errno<  0 ? -__entry->errno : __entry->reason)
> +);
> +
>   #if defined(__KVM_HAVE_IOAPIC)
>   TRACE_EVENT(kvm_set_irq,
>   	TP_PROTO(unsigned int gsi, int level, int irq_source_id),
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 361ad1a..88c42ae 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1537,6 +1537,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
>   		if (arg)
>   			goto out;
>   		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> +		trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
>   		break;

-- 
error compiling committee.c: too many arguments to function

Re: [PATCHv2] trace "exit to userspace" event

[Gleb Natapov]

On Sun, Oct 24, 2010 at 07:01:30PM +0200, Avi Kivity wrote:
>  On 10/24/2010 04:49 PM, Gleb Natapov wrote:
> >Add tracepoint for userspace exit.
> >
> >
> >+
> >+TRACE_EVENT(kvm_userspace_exit,
> >+	    TP_PROTO(__u32 reason, int errno),
> >+	    TP_ARGS(reason, errno),
> >+
> >+	TP_STRUCT__entry(
> >+		__field(	__u32,		reason		)
> >+		__field(	int,		errno		)
> >+	),
> >+
> >+	TP_fast_assign(
> >+		__entry->reason		= reason;
> >+		__entry->errno		= errno;
> 
> Shouldn't this be -errno?
> 
I check for __entry->errno < 0 below to see if exit was due to an error.

> >+	),
> >+
> >+	TP_printk("reason %s (%d)",
> >+		  __entry->errno<  0 ?
> >+		  (__entry->errno == -EINTR ? "restart" : "error") :
> >+		  __print_symbolic(__entry->reason, kvm_trace_exit_reason),
> >+		  __entry->errno<  0 ? -__entry->errno : __entry->reason)
> >+);
> >+
> >  #if defined(__KVM_HAVE_IOAPIC)
> >  TRACE_EVENT(kvm_set_irq,
> >  	TP_PROTO(unsigned int gsi, int level, int irq_source_id),
> >diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> >index 361ad1a..88c42ae 100644
> >--- a/virt/kvm/kvm_main.c
> >+++ b/virt/kvm/kvm_main.c
> >@@ -1537,6 +1537,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
> >  		if (arg)
> >  			goto out;
> >  		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> >+		trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
> >  		break;
> 
> -- 
> error compiling committee.c: too many arguments to function

--
			Gleb.

Re: [PATCHv2] trace "exit to userspace" event

[Avi Kivity]

  On 10/24/2010 07:24 PM, Gleb Natapov wrote:
> On Sun, Oct 24, 2010 at 07:01:30PM +0200, Avi Kivity wrote:
> >   On 10/24/2010 04:49 PM, Gleb Natapov wrote:
> >  >Add tracepoint for userspace exit.
> >  >
> >  >
> >  >+
> >  >+TRACE_EVENT(kvm_userspace_exit,
> >  >+	    TP_PROTO(__u32 reason, int errno),
> >  >+	    TP_ARGS(reason, errno),
> >  >+
> >  >+	TP_STRUCT__entry(
> >  >+		__field(	__u32,		reason		)
> >  >+		__field(	int,		errno		)
> >  >+	),
> >  >+
> >  >+	TP_fast_assign(
> >  >+		__entry->reason		= reason;
> >  >+		__entry->errno		= errno;
> >
> >  Shouldn't this be -errno?
> >
> I check for __entry->errno<  0 below to see if exit was due to an error.
>

Then check for > 0.  If a variable is named errno, it should work as an 
input to strerror() (or a __print_symbolic for errno).

-- 
error compiling committee.c: too many arguments to function

Re: [PATCHv2] trace "exit to userspace" event

[Gleb Natapov]

On Sun, Oct 24, 2010 at 07:27:21PM +0200, Avi Kivity wrote:
>  On 10/24/2010 07:24 PM, Gleb Natapov wrote:
> >On Sun, Oct 24, 2010 at 07:01:30PM +0200, Avi Kivity wrote:
> >>   On 10/24/2010 04:49 PM, Gleb Natapov wrote:
> >>  >Add tracepoint for userspace exit.
> >>  >
> >>  >
> >>  >+
> >>  >+TRACE_EVENT(kvm_userspace_exit,
> >>  >+	    TP_PROTO(__u32 reason, int errno),
> >>  >+	    TP_ARGS(reason, errno),
> >>  >+
> >>  >+	TP_STRUCT__entry(
> >>  >+		__field(	__u32,		reason		)
> >>  >+		__field(	int,		errno		)
> >>  >+	),
> >>  >+
> >>  >+	TP_fast_assign(
> >>  >+		__entry->reason		= reason;
> >>  >+		__entry->errno		= errno;
> >>
> >>  Shouldn't this be -errno?
> >>
> >I check for __entry->errno<  0 below to see if exit was due to an error.
> >
> 
> Then check for > 0.  If a variable is named errno, it should work as
Is it documented that kvm_arch_vcpu_ioctl_run() never returns positive
value? 

> an input to strerror() (or a __print_symbolic for errno).
> 
__print_symbolic for all errno will be rather long. Anyway I think
ftrace is not proper place to check for syscall errors. The only error
value that is interesting here is EINTR, all others will cause userspace
to halt. The only things that errno accomplish here is that TP_printk
string become less readable.

--
			Gleb.

Re: [PATCHv2] trace "exit to userspace" event

[Marcelo Tosatti]

On Sun, Oct 24, 2010 at 04:49:08PM +0200, Gleb Natapov wrote:
> Add tracepoint for userspace exit.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>   ChangeLog:
>   v1->v2
>    log error case too.

Applied, thanks.

Re: [PATCHv2] trace "exit to userspace" event

[Avi Kivity]

  On 11/01/2010 12:06 PM, Marcelo Tosatti wrote:
> On Sun, Oct 24, 2010 at 04:49:08PM +0200, Gleb Natapov wrote:
> >  Add tracepoint for userspace exit.
> >
> >  Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >  ---
> >    ChangeLog:
> >    v1->v2
> >     log error case too.
>
> Applied, thanks.
>

errno sign is still flipped, no?


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCHv2] trace "exit to userspace" event

[Marcelo Tosatti]

On Mon, Nov 01, 2010 at 03:33:20PM -0400, Avi Kivity wrote:
>  On 11/01/2010 12:06 PM, Marcelo Tosatti wrote:
> >On Sun, Oct 24, 2010 at 04:49:08PM +0200, Gleb Natapov wrote:
> >>  Add tracepoint for userspace exit.
> >>
> >>  Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >>  ---
> >>    ChangeLog:
> >>    v1->v2
> >>     log error case too.
> >
> >Applied, thanks.
> >
> 
> errno sign is still flipped, no?

No, kvm_arch_vcpu_ioctl_run return -errno on error.

Re: [PATCHv2] trace "exit to userspace" event

[Avi Kivity]

  On 11/01/2010 03:43 PM, Marcelo Tosatti wrote:
> On Mon, Nov 01, 2010 at 03:33:20PM -0400, Avi Kivity wrote:
> >   On 11/01/2010 12:06 PM, Marcelo Tosatti wrote:
> >  >On Sun, Oct 24, 2010 at 04:49:08PM +0200, Gleb Natapov wrote:
> >  >>   Add tracepoint for userspace exit.
> >  >>
> >  >>   Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >  >>   ---
> >  >>     ChangeLog:
> >  >>     v1->v2
> >  >>      log error case too.
> >  >
> >  >Applied, thanks.
> >  >
> >
> >  errno sign is still flipped, no?
>
> No, kvm_arch_vcpu_ioctl_run return -errno on error.
>

It's true that this is how the kernel ABI is defined, but for a user 
errnos are positive.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.