Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin

2012-12-27 Thread Gleb Natapov
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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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 Thread Steven Rostedt
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 g...@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.


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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 Thread Gleb Natapov
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 g...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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 Thread Steven Rostedt
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


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-12-25 Thread 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.

Signed-off-by: Gleb Natapov g...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html