This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]