[Systemtap][RFC] sample test script and tapset for markers

[Systemtap][RFC] sample test script and tapset for markers

[Masami Hiramatsu]

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

Hi,

Here is a tapset and sample script for markers.
This tapset partially covers LTTng markers(with our additional
patches for markers which I attached to the previous mails).

In this marker tapset, I used the marker.* namespace for
markers.

ex)
marker.irq.softirq.exit

2nd symbol means subsystem and 3nd or later symbols are
depends on the subsystem.

As you can see in sample.stp, we can easily specify
the events occurred on each subsystem.

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


[-- Attachment #2: marker.stp --]
[-- Type: text/plain, Size: 7227 bytes --]

//
// kernel marker tapset
//
// This file is part of systemtap, and is free software.  You can
// redistribute it and/or modify it under the terms of the GNU General
// Public License (GPL); either version 2, or (at your option) any
// later version.

/*
 * Each marker has "name" variable which stores marker name.
 */

/*-------------------------------------------*
 * marker.irq.* - IRQ related markers
 *-------------------------------------------*/

/*
 * probe marker.irq.hardirq.entry
 *
 *  Fires just before handing a hard interrupt.
 *
 * Context:
 *  A hard interrupt occurs.
 *
 * Arguments:
 *  irq_id - the id of hard irq.
 *  kernel_mode - boolean indicating whether the interrupt occured
 *                in kernel mode.
 */
probe marker.irq.hardirq.entry
	= kernel.mark("kernel_irq_entry")
{
	name = "kernel_irq_entry"
	irq_id = $arg1
	kernel_mode = $arg2
}

/*
 * probe marker.irq.hardirq.exit
 *
 *  Fires after handling a hard interrupt.
 *
 * Context:
 *  A hard interrupt was handled.
 *
 * Arguments:
 *  irq_id - the id of hard irq.
 *  retval - return value of handle_IRQ_event().
 */
probe marker.irq.hardirq.exit
	= kernel.mark("kernel_irq_exit")
{
	name = "kernel_irq_exit"
	irq_id = $arg1
	retval = $arg2
}

/*
 * probe marker.irq.softirq.enter
 *
 *  Fires before calling softirq handler.
 *
 * Context:
 *  Softirq handler will be invoked.
 *
 * Arguments:
 *  softirq_id - the id of softirq.
 *  func - the pointer of softirq handler.
 *
 * Note:
 *  On specific architecture(ex. ia64) the "func" is not the address of
 * the actuall function. You might need to derefer *func for getting
 * actuall adress.
 */
probe marker.irq.softirq.entry
	= kernel.mark("kernel_softirq_entry")
{
	name = "kernel_softirq_entry"
	softirq_id = $arg1
	func = $arg2
}

/*
 * probe marker.irq.softirq.exit
 *
 *  Fires after calling softirq handler.
 *
 * Context:
 *  Softirq handler was handled.
 *
 * Arguments:
 *  softirq_id - the id of softirq.
 */
probe marker.irq.softirq.exit
	= kernel.mark("kernel_softirq_exit")
{
	name = "kernel_softirq_exit"
	softirq_id = $arg1
}

/*
 * probe marker.irq.tasklet_low.enter
 *
 *  Fires before calling tasklet_low handler.
 *
 * Context:
 *  Tasklet(low) handler will be invoked.
 *
 * Arguments:
 *  func - the pointer of tasklet_low handler.
 *  data - the data passed to the handler.
 *
 * Note:
 *  On specific architecture(ex. ia64) the "func" is not the address of
 * the actuall function. You might need to derefer *func for getting
 * actuall adress.
 */
probe marker.irq.tasklet_low.entry
	= kernel.mark("kernel_tasklet_low_entry")
{
	name = "kernel_tasklet_low_entry"
	func = $arg1
	data = $arg2
}

/*
 * probe marker.irq.tasklet_low.exit
 *
 *  Fires after calling tasklet_low handler.
 *
 * Context:
 *  Tasklet(low) handler was handled.
 *
 * Arguments:
 *  func - the pointer of tasklet_low handler.
 *  data - the data passed to the handler.
 *
 * Note:
 *  On specific architecture(ex. ia64) the "func" is not the address of
 * the actuall function. You might need to derefer *func for getting
 * actuall adress.
 */
probe marker.irq.tasklet_low.exit
	= kernel.mark("kernel_tasklet_low_exit")
{
	name = "kernel_tasklet_low_exit"
	func = $arg1
	data = $arg2
}

/*
 * probe marker.irq.tasklet_high.enter
 *
 *  Fires before calling tasklet_high handler.
 *
 * Context:
 *  Tasklet(high) handler will be invoked.
 *
 * Arguments:
 *  func - the pointer of tasklet_high handler.
 *  data - the data passed to the handler.
 *
 * Note:
 *  On specific architecture(ex. ia64) the "func" is not the address of
 * the actuall function. You might need to derefer *func for getting
 * actuall adress.
 */
probe marker.irq.tasklet_high.entry
	= kernel.mark("kernel_tasklet_high_entry")
{
	name = "kernel_tasklet_high_entry"
	func = $arg1
	data = $arg2
}

/*
 * probe marker.irq.tasklet_high.exit
 *
 *  Fires after calling tasklet_high handler.
 *
 * Context:
 *  Tasklet(high) handler was handled.
 *
 * Arguments:
 *  func - the pointer of tasklet_high handler.
 *  data - the data passed to the handler.
 *
 * Note:
 *  On specific architecture(ex. ia64) the "func" is not the address of
 * the actuall function. You might need to derefer *func for getting
 * actuall adress.
 */
probe marker.irq.tasklet_high.exit
	= kernel.mark("kernel_tasklet_high_exit")
{
	name = "kernel_tasklet_high_exit"
	func = $arg1
	data = $arg2
}


/*-------------------------------------------*
 * marker.process.* - Process related markers
 *-------------------------------------------*/

/*
 * probe marker.process.free
 *
 *  Fires when task_struct is released.
 *
 * Context:
 *  A process is released.
 *
 * Arguments:
 *  pid - released process pid.
 */
probe marker.process.free
	= kernel.mark("kernel_process_free")
{
	name = "kernel_process_free"
	pid = $arg1
}

/*
 * probe marker.process.wait
 *
 *  Fires when do_wait() is called.
 *
 * Context:
 *  Current process waits another process.
 *
 * Arguments:
 *  pid - waiting target pid.
 */
probe marker.process.wait
	= kernel.mark("kernel_process_wait")
{
	name = "kernel_process_wait"
	pid = $arg1
}

/*
 * probe marker.process.exit
 *
 *  Fires when do_exit() is called.
 *
 * Context:
 *  A process exits.
 *
 * Arguments:
 *  pid - exit process pid.
 */
probe marker.process.exit
	= kernel.mark("kernel_process_exit")
{
	name = "kernel_process_exit"
	pid = $arg1
}

/*
 * probe marker.process.fork
 *
 *  Fires when a process forked/cloned.
 *
 * Context:
 *  A process forked/cloned.
 *
 * Arguments:
 *  pid - parent pid.
 *  parent_pid - same as above "pid".
 *  child_pid - forked child pid.
 *  child_tgid - forked child thread group id.
 */
probe marker.process.fork
	= kernel.mark("kernel_process_fork")
{
	name = "kernel_process_fork"
	pid = $arg1
	parent_pid = $arg1
	child_pid = $arg2
	child_tgid = $arg3
}


/*-----------------------------------------------*
 * marker.scheduler.* - scheduler related markers
 *-----------------------------------------------*/

probe __marker.scheduler.wakeup = kernel.mark("kernel_sched_wakeup")
{
	name = "kernel_sched_wakeup"
}
probe __marker.scheduler.wakeup_new = kernel.mark("kernel_sched_wakeup_new")
{
	name = "kernel_sched_wakeup_new"
}

/*
 * probe marker.scheduler.wakeup
 *
 *  Fires a process wakeup.
 *
 * Context:
 *  A process wakeup.
 *
 * Arguments:
 *  pid - wakeup process pid.
 *  state - process task state.
 *  cpu_id - the id of cpu where the process wakeup.
 */
probe marker.scheduler.wakeup
	= __marker.scheduler.wakeup,
	  __marker.scheduler.wakeup_new
{
	pid = $arg1
	state = $arg2
	cpu_id = $arg3
}

/*
 * probe marker.scheduler.switch
 *
 *  Fires scheduler switches tasks.
 *
 * Context:
 *  A process is switching to another process.
 *
 * Arguments:
 *  prev_pid - the pid of the process from which next task is switched.
 *  next_pid - the pid of the process to which previous task switches.
 *  prev_state - task state of previous process
 *  prev_prio - task priority of previous process
 *  next_prio - task priority of next process
 */
probe marker.scheduler.switch
	= kernel.mark("kernel_sched_switch")
{
	name = "kernel_sched_switch"
	prev_pid = $arg1
	next_pid = $arg2
	prev_state = $arg3
	prev_prio = $arg4
	next_prio = $arg5
}

[-- Attachment #3: sample.stp --]
[-- Type: text/plain, Size: 829 bytes --]

#!/usr/bin/stap
#
# sample script for kernel marker
#

#------------------------------------------------------------
# Counter
#------------------------------------------------------------

global num

#------------------------------------------------------------
# Probes
#------------------------------------------------------------

#
# Interrupts
#
probe marker.irq.* {
	num[name] <<< 1
}

#
# Scheduler
#
probe marker.scheduler.* {
	num[name] <<< 1
}

#
# Process
#

probe marker.process.* {
	num[name] <<< 1
}

#------------------------------------------------------------
# begin/end
#------------------------------------------------------------

probe begin {
	printf("[STP] --- start ---\n")
}

probe end {
	printf("[STP] --- end ---\n")

	foreach (n+ in num) {
		printf("%s is hit %d times\n", n, @count(num[n]))
	}
}

Re: [RFC] sample test script and tapset for markers

[Frank Ch. Eigler]

Masami Hiramatsu <mhiramat@redhat.com> writes:

> [...]
>  * Each marker has "name" variable which stores marker name.

It is unfortunate to have to hand-code this.  It would be easy for the
translator to provide the marker name as a $context variable.


- FChE

Re: [RFC] sample test script and tapset for markers

[Masami Hiramatsu]

Frank Ch. Eigler wrote:
> Masami Hiramatsu <mhiramat@redhat.com> writes:
> 
>> [...]
>>  * Each marker has "name" variable which stores marker name.
> 
> It is unfortunate to have to hand-code this.  It would be easy for the
> translator to provide the marker name as a $context variable.

Sure, that is a good idea.
BTW, How about $name and $format instead of $context?

Thank you,

> 
> 
> - FChE

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers)

[Frank Ch. Eigler]

Hi -

On Fri, Sep 05, 2008 at 11:32:24AM -0400, Masami Hiramatsu wrote:
> Here is a patch which add $name variable access from marker probe.
> I added _stp_mark_context structure which contains .name and .format
> strings for passing both of them to marker handler. [...]

That seems unnecessary.  You could have added the name/format as
separate fields, and have the embedded-C functions use
CONTEXT->marker_name / marker_format.  (Those fields then better be
cleared to zero for other types of probes, and the embedded-C code
better check for that.)


- FChE

[PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers)

[Masami Hiramatsu]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

Hi,

Frank Ch. Eigler wrote:
> Masami Hiramatsu <mhiramat@redhat.com> writes:
> 
>> [...]
>>  * Each marker has "name" variable which stores marker name.
> 
> It is unfortunate to have to hand-code this.  It would be easy for the
> translator to provide the marker name as a $context variable.

Here is a patch which add $name variable access from marker probe.
I added _stp_mark_context structure which contains .name and .format
strings for passing both of them to marker handler. Now you can get
both of $format and $name from marker probes.

Thank you,


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


[-- Attachment #2: stap-marker-name-variable.patch --]
[-- Type: text/plain, Size: 7717 bytes --]

---
 runtime/mark.h    |   16 ++++++++++++++
 tapset/marker.stp |   25 ++++++++++++++++++++++
 tapsets.cxx       |   60 ++++++++++++++++++++++--------------------------------
 3 files changed, 66 insertions(+), 35 deletions(-)

Index: systemtap/tapsets.cxx
===================================================================
--- systemtap.orig/tapsets.cxx	2008-09-04 11:29:52.000000000 -0400
+++ systemtap/tapsets.cxx	2008-09-04 11:29:57.000000000 -0400
@@ -7753,7 +7753,7 @@
 
   void visit_target_symbol (target_symbol* e);
   void visit_target_symbol_arg (target_symbol* e);
-  void visit_target_symbol_format (target_symbol* e);
+  void visit_target_symbol_context (target_symbol* e);
 };
 
 
@@ -7860,48 +7860,37 @@
 
 
 void
-mark_var_expanding_copy_visitor::visit_target_symbol_format (target_symbol* e)
+mark_var_expanding_copy_visitor::visit_target_symbol_context (target_symbol* e)
 {
-  static bool function_synthesized = false;
+  string sname = e->base_name;
 
   if (is_active_lvalue (e))
-    throw semantic_error("write to marker format not permitted", e->tok);
+    throw semantic_error("write to marker '" + sname + "' not permitted", e->tok);
 
   if (e->components.size() > 0)
     {
       switch (e->components[0].first)
 	{
 	case target_symbol::comp_literal_array_index:
-	  throw semantic_error("marker format may not be used as array",
+	  throw semantic_error("marker '" + sname + "' may not be used as array",
 			       e->tok);
 	  break;
 	case target_symbol::comp_struct_member:
-	  throw semantic_error("marker format may not be used as a structure",
+	  throw semantic_error("marker '" + sname + "' may not be used as a structure",
 			       e->tok);
 	  break;
 	default:
-	  throw semantic_error ("invalid marker format use", e->tok);
+	  throw semantic_error ("invalid marker '" + sname + "' use", e->tok);
 	  break;
 	}
     }
 
-  string fname = string("_mark_format_get");
-
-  // Synthesize a function (if not already synthesized).
-  if (! function_synthesized)
-    {
-      function_synthesized = true;
-      functiondecl *fdecl = new functiondecl;
-      fdecl->tok = e->tok;
-      embeddedcode *ec = new embeddedcode;
-      ec->tok = e->tok;
-
-      ec->code = string("strlcpy (THIS->__retvalue, CONTEXT->data, MAXSTRINGLEN); /* pure */");
-      fdecl->name = fname;
-      fdecl->body = ec;
-      fdecl->type = pe_string;
-      sess.functions.push_back(fdecl);
-    }
+  string fname;
+  if (e->base_name == "$format") {
+    fname = string("_mark_format_get");
+  } else {
+    fname = string("_mark_name_get");
+  }
 
   // Synthesize a functioncall.
   functioncall* n = new functioncall;
@@ -7918,10 +7907,10 @@
 
   if (e->base_name.substr(0,4) == "$arg")
     visit_target_symbol_arg (e);
-  else if (e->base_name == "$format")
-    visit_target_symbol_format (e);
+  else if (e->base_name == "$format" || e->base_name == "$name")
+    visit_target_symbol_context (e);
   else
-    throw semantic_error ("invalid target symbol for marker, $argN or $format expected",
+    throw semantic_error ("invalid target symbol for marker, $argN, $name or $format expected",
 			  e->tok);
 }
 
@@ -8108,7 +8097,8 @@
       ec->code = string("#if ! defined(CONFIG_MARKERS)\n")
 	+ string("#error \"Need CONFIG_MARKERS!\"\n")
 	+ string("#endif\n")
-	+ string("#include <linux/marker.h>\n");
+	+ string("#include <linux/marker.h>\n")
+	+ string("#include \"mark.h\"\n");
 
       s.embeds.push_back(ec);
     }
@@ -8191,8 +8181,7 @@
   s.op->newline() << "/* ---- marker probes ---- */";
 
   s.op->newline() << "struct stap_marker_probe {";
-  s.op->newline(1) << "const char * const name;";
-  s.op->newline() << "const char * const format;";
+  s.op->newline(1) << "struct _stp_mark_context mark;";
   s.op->newline() << "const char * const pp;";
   s.op->newline() << "void (* const ph) (struct context *);";
 
@@ -8201,10 +8190,11 @@
   for (unsigned i=0; i < probes.size(); i++)
     {
       s.op->newline () << "{";
+      s.op->line() << " .mark = {";
       s.op->line() << " .name=" << lex_cast_qstring(probes[i]->probe_name)
 		   << ",";
       s.op->line() << " .format=" << lex_cast_qstring(probes[i]->probe_format)
-		   << ",";
+		   << " },";
       s.op->line() << " .pp=" << lex_cast_qstring (*probes[i]->sole_location())
 		   << ",";
       s.op->line() << " .ph=&" << probes[i]->name;
@@ -8220,7 +8210,7 @@
   s.op->newline(1) << "struct stap_marker_probe *smp = (struct stap_marker_probe *)probe_data;";
   common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING");
   s.op->newline() << "c->probe_point = smp->pp;";
-  s.op->newline() << "c->data = (char *)smp->format;";
+  s.op->newline() << "c->data = (void *)&smp->mark;";
 
   s.op->newline() << "c->mark_va_list = args;";
   s.op->newline() << "(*smp->ph) (c);";
@@ -8244,11 +8234,11 @@
   s.op->newline() << "for (i=0; i<" << probes.size() << "; i++) {";
   s.op->newline(1) << "struct stap_marker_probe *smp = &stap_marker_probes[i];";
   s.op->newline() << "probe_point = smp->pp;";
-  s.op->newline() << "rc = marker_probe_register(smp->name, smp->format, enter_marker_probe, smp);";
+  s.op->newline() << "rc = marker_probe_register(smp->mark.name, smp->mark.format, enter_marker_probe, smp);";
   s.op->newline() << "if (rc) {";
   s.op->newline(1) << "for (j=i-1; j>=0; j--) {"; // partial rollback
   s.op->newline(1) << "struct stap_marker_probe *smp2 = &stap_marker_probes[j];";
-  s.op->newline() << "marker_probe_unregister(smp2->name, enter_marker_probe, smp2);";
+  s.op->newline() << "marker_probe_unregister(smp2->mark.name, enter_marker_probe, smp2);";
   s.op->newline(-1) << "}";
   s.op->newline() << "break;"; // don't attempt to register any more probes
   s.op->newline(-1) << "}";
@@ -8265,7 +8255,7 @@
   s.op->newline() << "/* deregister marker probes */";
   s.op->newline() << "for (i=0; i<" << probes.size() << "; i++) {";
   s.op->newline(1) << "struct stap_marker_probe *smp = &stap_marker_probes[i];";
-  s.op->newline() << "marker_probe_unregister(smp->name, enter_marker_probe, smp);";
+  s.op->newline() << "marker_probe_unregister(smp->mark.name, enter_marker_probe, smp);";
   s.op->newline(-1) << "}"; // for loop
 }
 
Index: systemtap/runtime/mark.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ systemtap/runtime/mark.h	2008-09-04 11:29:57.000000000 -0400
@@ -0,0 +1,16 @@
+/* marker support header
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+#ifndef _STP_MARK_H_
+#define _STP_MARK_H_
+
+struct _stp_mark_context {
+	const char * const name;
+	const char * const format;
+};
+
+#endif
Index: systemtap/tapset/marker.stp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ systemtap/tapset/marker.stp	2008-09-04 11:29:57.000000000 -0400
@@ -0,0 +1,25 @@
+//
+// kernel marker tapset
+//
+// This file is part of systemtap, and is free software.  You can
+// redistribute it and/or modify it under the terms of the GNU General
+// Public License (GPL); either version 2, or (at your option) any
+// later version.
+
+/* marker-only context accessors */
+
+%{
+#include "mark.h"
+%}
+
+function _mark_name_get:string () %{
+	strlcpy (THIS->__retvalue,
+		 ((struct _stp_mark_context *)(CONTEXT->data))->name,
+		 MAXSTRINGLEN); /* pure */
+%}
+
+function _mark_format_get:string () %{
+	strlcpy (THIS->__retvalue,
+		 ((struct _stp_mark_context *)(CONTEXT->data))->format,
+		 MAXSTRINGLEN); /* pure */
+%}

[PATCH 2/2] marker probe: add $name testcases

[Masami Hiramatsu]

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

Hi,

Masami Hiramatsu wrote:
> Here is a patch which add $name variable access from marker probe.
> I added _stp_mark_context structure which contains .name and .format
> strings for passing both of them to marker handler. Now you can get
> both of $format and $name from marker probes.

Here is a patch of the testcases for $name support.


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


[-- Attachment #2: testsuite-marker-name-testing.patch --]
[-- Type: text/plain, Size: 1997 bytes --]

---
 testsuite/systemtap.base/marker.exp |   45 ++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Index: systemtap/testsuite/systemtap.base/marker.exp
===================================================================
--- systemtap.orig/testsuite/systemtap.base/marker.exp	2008-09-02 05:35:55.000000000 -0400
+++ systemtap/testsuite/systemtap.base/marker.exp	2008-09-03 17:46:51.000000000 -0400
@@ -230,3 +230,48 @@
 		    [lindex $kernel_marker_names 0] "foo"]
     stap_compile $TEST_NAME 0 $script
 }
+
+set TEST_NAME "K_MARKER18"
+if {$kernel_markers_found == 0} {
+    untested "$TEST_NAME : no kernel markers present"
+} else {
+    # Try compiling a script that prints the name string of a
+    # marker.
+    set script [format $kernel_script_arg \
+		    [lindex $kernel_marker_names 0] {\$name}]
+    stap_compile $TEST_NAME 1 $script
+}
+
+set TEST_NAME "K_MARKER19"
+if {$kernel_markers_found == 0} {
+    untested "$TEST_NAME : no kernel markers present"
+} else {
+    # Try compiling a script that writes to a marker name string
+    # (which isn't allowed).
+    set script [format $kernel_script_arg2 \
+		    [lindex $kernel_marker_names 0] {\$name}]
+    stap_compile $TEST_NAME 0 $script
+}
+
+set TEST_NAME "K_MARKER20"
+if {$kernel_markers_found == 0} {
+    untested "$TEST_NAME : no kernel markers present"
+} else {
+    # Try compiling a script that treats the marker name string as a
+    # structure (which isn't allowed).
+    set script [format $kernel_script_arg \
+		    [lindex $kernel_marker_names 0] {\$name->foo}]
+    stap_compile $TEST_NAME 0 $script
+}
+
+set TEST_NAME "K_MARKER21"
+if {$kernel_markers_found == 0} {
+    untested "$TEST_NAME : no kernel markers present"
+} else {
+    # Try compiling a script that treats the marker name string like
+    # an array (which isn't allowed).
+    set script [format $kernel_script_arg \
+		    [lindex $kernel_marker_names 0] {\$name\[0\]}]
+    stap_compile $TEST_NAME 0 $script
+}
+

Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers)

[Masami Hiramatsu]

Frank Ch. Eigler wrote:
> Hi -
> 
> On Fri, Sep 05, 2008 at 11:32:24AM -0400, Masami Hiramatsu wrote:
>> Here is a patch which add $name variable access from marker probe.
>> I added _stp_mark_context structure which contains .name and .format
>> strings for passing both of them to marker handler. [...]
> 
> That seems unnecessary.  You could have added the name/format as
> separate fields, and have the embedded-C functions use
> CONTEXT->marker_name / marker_format.  (Those fields then better be
> cleared to zero for other types of probes, and the embedded-C code
> better check for that.)

I think adding those fields increases the overhead of cleanup/initialize
and memory usage. So I decided to introduce new structure for that.
Could you tell me what the advantage of those separated fields is?

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers)

