[PATCH] trace "exit to userspace" event

[PATCH] trace "exit to userspace" event

[Gleb Natapov]

Add tracepoint for userspace exit.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
index 6dd3a51..fb44da0 100644
--- a/include/trace/events/kvm.h
+++ b/include/trace/events/kvm.h
@@ -6,6 +6,31 @@
 #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),
+	    TP_ARGS(reason),
+
+	TP_STRUCT__entry(
+		__field(	__u32,		reason		)
+	),
+
+	TP_fast_assign(
+		__entry->reason		= reason;
+	),
+
+	TP_printk("reason %s", __print_symbolic(__entry->reason,
+						kvm_trace_exit_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 b8499f5..8800713 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
 		if (arg)
 			goto out;
 		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
+		if (r >= 0)
+			trace_kvm_userspace_exit(vcpu->run->exit_reason);
 		break;
 	case KVM_GET_REGS: {
 		struct kvm_regs *kvm_regs;
--
			Gleb.

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

[Marcelo Tosatti]

On Tue, Oct 05, 2010 at 03:14:32PM +0200, Gleb Natapov wrote:
> Add tracepoint for userspace exit.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
> index 6dd3a51..fb44da0 100644
> --- a/include/trace/events/kvm.h
> +++ b/include/trace/events/kvm.h
> @@ -6,6 +6,31 @@
>  #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),
> +	    TP_ARGS(reason),
> +
> +	TP_STRUCT__entry(
> +		__field(	__u32,		reason		)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->reason		= reason;
> +	),
> +
> +	TP_printk("reason %s", __print_symbolic(__entry->reason,
> +						kvm_trace_exit_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 b8499f5..8800713 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
>  		if (arg)
>  			goto out;
>  		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> +		if (r >= 0)
> +			trace_kvm_userspace_exit(vcpu->run->exit_reason);
>  		break;
>  	case KVM_GET_REGS: {
>  		struct kvm_regs *kvm_regs;
> --
> 			Gleb.

Exit codes are also valid for r == -EINTR and -EAGAIN cases, eg
EXIT_INTR. Better print it out for all cases, and let the reader
decide whether exit_reason is valid.

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

[Gleb Natapov]

On Thu, Oct 07, 2010 at 01:18:39PM -0300, Marcelo Tosatti wrote:
> On Tue, Oct 05, 2010 at 03:14:32PM +0200, Gleb Natapov wrote:
> > Add tracepoint for userspace exit.
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
> > index 6dd3a51..fb44da0 100644
> > --- a/include/trace/events/kvm.h
> > +++ b/include/trace/events/kvm.h
> > @@ -6,6 +6,31 @@
> >  #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),
> > +	    TP_ARGS(reason),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(	__u32,		reason		)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->reason		= reason;
> > +	),
> > +
> > +	TP_printk("reason %s", __print_symbolic(__entry->reason,
> > +						kvm_trace_exit_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 b8499f5..8800713 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
> >  		if (arg)
> >  			goto out;
> >  		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> > +		if (r >= 0)
> > +			trace_kvm_userspace_exit(vcpu->run->exit_reason);
> >  		break;
> >  	case KVM_GET_REGS: {
> >  		struct kvm_regs *kvm_regs;
> > --
> > 			Gleb.
> 
> Exit codes are also valid for r == -EINTR and -EAGAIN cases, eg
> EXIT_INTR. Better print it out for all cases, and let the reader
> decide whether exit_reason is valid.
Are they? I see that userspace does not look into run->exit_reason in
case of -EINTR and -EAGAIN.

--
			Gleb.

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

[Marcelo Tosatti]

On Thu, Oct 07, 2010 at 07:56:55PM +0200, Gleb Natapov wrote:
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > index b8499f5..8800713 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
> > >  		if (arg)
> > >  			goto out;
> > >  		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> > > +		if (r >= 0)
> > > +			trace_kvm_userspace_exit(vcpu->run->exit_reason);
> > >  		break;
> > >  	case KVM_GET_REGS: {
> > >  		struct kvm_regs *kvm_regs;
> > > --
> > > 			Gleb.
> > 
> > Exit codes are also valid for r == -EINTR and -EAGAIN cases, eg
> > EXIT_INTR. Better print it out for all cases, and let the reader
> > decide whether exit_reason is valid.
> Are they? I see that userspace does not look into run->exit_reason in
> case of -EINTR and -EAGAIN.

Not userspace, but for a human reader. Otherwise the trace information
is incomplete and confusing.

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

[Gleb Natapov]

On Thu, Oct 07, 2010 at 09:49:17PM -0300, Marcelo Tosatti wrote:
> On Thu, Oct 07, 2010 at 07:56:55PM +0200, Gleb Natapov wrote:
> > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > > index b8499f5..8800713 100644
> > > > --- a/virt/kvm/kvm_main.c
> > > > +++ b/virt/kvm/kvm_main.c
> > > > @@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
> > > >  		if (arg)
> > > >  			goto out;
> > > >  		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> > > > +		if (r >= 0)
> > > > +			trace_kvm_userspace_exit(vcpu->run->exit_reason);
> > > >  		break;
> > > >  	case KVM_GET_REGS: {
> > > >  		struct kvm_regs *kvm_regs;
> > > > --
> > > > 			Gleb.
> > > 
> > > Exit codes are also valid for r == -EINTR and -EAGAIN cases, eg
> > > EXIT_INTR. Better print it out for all cases, and let the reader
> > > decide whether exit_reason is valid.
> > Are they? I see that userspace does not look into run->exit_reason in
> > case of -EINTR and -EAGAIN.
> 
> Not userspace, but for a human reader. Otherwise the trace information
> is incomplete and confusing.
But if we will always call trace_kvm_userspace_exit() here trace
information will show erroneous info since on any error but -EINTR
exit_reason contains stale info. What about changing r >= 0 to r >= 0 ||
r ==  -EINTR.

--
			Gleb.

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

[Avi Kivity]

  On 10/08/2010 06:44 PM, Gleb Natapov wrote:
> On Thu, Oct 07, 2010 at 09:49:17PM -0300, Marcelo Tosatti wrote:
> >  On Thu, Oct 07, 2010 at 07:56:55PM +0200, Gleb Natapov wrote:
> >  >  >  >  diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> >  >  >  >  index b8499f5..8800713 100644
> >  >  >  >  --- a/virt/kvm/kvm_main.c
> >  >  >  >  +++ b/virt/kvm/kvm_main.c
> >  >  >  >  @@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
> >  >  >  >   		if (arg)
> >  >  >  >   			goto out;
> >  >  >  >   		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> >  >  >  >  +		if (r>= 0)
> >  >  >  >  +			trace_kvm_userspace_exit(vcpu->run->exit_reason);
> >  >  >  >   		break;
> >  >  >  >   	case KVM_GET_REGS: {
> >  >  >  >   		struct kvm_regs *kvm_regs;
> >  >  >  >  --
> >  >  >  >  			Gleb.
> >  >  >
> >  >  >  Exit codes are also valid for r == -EINTR and -EAGAIN cases, eg
> >  >  >  EXIT_INTR. Better print it out for all cases, and let the reader
> >  >  >  decide whether exit_reason is valid.
> >  >  Are they? I see that userspace does not look into run->exit_reason in
> >  >  case of -EINTR and -EAGAIN.
> >
> >  Not userspace, but for a human reader. Otherwise the trace information
> >  is incomplete and confusing.
> But if we will always call trace_kvm_userspace_exit() here trace
> information will show erroneous info since on any error but -EINTR
> exit_reason contains stale info. What about changing r>= 0 to r>= 0 ||
> r ==  -EINTR.

We should log both errno and exit_reason.  If we want to be clever, we 
can display strerror(errno) if it's nonzero, and exit_reason otherwise 
(easy to do in a trace-cmd plugin).

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

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

[Gleb Natapov]

On Sun, Oct 10, 2010 at 10:45:45AM +0200, Avi Kivity wrote:
>  On 10/08/2010 06:44 PM, Gleb Natapov wrote:
> >On Thu, Oct 07, 2010 at 09:49:17PM -0300, Marcelo Tosatti wrote:
> >>  On Thu, Oct 07, 2010 at 07:56:55PM +0200, Gleb Natapov wrote:
> >>  >  >  >  diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> >>  >  >  >  index b8499f5..8800713 100644
> >>  >  >  >  --- a/virt/kvm/kvm_main.c
> >>  >  >  >  +++ b/virt/kvm/kvm_main.c
> >>  >  >  >  @@ -1458,6 +1458,8 @@ static long kvm_vcpu_ioctl(struct file *filp,
> >>  >  >  >   		if (arg)
> >>  >  >  >   			goto out;
> >>  >  >  >   		r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> >>  >  >  >  +		if (r>= 0)
> >>  >  >  >  +			trace_kvm_userspace_exit(vcpu->run->exit_reason);
> >>  >  >  >   		break;
> >>  >  >  >   	case KVM_GET_REGS: {
> >>  >  >  >   		struct kvm_regs *kvm_regs;
> >>  >  >  >  --
> >>  >  >  >  			Gleb.
> >>  >  >
> >>  >  >  Exit codes are also valid for r == -EINTR and -EAGAIN cases, eg
> >>  >  >  EXIT_INTR. Better print it out for all cases, and let the reader
> >>  >  >  decide whether exit_reason is valid.
> >>  >  Are they? I see that userspace does not look into run->exit_reason in
> >>  >  case of -EINTR and -EAGAIN.
> >>
> >>  Not userspace, but for a human reader. Otherwise the trace information
> >>  is incomplete and confusing.
> >But if we will always call trace_kvm_userspace_exit() here trace
> >information will show erroneous info since on any error but -EINTR
> >exit_reason contains stale info. What about changing r>= 0 to r>= 0 ||
> >r ==  -EINTR.
> 
> We should log both errno and exit_reason.  If we want to be clever,
> we can display strerror(errno) if it's nonzero, and exit_reason
> otherwise (easy to do in a trace-cmd plugin).
> 
For starters we should remove KVM_EXIT_INTR exit reason. Looking into
qemu-kvm history it was never used and there is at least one code path
that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
contains stale info on exit.

--
			Gleb.

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

[Avi Kivity]

  On 10/10/2010 05:46 PM, Gleb Natapov wrote:
> >
> >  We should log both errno and exit_reason.  If we want to be clever,
> >  we can display strerror(errno) if it's nonzero, and exit_reason
> >  otherwise (easy to do in a trace-cmd plugin).
> >
> For starters we should remove KVM_EXIT_INTR exit reason. Looking into
> qemu-kvm history it was never used and there is at least one code path
> that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
> contains stale info on exit.
>

The two issues are unrelated.

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

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

[Gleb Natapov]

On Thu, Oct 14, 2010 at 12:27:15PM +0200, Avi Kivity wrote:
>  On 10/10/2010 05:46 PM, Gleb Natapov wrote:
> >>
> >>  We should log both errno and exit_reason.  If we want to be clever,
> >>  we can display strerror(errno) if it's nonzero, and exit_reason
> >>  otherwise (easy to do in a trace-cmd plugin).
> >>
> >For starters we should remove KVM_EXIT_INTR exit reason. Looking into
> >qemu-kvm history it was never used and there is at least one code path
> >that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
> >contains stale info on exit.
> >
> 
> The two issues are unrelated.
> 
So what do you propose? I see no issue with my original patch.

--
			Gleb.

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

[Avi Kivity]

  On 10/14/2010 12:29 PM, Gleb Natapov wrote:
> On Thu, Oct 14, 2010 at 12:27:15PM +0200, Avi Kivity wrote:
> >   On 10/10/2010 05:46 PM, Gleb Natapov wrote:
> >  >>
> >  >>   We should log both errno and exit_reason.  If we want to be clever,
> >  >>   we can display strerror(errno) if it's nonzero, and exit_reason
> >  >>   otherwise (easy to do in a trace-cmd plugin).
> >  >>
> >  >For starters we should remove KVM_EXIT_INTR exit reason. Looking into
> >  >qemu-kvm history it was never used and there is at least one code path
> >  >that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
> >  >contains stale info on exit.
> >  >
> >
> >  The two issues are unrelated.
> >
> So what do you propose? I see no issue with my original patch.

Record both errno and exit_reason.  While they're never both valid at 
the same time, they're both necessary.

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

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

[Gleb Natapov]

On Thu, Oct 14, 2010 at 01:11:20PM +0200, Avi Kivity wrote:
>  On 10/14/2010 12:29 PM, Gleb Natapov wrote:
> >On Thu, Oct 14, 2010 at 12:27:15PM +0200, Avi Kivity wrote:
> >>   On 10/10/2010 05:46 PM, Gleb Natapov wrote:
> >>  >>
> >>  >>   We should log both errno and exit_reason.  If we want to be clever,
> >>  >>   we can display strerror(errno) if it's nonzero, and exit_reason
> >>  >>   otherwise (easy to do in a trace-cmd plugin).
> >>  >>
> >>  >For starters we should remove KVM_EXIT_INTR exit reason. Looking into
> >>  >qemu-kvm history it was never used and there is at least one code path
> >>  >that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
> >>  >contains stale info on exit.
> >>  >
> >>
> >>  The two issues are unrelated.
> >>
> >So what do you propose? I see no issue with my original patch.
> 
> Record both errno and exit_reason.  While they're never both valid
> at the same time, they're both necessary.
> 
If they can't be valid at the same time, why not record one of them? The
one that happened? Also any error other then -EINTR will cause qemu to
stop, so it is not very interesting. And ioctl return value can be
traced by strace anyway.

--
			Gleb.

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

[Avi Kivity]

  On 10/14/2010 01:28 PM, Gleb Natapov wrote:
> On Thu, Oct 14, 2010 at 01:11:20PM +0200, Avi Kivity wrote:
> >   On 10/14/2010 12:29 PM, Gleb Natapov wrote:
> >  >On Thu, Oct 14, 2010 at 12:27:15PM +0200, Avi Kivity wrote:
> >  >>    On 10/10/2010 05:46 PM, Gleb Natapov wrote:
> >  >>   >>
> >  >>   >>    We should log both errno and exit_reason.  If we want to be clever,
> >  >>   >>    we can display strerror(errno) if it's nonzero, and exit_reason
> >  >>   >>    otherwise (easy to do in a trace-cmd plugin).
> >  >>   >>
> >  >>   >For starters we should remove KVM_EXIT_INTR exit reason. Looking into
> >  >>   >qemu-kvm history it was never used and there is at least one code path
> >  >>   >that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
> >  >>   >contains stale info on exit.
> >  >>   >
> >  >>
> >  >>   The two issues are unrelated.
> >  >>
> >  >So what do you propose? I see no issue with my original patch.
> >
> >  Record both errno and exit_reason.  While they're never both valid
> >  at the same time, they're both necessary.
> >
> If they can't be valid at the same time, why not record one of them?

If you record just one, you don't know if the other one happened.

> The
> one that happened? Also any error other then -EINTR will cause qemu to
> stop, so it is not very interesting. And ioctl return value can be
> traced by strace anyway.

You can't correlate it with ftrace.

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

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

[Gleb Natapov]

On Thu, Oct 14, 2010 at 01:32:14PM +0200, Avi Kivity wrote:
>  On 10/14/2010 01:28 PM, Gleb Natapov wrote:
> >On Thu, Oct 14, 2010 at 01:11:20PM +0200, Avi Kivity wrote:
> >>   On 10/14/2010 12:29 PM, Gleb Natapov wrote:
> >>  >On Thu, Oct 14, 2010 at 12:27:15PM +0200, Avi Kivity wrote:
> >>  >>    On 10/10/2010 05:46 PM, Gleb Natapov wrote:
> >>  >>   >>
> >>  >>   >>    We should log both errno and exit_reason.  If we want to be clever,
> >>  >>   >>    we can display strerror(errno) if it's nonzero, and exit_reason
> >>  >>   >>    otherwise (easy to do in a trace-cmd plugin).
> >>  >>   >>
> >>  >>   >For starters we should remove KVM_EXIT_INTR exit reason. Looking into
> >>  >>   >qemu-kvm history it was never used and there is at least one code path
> >>  >>   >that returns -EINTR and does not set KVM_EXIT_INTR, so exit_reason field
> >>  >>   >contains stale info on exit.
> >>  >>   >
> >>  >>
> >>  >>   The two issues are unrelated.
> >>  >>
> >>  >So what do you propose? I see no issue with my original patch.
> >>
> >>  Record both errno and exit_reason.  While they're never both valid
> >>  at the same time, they're both necessary.
> >>
> >If they can't be valid at the same time, why not record one of them?
> 
> If you record just one, you don't know if the other one happened.
> 
I mean to record type/value. So it can be (type error/value -EINVAL) or
(type exit/value HALT). The goal is to not print non-relevant info in
ftrace, but we can do the same with recording errno and exit_reason and
show only exit_reason is errno>=0 or errno otherwise.

> >The
> >one that happened? Also any error other then -EINTR will cause qemu to
> >stop, so it is not very interesting. And ioctl return value can be
> >traced by strace anyway.
> 
> You can't correlate it with ftrace.
> 
True. But given that the only interesting error code in -EINTR I do not
know if this is useful, but potentially may generate a lot of events in
ftrace. Sometimes too much info is almost as bad as not enough and if we
will use the same event for both exit_reason and errno it will be
impossible to enable one without the other.

--
			Gleb.

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

[Avi Kivity]

  On 10/14/2010 01:41 PM, Gleb Natapov wrote:
> >  >>   >So what do you propose? I see no issue with my original patch.
> >  >>
> >  >>   Record both errno and exit_reason.  While they're never both valid
> >  >>   at the same time, they're both necessary.
> >  >>
> >  >If they can't be valid at the same time, why not record one of them?
> >
> >  If you record just one, you don't know if the other one happened.
> >
> I mean to record type/value. So it can be (type error/value -EINVAL) or
> (type exit/value HALT). The goal is to not print non-relevant info in
> ftrace, but we can do the same with recording errno and exit_reason and
> show only exit_reason is errno>=0 or errno otherwise.

That's fine.  As long as you don't drop information.

> >  >The
> >  >one that happened? Also any error other then -EINTR will cause qemu to
> >  >stop, so it is not very interesting. And ioctl return value can be
> >  >traced by strace anyway.
> >
> >  You can't correlate it with ftrace.
> >
> True. But given that the only interesting error code in -EINTR I do not
> know if this is useful, but potentially may generate a lot of events in
> ftrace. Sometimes too much info is almost as bad as not enough and if we
> will use the same event for both exit_reason and errno it will be
> impossible to enable one without the other.

You can always filter excess information away.  If you're looking for 
exits to userspace (a major performance issue) then you want to see both 
normal exits and signal exits.

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

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

[Gleb Natapov]

On Thu, Oct 14, 2010 at 01:43:52PM +0200, Avi Kivity wrote:
>  On 10/14/2010 01:41 PM, Gleb Natapov wrote:
> >>  >>   >So what do you propose? I see no issue with my original patch.
> >>  >>
> >>  >>   Record both errno and exit_reason.  While they're never both valid
> >>  >>   at the same time, they're both necessary.
> >>  >>
> >>  >If they can't be valid at the same time, why not record one of them?
> >>
> >>  If you record just one, you don't know if the other one happened.
> >>
> >I mean to record type/value. So it can be (type error/value -EINVAL) or
> >(type exit/value HALT). The goal is to not print non-relevant info in
> >ftrace, but we can do the same with recording errno and exit_reason and
> >show only exit_reason is errno>=0 or errno otherwise.
> 
> That's fine.  As long as you don't drop information.
> 
> >>  >The
> >>  >one that happened? Also any error other then -EINTR will cause qemu to
> >>  >stop, so it is not very interesting. And ioctl return value can be
> >>  >traced by strace anyway.
> >>
> >>  You can't correlate it with ftrace.
> >>
> >True. But given that the only interesting error code in -EINTR I do not
> >know if this is useful, but potentially may generate a lot of events in
> >ftrace. Sometimes too much info is almost as bad as not enough and if we
> >will use the same event for both exit_reason and errno it will be
> >impossible to enable one without the other.
> 
> You can always filter excess information away.  If you're looking
> for exits to userspace (a major performance issue) then you want to
> see both normal exits and signal exits.
> 
How do I do it via /debug file system? Now I just echo name of the event
I am interested in set_event, how do I add filter to that?

--
			Gleb.

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

[Avi Kivity]

  On 10/14/2010 01:47 PM, Gleb Natapov wrote:
> >
> >  You can always filter excess information away.  If you're looking
> >  for exits to userspace (a major performance issue) then you want to
> >  see both normal exits and signal exits.
> >
> How do I do it via /debug file system? Now I just echo name of the event
> I am interested in set_event, how do I add filter to that?

echo exit_reason==33 > /sys/kernel/debug/tracing/events/kvm/kvm_exit/filter

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

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

[Avi Kivity]

  On 10/14/2010 02:09 PM, Avi Kivity wrote:
>  On 10/14/2010 01:47 PM, Gleb Natapov wrote:
>> >
>> >  You can always filter excess information away.  If you're looking
>> >  for exits to userspace (a major performance issue) then you want to
>> >  see both normal exits and signal exits.
>> >
>> How do I do it via /debug file system? Now I just echo name of the event
>> I am interested in set_event, how do I add filter to that?
>
> echo exit_reason==33 > 
> /sys/kernel/debug/tracing/events/kvm/kvm_exit/filter
>

Or, easier,

   trace-cmd record -e kvm:kvm_exit -f exit_reason==33

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