* [Systemtap][RFC] sample test script and tapset for markers @ 2008-08-26 16:13 Masami Hiramatsu 2008-08-26 19:01 ` [RFC] " Frank Ch. Eigler 0 siblings, 1 reply; 9+ messages in thread From: Masami Hiramatsu @ 2008-08-26 16:13 UTC (permalink / raw) To: systemtap-ml; +Cc: ltt-dev, Hideo AOKI, Takahiro Yasui [-- 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])) } } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] sample test script and tapset for markers 2008-08-26 16:13 [Systemtap][RFC] sample test script and tapset for markers Masami Hiramatsu @ 2008-08-26 19:01 ` Frank Ch. Eigler 2008-08-26 20:19 ` Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Masami Hiramatsu 0 siblings, 2 replies; 9+ messages in thread From: Frank Ch. Eigler @ 2008-08-26 19:01 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: systemtap-ml, ltt-dev, Hideo AOKI, Takahiro Yasui 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] sample test script and tapset for markers 2008-08-26 19:01 ` [RFC] " Frank Ch. Eigler @ 2008-08-26 20:19 ` Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Masami Hiramatsu 1 sibling, 0 replies; 9+ messages in thread From: Masami Hiramatsu @ 2008-08-26 20:19 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: systemtap-ml, ltt-dev, Hideo AOKI, Takahiro Yasui 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) 2008-08-26 19:01 ` [RFC] " Frank Ch. Eigler 2008-08-26 20:19 ` Masami Hiramatsu @ 2008-09-05 16:19 ` Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 2/2] marker probe: add $name testcases Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Frank Ch. Eigler 1 sibling, 2 replies; 9+ messages in thread From: Masami Hiramatsu @ 2008-09-05 16:19 UTC (permalink / raw) To: Frank Ch. Eigler, systemtap-ml; +Cc: Hideo AOKI, Takahiro Yasui [-- 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 */ +%} ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] marker probe: add $name testcases 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Masami Hiramatsu @ 2008-09-05 16:19 ` Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Frank Ch. Eigler 1 sibling, 0 replies; 9+ messages in thread From: Masami Hiramatsu @ 2008-09-05 16:19 UTC (permalink / raw) To: Frank Ch. Eigler, systemtap-ml; +Cc: Hideo AOKI, Takahiro Yasui [-- 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 +} + ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 2/2] marker probe: add $name testcases Masami Hiramatsu @ 2008-09-05 16:19 ` Frank Ch. Eigler 2008-09-05 16:47 ` Masami Hiramatsu 1 sibling, 1 reply; 9+ messages in thread From: Frank Ch. Eigler @ 2008-09-05 16:19 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: systemtap-ml, Hideo AOKI, Takahiro Yasui 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Frank Ch. Eigler @ 2008-09-05 16:47 ` Masami Hiramatsu 2008-09-05 16:47 ` Frank Ch. Eigler 0 siblings, 1 reply; 9+ messages in thread From: Masami Hiramatsu @ 2008-09-05 16:47 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: systemtap-ml, Hideo AOKI, Takahiro Yasui 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) 2008-09-05 16:47 ` Masami Hiramatsu @ 2008-09-05 16:47 ` Frank Ch. Eigler 2008-09-05 23:28 ` Masami Hiramatsu 0 siblings, 1 reply; 9+ messages in thread From: Frank Ch. Eigler @ 2008-09-05 16:47 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: systemtap-ml, Hideo AOKI, Takahiro Yasui 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) 2008-09-05 16:47 ` Frank Ch. Eigler @ 2008-09-05 23:28 ` Masami Hiramatsu 0 siblings, 0 replies; 9+ messages in thread From: Masami Hiramatsu @ 2008-09-05 23:28 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: systemtap-ml, Hideo AOKI, Takahiro Yasui [-- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2008-09-05 23:28 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-08-26 16:13 [Systemtap][RFC] sample test script and tapset for markers Masami Hiramatsu 2008-08-26 19:01 ` [RFC] " Frank Ch. Eigler 2008-08-26 20:19 ` Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 2/2] marker probe: add $name testcases Masami Hiramatsu 2008-09-05 16:19 ` [PATCH 1/2] marker probe: $name support (Re: [RFC] sample test script and tapset for markers) Frank Ch. Eigler 2008-09-05 16:47 ` Masami Hiramatsu 2008-09-05 16:47 ` Frank Ch. Eigler 2008-09-05 23:28 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).