2009-11-23 11:06:14

by Johannes Berg

[permalink] [raw]
Subject: kvmmmu tracing

Commit f691fe1da7e2715137d21ae5a80bec64db4625db is really broken wrt.
the userspace interface for tracing because of the weird
KVM_MMU_PAGE_PRINTK macro.

Maybe we should have a "unsafe for export" flag for events, if they do
strange things like that?

As it stands, I can't use trace-cmd on an x86 machine that has kvm
enabled because it will try to read all the kvm stuff. Arguably, it
should only try to parse it when it needs it (i.e. not for me) but still
it's very inconvenient to export something to userspace that it cannot
possibly understand.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-11-23 14:59:20

by Steven Rostedt

[permalink] [raw]
Subject: Re: kvmmmu tracing

On Mon, 2009-11-23 at 12:06 +0100, Johannes Berg wrote:
> Commit f691fe1da7e2715137d21ae5a80bec64db4625db is really broken wrt.
> the userspace interface for tracing because of the weird
> KVM_MMU_PAGE_PRINTK macro.
>
> Maybe we should have a "unsafe for export" flag for events, if they do
> strange things like that?
>
> As it stands, I can't use trace-cmd on an x86 machine that has kvm
> enabled because it will try to read all the kvm stuff. Arguably, it
> should only try to parse it when it needs it (i.e. not for me) but still
> it's very inconvenient to export something to userspace that it cannot
> possibly understand.

Note, I updated trace-cmd. I still does not read this macro nicely, but
it at least does not fail anymore.

-- Steve

2009-11-23 15:46:43

by Johannes Berg

[permalink] [raw]
Subject: Re: kvmmmu tracing

On Mon, 2009-11-23 at 09:58 -0500, Steven Rostedt wrote:
> On Mon, 2009-11-23 at 12:06 +0100, Johannes Berg wrote:
> > Commit f691fe1da7e2715137d21ae5a80bec64db4625db is really broken wrt.
> > the userspace interface for tracing because of the weird
> > KVM_MMU_PAGE_PRINTK macro.
> >
> > Maybe we should have a "unsafe for export" flag for events, if they do
> > strange things like that?
> >
> > As it stands, I can't use trace-cmd on an x86 machine that has kvm
> > enabled because it will try to read all the kvm stuff. Arguably, it
> > should only try to parse it when it needs it (i.e. not for me) but still
> > it's very inconvenient to export something to userspace that it cannot
> > possibly understand.
>
> Note, I updated trace-cmd. I still does not read this macro nicely, but
> it at least does not fail anymore.

Ah, good to know. So I guess I don't care any more since I'm not
actually tracing this. However, since trace-cmd is very convenient for
record + send-by-email + report/analyse, it might make sense to fix
anyway.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-11-24 09:51:28

by Avi Kivity

[permalink] [raw]
Subject: Re: kvmmmu tracing

On 11/23/2009 01:06 PM, Johannes Berg wrote:
> Commit f691fe1da7e2715137d21ae5a80bec64db4625db is really broken wrt.
> the userspace interface for tracing because of the weird
> KVM_MMU_PAGE_PRINTK macro.
>
>

Can you explain what is wrong with it?

> Maybe we should have a "unsafe for export" flag for events, if they do
> strange things like that?
>
> As it stands, I can't use trace-cmd on an x86 machine that has kvm
> enabled because it will try to read all the kvm stuff. Arguably, it
> should only try to parse it when it needs it (i.e. not for me) but still
> it's very inconvenient to export something to userspace that it cannot
> possibly understand.
>

Is userspace reading mmutrace.h? When the structure attributes can be
exported via /sys/kernel/debug/tracing?


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

2009-11-24 10:05:59

by Johannes Berg

[permalink] [raw]
Subject: Re: kvmmmu tracing

On Tue, 2009-11-24 at 11:50 +0200, Avi Kivity wrote:
> On 11/23/2009 01:06 PM, Johannes Berg wrote:
> > Commit f691fe1da7e2715137d21ae5a80bec64db4625db is really broken wrt.
> > the userspace interface for tracing because of the weird
> > KVM_MMU_PAGE_PRINTK macro.
> >
> >
>
> Can you explain what is wrong with it?

It's a big C expression that trace-cmd can't parse :)


> Is userspace reading mmutrace.h? When the structure attributes can be
> exported via /sys/kernel/debug/tracing?

Yes ... look
at /sys/kernel/debug/tracing/events/kvmmmu/kvm_mmu_unsync_page/format
for instance.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-11-24 10:34:29

by Avi Kivity

[permalink] [raw]
Subject: Re: kvmmmu tracing

On 11/24/2009 12:05 PM, Johannes Berg wrote:
> On Tue, 2009-11-24 at 11:50 +0200, Avi Kivity wrote:
>
>> On 11/23/2009 01:06 PM, Johannes Berg wrote:
>>
>>> Commit f691fe1da7e2715137d21ae5a80bec64db4625db is really broken wrt.
>>> the userspace interface for tracing because of the weird
>>> KVM_MMU_PAGE_PRINTK macro.
>>>
>>>
>>>
>> Can you explain what is wrong with it?
>>
> It's a big C expression that trace-cmd can't parse :)
>

Um, C can be easily parsed with a C compiler. I don't think you can
expect it to be a plain format string and argument list.

>> Is userspace reading mmutrace.h? When the structure attributes can be
>> exported via /sys/kernel/debug/tracing?
>>
> Yes ... look
> at /sys/kernel/debug/tracing/events/kvmmmu/kvm_mmu_unsync_page/format
> for instance.
>

You can fall back to using the attributes to build your own format string.

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

2009-11-24 14:09:01

by Johannes Berg

[permalink] [raw]
Subject: Re: kvmmmu tracing

On Tue, 2009-11-24 at 12:34 +0200, Avi Kivity wrote:

> Um, C can be easily parsed with a C compiler. I don't think you can
> expect it to be a plain format string and argument list.

Actually, it turns out that it cannot be parsed even with a C compiler:

({ const char *ret = p->buffer + p->len; static const char *access_str[]
= { "---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux" }; union
kvm_mmu_page_role role;

...

userspace cannot possibly know from this what "union kvm_mmu_page_role"
is.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-11-24 14:18:06

by Avi Kivity

[permalink] [raw]
Subject: Re: kvmmmu tracing

On 11/24/2009 04:08 PM, Johannes Berg wrote:
> On Tue, 2009-11-24 at 12:34 +0200, Avi Kivity wrote:
>
>
>> Um, C can be easily parsed with a C compiler. I don't think you can
>> expect it to be a plain format string and argument list.
>>
> Actually, it turns out that it cannot be parsed even with a C compiler:
>
> ({ const char *ret = p->buffer + p->len; static const char *access_str[]
> = { "---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux" }; union
> kvm_mmu_page_role role;
>
> ...
>
> userspace cannot possibly know from this what "union kvm_mmu_page_role"
> is.
>

We can expose kvm_mmu_page_role, but that's a new can of worms. And
it's certainly not meant to be stable across kernel versions.

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