This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
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