[Frank Ch. Eigler]

Hi -

On Fri, Sep 05, 2008 at 12:04:43PM -0400, Masami Hiramatsu wrote:
> [...]
> I think adding those fields increases the overhead of cleanup/initialize
> and memory usage. So I decided to introduce new structure for that.
> Could you tell me what the advantage of those separated fields is?

It's not a big difference, just that we tend to keep such data in the
context.  I mostly don't like the new struct being passed by void-*
and then suffering unprotected dereference in the embedded-c
functions.

(Extra memory consumption is negligible - one more word per CPU.
Initialization time/space could be further reduced if
per-probe-point-type data were gathered into unions.)

- FChE

Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers)

[Masami Hiramatsu]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

Hi Frank,

Frank Ch. Eigler wrote:
> Hi -
> 
> On Fri, Sep 05, 2008 at 12:04:43PM -0400, Masami Hiramatsu wrote:
>> [...]
>> I think adding those fields increases the overhead of cleanup/initialize
>> and memory usage. So I decided to introduce new structure for that.
>> Could you tell me what the advantage of those separated fields is?
> 
> It's not a big difference, just that we tend to keep such data in the
> context.  I mostly don't like the new struct being passed by void-*
> and then suffering unprotected dereference in the embedded-c
> functions.
> 
> (Extra memory consumption is negligible - one more word per CPU.
> Initialization time/space could be further reduced if
> per-probe-point-type data were gathered into unions.)

