* [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin @ 2012-12-25 11:46 Gleb Natapov 2012-12-26 13:58 ` Steven Rostedt 0 siblings, 1 reply; 5+ messages in thread From: Gleb Natapov @ 2012-12-25 11:46 UTC (permalink / raw) To: srostedt; +Cc: kvm kvm_mmu_zap_page even was renamed to kvm_mmu_prepare_zap_page. Print out created field for kvm_mmu_get_page event. Signed-off-by: Gleb Natapov <gleb@redhat.com> diff --git a/plugin_kvm.c b/plugin_kvm.c index 55812ef..adc5694 100644 --- a/plugin_kvm.c +++ b/plugin_kvm.c @@ -382,7 +382,7 @@ static int kvm_mmu_print_role(struct trace_seq *s, struct pevent_record *record, } else trace_seq_printf(s, "WORD: %08x", role.word); - pevent_print_num_field(s, " root %u", event, + pevent_print_num_field(s, " root %u ", event, "root_count", record, 1); if (pevent_get_field_val(s, event, "unsync", record, &val, 1) < 0) @@ -397,6 +397,11 @@ static int kvm_mmu_get_page_handler(struct trace_seq *s, struct pevent_record *r { unsigned long long val; + if (pevent_get_field_val(s, event, "created", record, &val, 1) < 0) + return -1; + + trace_seq_printf(s, "%s ", val ? "new" : "existing"); + if (pevent_get_field_val(s, event, "gfn", record, &val, 1) < 0) return -1; @@ -430,7 +435,7 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent) pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_unsync_page", kvm_mmu_print_role, NULL); - pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_zap_page", + pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_prepare_zap_page", kvm_mmu_print_role, NULL); return 0; -- Gleb. ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin 2012-12-25 11:46 [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin Gleb Natapov @ 2012-12-26 13:58 ` Steven Rostedt 2012-12-26 14:13 ` Gleb Natapov 0 siblings, 1 reply; 5+ messages in thread From: Steven Rostedt @ 2012-12-26 13:58 UTC (permalink / raw) To: Gleb Natapov; +Cc: kvm On Tue, 2012-12-25 at 13:46 +0200, Gleb Natapov wrote: > kvm_mmu_zap_page even was renamed to kvm_mmu_prepare_zap_page. > Print out created field for kvm_mmu_get_page event. trace-cmd needs to be backward compatible with older kernels. If older kernels used kvm_mmu_zap_page, then please add a check for that and have the plugin cope with either one. Thanks, -- Steve > > Signed-off-by: Gleb Natapov <gleb@redhat.com> > diff --git a/plugin_kvm.c b/plugin_kvm.c > index 55812ef..adc5694 100644 > --- a/plugin_kvm.c > +++ b/plugin_kvm.c > @@ -382,7 +382,7 @@ static int kvm_mmu_print_role(struct trace_seq *s, struct pevent_record *record, > } else > trace_seq_printf(s, "WORD: %08x", role.word); > > - pevent_print_num_field(s, " root %u", event, > + pevent_print_num_field(s, " root %u ", event, > "root_count", record, 1); > > if (pevent_get_field_val(s, event, "unsync", record, &val, 1) < 0) > @@ -397,6 +397,11 @@ static int kvm_mmu_get_page_handler(struct trace_seq *s, struct pevent_record *r > { > unsigned long long val; > > + if (pevent_get_field_val(s, event, "created", record, &val, 1) < 0) > + return -1; > + > + trace_seq_printf(s, "%s ", val ? "new" : "existing"); > + > if (pevent_get_field_val(s, event, "gfn", record, &val, 1) < 0) > return -1; > > @@ -430,7 +435,7 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent) > pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_unsync_page", > kvm_mmu_print_role, NULL); > > - pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_zap_page", > + pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_prepare_zap_page", > kvm_mmu_print_role, NULL); > > return 0; > -- > Gleb. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin 2012-12-26 13:58 ` Steven Rostedt @ 2012-12-26 14:13 ` Gleb Natapov 2012-12-26 14:22 ` Steven Rostedt 0 siblings, 1 reply; 5+ messages in thread From: Gleb Natapov @ 2012-12-26 14:13 UTC (permalink / raw) To: Steven Rostedt; +Cc: kvm On Wed, Dec 26, 2012 at 08:58:30AM -0500, Steven Rostedt wrote: > On Tue, 2012-12-25 at 13:46 +0200, Gleb Natapov wrote: > > kvm_mmu_zap_page even was renamed to kvm_mmu_prepare_zap_page. > > Print out created field for kvm_mmu_get_page event. > > trace-cmd needs to be backward compatible with older kernels. If older > kernels used kvm_mmu_zap_page, then please add a check for that and have > the plugin cope with either one. > Something like this? if (pevent_find_event_by_name(pevent, "kvmmmu", "kvm_mmu_prepare_zap_page")) pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_prepare_zap_page", kvm_mmu_print_role, NULL); else pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_zap_page", kvm_mmu_print_role, NULL); Also when trace-cmd encounters an event it does not recognize it prints mysterious message: trace-cmd: No such file or directory bad op token { Is this a bug? > Thanks, > > -- Steve > > > > > Signed-off-by: Gleb Natapov <gleb@redhat.com> > > diff --git a/plugin_kvm.c b/plugin_kvm.c > > index 55812ef..adc5694 100644 > > --- a/plugin_kvm.c > > +++ b/plugin_kvm.c > > @@ -382,7 +382,7 @@ static int kvm_mmu_print_role(struct trace_seq *s, struct pevent_record *record, > > } else > > trace_seq_printf(s, "WORD: %08x", role.word); > > > > - pevent_print_num_field(s, " root %u", event, > > + pevent_print_num_field(s, " root %u ", event, > > "root_count", record, 1); > > > > if (pevent_get_field_val(s, event, "unsync", record, &val, 1) < 0) > > @@ -397,6 +397,11 @@ static int kvm_mmu_get_page_handler(struct trace_seq *s, struct pevent_record *r > > { > > unsigned long long val; > > > > + if (pevent_get_field_val(s, event, "created", record, &val, 1) < 0) > > + return -1; > > + > > + trace_seq_printf(s, "%s ", val ? "new" : "existing"); > > + > > if (pevent_get_field_val(s, event, "gfn", record, &val, 1) < 0) > > return -1; > > > > @@ -430,7 +435,7 @@ int PEVENT_PLUGIN_LOADER(struct pevent *pevent) > > pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_unsync_page", > > kvm_mmu_print_role, NULL); > > > > - pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_zap_page", > > + pevent_register_event_handler(pevent, -1, "kvmmmu", "kvm_mmu_prepare_zap_page", > > kvm_mmu_print_role, NULL); > > > > return 0; > > -- > > Gleb. > -- Gleb. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin 2012-12-26 14:13 ` Gleb Natapov @ 2012-12-26 14:22 ` Steven Rostedt 2012-12-27 11:22 ` Gleb Natapov 0 siblings, 1 reply; 5+ messages in thread From: Steven Rostedt @ 2012-12-26 14:22 UTC (permalink / raw) To: Gleb Natapov; +Cc: kvm On Wed, 2012-12-26 at 16:13 +0200, Gleb Natapov wrote: > On Wed, Dec 26, 2012 at 08:58:30AM -0500, Steven Rostedt wrote: > > On Tue, 2012-12-25 at 13:46 +0200, Gleb Natapov wrote: > > > kvm_mmu_zap_page even was renamed to kvm_mmu_prepare_zap_page. > > > Print out created field for kvm_mmu_get_page event. > > > > trace-cmd needs to be backward compatible with older kernels. If older > > kernels used kvm_mmu_zap_page, then please add a check for that and have > > the plugin cope with either one. > > > Something like this? > > if (pevent_find_event_by_name(pevent, "kvmmmu", "kvm_mmu_prepare_zap_page")) > pevent_register_event_handler(pevent, -1, "kvmmmu", > "kvm_mmu_prepare_zap_page", kvm_mmu_print_role, > NULL); > else > pevent_register_event_handler(pevent, -1, "kvmmmu", > "kvm_mmu_zap_page", kvm_mmu_print_role, NULL); Sure, if it works. > > Also when trace-cmd encounters an event it does not recognize it prints > mysterious message: > > trace-cmd: No such file or directory > bad op token { > > Is this a bug? > Yes and no ;-) I need to get rid of the "perror" part, for errors that don't set errno. That's been on my todo list for a long time. Maybe when I come back to work next week I'll fix that. The 'bad op token {' happens when it tries to parse an event and it comes across something that it doesn't recognize. In this case a '{'. The kvm events are notorious with having extremely complex "print_fmt" fields in their event format files. When no event handler is registered for an event, trace-cmd uses the print_fmt to figure out how to print it. This was never meant to be too complex of a parser. For example, looking at my kvm_mmu_prepare_zap_page, we have: print fmt: "%s", ({ 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; role.word = REC->role; trace_seq_printf(p, "sp gfn %llx %u%s q%u%s %s%s" " %snxe root %u %s%c", REC->gfn, role.level, role.cr4_pae ? " pae" : "", role.quadrant, role.direct ? " direct" : "", access_str[role.access], role.invalid ? " invalid" : "", role.nxe ? "" : "!", REC->root_count, REC->unsync ? "unsync" : "sync", 0); ret; }) trace-cmd has no idea on how to parse "({ const char *ret ..." in fact it dies on that first "{" because trace-cmd is not a full C parser. This was why the plugins were created in the first place. To handle various events that have too complex print_fmts for trace-cmd to understand. Most events are simple print_fmts and do not need plugins. Like sock_rcvqueue_full: print fmt: "rmem_alloc=%d truesize=%u sk_rcvbuf=%d", REC->rmem_alloc, REC->truesize, REC->sk_rcvbuf trace-cmd has no problem parsing events like that. -- Steve ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin 2012-12-26 14:22 ` Steven Rostedt @ 2012-12-27 11:22 ` Gleb Natapov 0 siblings, 0 replies; 5+ messages in thread From: Gleb Natapov @ 2012-12-27 11:22 UTC (permalink / raw) To: Steven Rostedt; +Cc: kvm On Wed, Dec 26, 2012 at 09:22:32AM -0500, Steven Rostedt wrote: > On Wed, 2012-12-26 at 16:13 +0200, Gleb Natapov wrote: > > On Wed, Dec 26, 2012 at 08:58:30AM -0500, Steven Rostedt wrote: > > > On Tue, 2012-12-25 at 13:46 +0200, Gleb Natapov wrote: > > > > kvm_mmu_zap_page even was renamed to kvm_mmu_prepare_zap_page. > > > > Print out created field for kvm_mmu_get_page event. > > > > > > trace-cmd needs to be backward compatible with older kernels. If older > > > kernels used kvm_mmu_zap_page, then please add a check for that and have > > > the plugin cope with either one. > > > > > Something like this? > > > > if (pevent_find_event_by_name(pevent, "kvmmmu", "kvm_mmu_prepare_zap_page")) > > pevent_register_event_handler(pevent, -1, "kvmmmu", > > "kvm_mmu_prepare_zap_page", kvm_mmu_print_role, > > NULL); > > else > > pevent_register_event_handler(pevent, -1, "kvmmmu", > > "kvm_mmu_zap_page", kvm_mmu_print_role, NULL); > > Sure, if it works. > Well, it isn't and if it were it would have prevented tarce-cmd running on new kernel from reading traces generated on older kernels. I will register both. > > > > Also when trace-cmd encounters an event it does not recognize it prints > > mysterious message: > > > > trace-cmd: No such file or directory > > bad op token { > > > > Is this a bug? > > > > Yes and no ;-) > > I need to get rid of the "perror" part, for errors that don't set errno. > That's been on my todo list for a long time. Maybe when I come back to > work next week I'll fix that. > > The 'bad op token {' happens when it tries to parse an event and it > comes across something that it doesn't recognize. In this case a '{'. > > The kvm events are notorious with having extremely complex "print_fmt" > fields in their event format files. When no event handler is registered > for an event, trace-cmd uses the print_fmt to figure out how to print > it. This was never meant to be too complex of a parser. For example, > looking at my kvm_mmu_prepare_zap_page, we have: > > print fmt: "%s", ({ 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; > role.word = REC->role; trace_seq_printf(p, "sp gfn %llx %u%s q%u%s %s%s" " %snxe root %u > %s%c", REC->gfn, role.level, role.cr4_pae ? " pae" : "", role.quadrant, role.direct ? " > direct" : "", access_str[role.access], role.invalid ? " invalid" : "", role.nxe ? "" : "!", > REC->root_count, REC->unsync ? "unsync" : "sync", 0); ret; }) > > trace-cmd has no idea on how to parse "({ const char *ret ..." in fact > it dies on that first "{" because trace-cmd is not a full C parser. > > This was why the plugins were created in the first place. To handle > various events that have too complex print_fmts for trace-cmd to > understand. > > Most events are simple print_fmts and do not need plugins. Like > sock_rcvqueue_full: > > print fmt: "rmem_alloc=%d truesize=%u sk_rcvbuf=%d", REC->rmem_alloc, > REC->truesize, REC->sk_rcvbuf > > trace-cmd has no problem parsing events like that. > Got it. -- Gleb. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-27 11:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-25 11:46 [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin Gleb Natapov 2012-12-26 13:58 ` Steven Rostedt 2012-12-26 14:13 ` Gleb Natapov 2012-12-26 14:22 ` Steven Rostedt 2012-12-27 11:22 ` Gleb Natapov
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).