trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin

PATCH Superseded Gleb Natapov 2 participants 5 messages Dec 25, 2012 – Dec 27, 2012 KVM

Share & tools

Gleb Natapov

author

2012-12-26 14:13 UTC #2 lore raw

Quoted 8 lines · Steven Rostedt

Quoted 2 lines · Gleb Natapov
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?

Quoted 35 lines

Thanks,

-- Steve

Quoted 31 lines

Signed-off-by Gleb Natapov <[email protected]>

plugin_kvm.c +6 -1

index 55812ef..adc5694 100644
@@ -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);

Quoted 8 lines

Quoted 7 lines

plugin_kvm.c +1 -2

-	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.

Steven Rostedt

2012-12-26 14:22 UTC #3 lore raw

Quoted 11 lines · Gleb Natapov

Quoted 8 lines · Steven Rostedt
Quoted 2 lines · Gleb Natapov
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?

Quoted 7 lines

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.

Quoted 9 lines

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

Gleb Natapov

author

2012-12-27 11:22 UTC #4 lore raw

Quoted 20 lines · Steven Rostedt

Quoted 11 lines · Gleb Natapov
Quoted 8 lines · Steven Rostedt
Quoted 2 lines · Gleb Natapov
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?
Quoted 7 lines
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);

Quoted 2 lines

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.

Quoted 39 lines

Quoted 9 lines
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.

Quoted 8 lines

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.