OK, so here is the updated patch, which also includes stapprobes.5 update :-)

Thank you,


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


[-- Attachment #2: stap-marker-name-variable.patch --]
[-- Type: text/plain, Size: 5977 bytes --]

---
Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>

 stapprobes.5.in   |    2 ++
 tapset/marker.stp |   22 ++++++++++++++++++++++
 tapsets.cxx       |   49 ++++++++++++++++++++-----------------------------
 translate.cxx     |    2 ++
 4 files changed, 46 insertions(+), 29 deletions(-)

Index: systemtap/tapsets.cxx
===================================================================
--- systemtap.orig/tapsets.cxx
+++ systemtap/tapsets.cxx
@@ -221,6 +221,8 @@ common_probe_entryfn_prologue (translato
   // reset unwound address cache
   o->newline() << "c->pi = 0;";
   o->newline() << "c->regparm = 0;";
+  o->newline() << "c->marker_name = NULL;";
+  o->newline() << "c->marker_format = NULL;";
   o->newline() << "c->probe_point = 0;";
   if (! interruptible)
     o->newline() << "c->actionremaining = MAXACTION;";
@@ -7753,7 +7755,7 @@ struct mark_var_expanding_copy_visitor: 
 
   void visit_target_symbol (target_symbol* e);
   void visit_target_symbol_arg (target_symbol* e);
-  void visit_target_symbol_format (target_symbol* e);
+  void visit_target_symbol_context (target_symbol* e);
 };
 
 
@@ -7860,48 +7862,37 @@ mark_var_expanding_copy_visitor::visit_t
 
 
 void
-mark_var_expanding_copy_visitor::visit_target_symbol_format (target_symbol* e)
+mark_var_expanding_copy_visitor::visit_target_symbol_context (target_symbol* e)
 {
-  static bool function_synthesized = false;
+  string sname = e->base_name;
 
   if (is_active_lvalue (e))
-    throw semantic_error("write to marker format not permitted", e->tok);
+    throw semantic_error("write to marker '" + sname + "' not permitted", e->tok);
 
   if (e->components.size() > 0)
     {
       switch (e->components[0].first)
 	{
 	case target_symbol::comp_literal_array_index:
-	  throw semantic_error("marker format may not be used as array",
+	  throw semantic_error("marker '" + sname + "' may not be used as array",
 			       e->tok);
 	  break;
 	case target_symbol::comp_struct_member:
-	  throw semantic_error("marker format may not be used as a structure",
+	  throw semantic_error("marker '" + sname + "' may not be used as a structure",
 			       e->tok);
 	  break;
 	default:
-	  throw semantic_error ("invalid marker format use", e->tok);
+	  throw semantic_error ("invalid marker '" + sname + "' use", e->tok);
 	  break;
 	}
     }
 
-  string fname = string("_mark_format_get");
-
-  // Synthesize a function (if not already synthesized).
-  if (! function_synthesized)
-    {
-      function_synthesized = true;
-      functiondecl *fdecl = new functiondecl;
-      fdecl->tok = e->tok;
-      embeddedcode *ec = new embeddedcode;
-      ec->tok = e->tok;
-
-      ec->code = string("strlcpy (THIS->__retvalue, CONTEXT->data, MAXSTRINGLEN); /* pure */");
-      fdecl->name = fname;
-      fdecl->body = ec;
-      fdecl->type = pe_string;
-      sess.functions.push_back(fdecl);
-    }
+  string fname;
+  if (e->base_name == "$format") {
+    fname = string("_mark_format_get");
+  } else {
+    fname = string("_mark_name_get");
+  }
 
   // Synthesize a functioncall.
   functioncall* n = new functioncall;
@@ -7918,10 +7909,10 @@ mark_var_expanding_copy_visitor::visit_t
 
   if (e->base_name.substr(0,4) == "$arg")
     visit_target_symbol_arg (e);
-  else if (e->base_name == "$format")
-    visit_target_symbol_format (e);
+  else if (e->base_name == "$format" || e->base_name == "$name")
+    visit_target_symbol_context (e);
   else
-    throw semantic_error ("invalid target symbol for marker, $argN or $format expected",
+    throw semantic_error ("invalid target symbol for marker, $argN, $name or $format expected",
 			  e->tok);
 }
 
@@ -8220,8 +8211,8 @@ mark_derived_probe_group::emit_module_de
   s.op->newline(1) << "struct stap_marker_probe *smp = (struct stap_marker_probe *)probe_data;";
   common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING");
   s.op->newline() << "c->probe_point = smp->pp;";
-  s.op->newline() << "c->data = (char *)smp->format;";
-
+  s.op->newline() << "c->marker_name = smp->name;";
+  s.op->newline() << "c->marker_format = smp->format;";
   s.op->newline() << "c->mark_va_list = args;";
   s.op->newline() << "(*smp->ph) (c);";
   s.op->newline() << "c->mark_va_list = NULL;";
Index: systemtap/tapset/marker.stp
===================================================================
--- /dev/null
+++ systemtap/tapset/marker.stp
@@ -0,0 +1,22 @@
+//
+// kernel marker tapset
+//
+// This file is part of systemtap, and is free software.  You can
+// redistribute it and/or modify it under the terms of the GNU General
+// Public License (GPL); either version 2, or (at your option) any
+// later version.
+
+/* marker-only context accessors */
+
+
+function _mark_name_get:string () %{
+	strlcpy (THIS->__retvalue,
+		 (CONTEXT->marker_name)?CONTEXT->marker_name:"",
+		 MAXSTRINGLEN); /* pure */
+%}
+
+function _mark_format_get:string () %{
+	strlcpy (THIS->__retvalue,
+		 (CONTEXT->marker_format)?CONTEXT->marker_format:"",
+		 MAXSTRINGLEN); /* pure */
+%}
Index: systemtap/translate.cxx
===================================================================
--- systemtap.orig/translate.cxx
+++ systemtap/translate.cxx
@@ -881,6 +881,8 @@ c_unparser::emit_common_header ()
   o->newline() << "struct kretprobe_instance *pi;";
   o->newline() << "int regparm;";
   o->newline() << "va_list *mark_va_list;";
+  o->newline() << "const char * marker_name;";
+  o->newline() << "const char * marker_format;";
   o->newline() << "void *data;";
   o->newline() << "#ifdef STP_TIMING";
   o->newline() << "Stat *statp;";
Index: systemtap/stapprobes.5.in
===================================================================
--- systemtap.orig/stapprobes.5.in
+++ systemtap/stapprobes.5.in
@@ -520,6 +520,8 @@ and string parameters are passed in a ty
 
 The marker format string associated with a marker is available in
 .BR $format .
+And also the marker name string is avalable in
+.BR $name .
 
 .SS PERFORMANCE MONITORING HARDWARE