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

[Masami Hiramatsu <mhiramat at redhat dot com>]

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

---
 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 */
+%}