* [PATCH v2 0/3] initscript: add support for uprobes scripts @ 2014-08-11 13:25 Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 2/3] initscript: copy uprobes.ko to cache directory Stefan Hajnoczi ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Stefan Hajnoczi @ 2014-08-11 13:25 UTC (permalink / raw) To: systemtap Cc: Frank Ch. Eigler, Josh Stone, Jonathan Lebon, Masami Hiramatsu, Stefan Hajnoczi v2: * Add Patch 1 with stap --save-uprobes option [Josh] * Switch from stap -k to stap --save-uprobes [Josh] The initscript currently fails for user-space probing scripts on systems where uprobes.ko is built from source by stap(1). This is because the initscript uses a two-phase "compile and then run" approach: The uprobes.ko module is generated during the compile phase but not copied into the cache directory where modules are placed for the run phase. The staprun(8) command fails because the script module cannot be loaded without uprobes.ko. These patches address the issue by copying uprobes.ko into the cache directory. If a script specifies the -u option in its initscript configuration file, staprun(8) will receive the path to uprobes.ko. There is no change in behavior on systems that do not build uprobes.ko. This has been tested on RHEL6 (builds uprobes.ko) and RHEL7 (systemtap and does not build uprobes.ko). Stefan Hajnoczi (3): stap: add --save-uprobes initscript: copy uprobes.ko to cache directory initscript: allow scripts to load uprobes buildrun.cxx | 4 +++- cmdline.cxx | 1 + cmdline.h | 1 + initscript/systemtap.in | 15 +++++++++++++++ main.cxx | 8 ++++++++ session.cxx | 10 ++++++++++ session.h | 2 ++ 7 files changed, 40 insertions(+), 1 deletion(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] initscript: copy uprobes.ko to cache directory 2014-08-11 13:25 [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi @ 2014-08-11 13:25 ` Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 3/3] initscript: allow scripts to load uprobes Stefan Hajnoczi ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Stefan Hajnoczi @ 2014-08-11 13:25 UTC (permalink / raw) To: systemtap Cc: Frank Ch. Eigler, Josh Stone, Jonathan Lebon, Masami Hiramatsu, Stefan Hajnoczi If the uprobes.ko module was built then it will be needed at staprun time. Copy the module into the cache directory alongside compiled scripts. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- initscript/systemtap.in | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/initscript/systemtap.in b/initscript/systemtap.in index 2d770e4..ebd28e4 100755 --- a/initscript/systemtap.in +++ b/initscript/systemtap.in @@ -403,6 +403,8 @@ get_compile_opts () { # opts skip=1 ;; -h|-V|-k|-F) ;; + -u) + echo -n "--save-uprobes " ;; *) echo -n $o" " ;; esac @@ -492,6 +494,13 @@ compile_script () { # script checkcache logex $STAP -m "$1" -p4 -r $KRELEASE $opts "$f" ret=$? if [ $ret -eq 0 ]; then + if [ -f "uprobes/uprobes.ko" ]; then + logex mkdir -p "$CACHE_PATH/uprobes" + logex mv "uprobes/uprobes.ko" "$CACHE_PATH/uprobes/" + ret=$? + fi + fi + if [ $ret -eq 0 ]; then $UNAME -a > "$1.opts" echo $opts >> "$1.opts" logex mv "$1.ko" "$1.opts" "$CACHE_PATH/" -- 1.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] initscript: allow scripts to load uprobes 2014-08-11 13:25 [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 2/3] initscript: copy uprobes.ko to cache directory Stefan Hajnoczi @ 2014-08-11 13:25 ` Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 1/3] stap: add --save-uprobes Stefan Hajnoczi 2014-09-01 12:15 ` [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi 3 siblings, 0 replies; 12+ messages in thread From: Stefan Hajnoczi @ 2014-08-11 13:25 UTC (permalink / raw) To: systemtap Cc: Frank Ch. Eigler, Josh Stone, Jonathan Lebon, Masami Hiramatsu, Stefan Hajnoczi Scripts that rely on userspace probing must include the -u option in their conf file: # cat /etc/systemtap/conf.d/example.conf example_OPT=-u The uprobe kernel module will be loaded when the script runs. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- initscript/systemtap.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/initscript/systemtap.in b/initscript/systemtap.in index ebd28e4..d61d480 100755 --- a/initscript/systemtap.in +++ b/initscript/systemtap.in @@ -422,6 +422,12 @@ get_run_opts () { # normalized_opts show=0 for o in $opts; do case $o in + -u) + # Use cached uprobes module, if available + if [ -f "$CACHE_PATH/uprobes/uprobes.ko" ]; then + echo -n "-u$CACHE_PATH/uprobes/uprobes.ko " + fi + ;; -c|-x|-s|-o|-S) [ $o == '-s' ] && o='-b' [ $o == '-o' ] && mode='-D' -- 1.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/3] stap: add --save-uprobes 2014-08-11 13:25 [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 2/3] initscript: copy uprobes.ko to cache directory Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 3/3] initscript: allow scripts to load uprobes Stefan Hajnoczi @ 2014-08-11 13:25 ` Stefan Hajnoczi 2014-09-01 12:15 ` [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi 3 siblings, 0 replies; 12+ messages in thread From: Stefan Hajnoczi @ 2014-08-11 13:25 UTC (permalink / raw) To: systemtap Cc: Frank Ch. Eigler, Josh Stone, Jonathan Lebon, Masami Hiramatsu, Stefan Hajnoczi The stap -m <name> option saves the script module. For scripts that rely on uprobes it may also be necessary to get the uprobes module (if it was built by stap). The new stap --save-uprobes option saves uprobes/uprobes.ko into the current directory. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- buildrun.cxx | 4 +++- cmdline.cxx | 1 + cmdline.h | 1 + main.cxx | 8 ++++++++ session.cxx | 10 ++++++++++ session.h | 2 ++ 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/buildrun.cxx b/buildrun.cxx index 6f9b78b..9339b49 100644 --- a/buildrun.cxx +++ b/buildrun.cxx @@ -601,8 +601,10 @@ make_uprobes (systemtap_session& s) clog << _("uprobes rebuild exit code: ") << rc << endl; if (rc) s.set_try_server (); - else + else { s.uprobes_path = dir + "/uprobes.ko"; + s.built_uprobes = true; + } return rc; } diff --git a/cmdline.cxx b/cmdline.cxx index 090659d..d50ca1a 100644 --- a/cmdline.cxx +++ b/cmdline.cxx @@ -61,5 +61,6 @@ struct option stap_long_options[] = { { "benchmark-sdt-threads", required_argument, NULL, LONG_OPT_BENCHMARK_SDT_THREADS }, { "color", optional_argument, NULL, LONG_OPT_COLOR_ERRS }, { "colour", optional_argument, NULL, LONG_OPT_COLOR_ERRS }, + { "save-uprobes", no_argument, NULL, LONG_OPT_SAVE_UPROBES }, { NULL, 0, NULL, 0 } }; diff --git a/cmdline.h b/cmdline.h index fa19d65..297096f 100644 --- a/cmdline.h +++ b/cmdline.h @@ -59,6 +59,7 @@ enum { LONG_OPT_BENCHMARK_SDT_LOOPS, LONG_OPT_BENCHMARK_SDT_THREADS, LONG_OPT_COLOR_ERRS, + LONG_OPT_SAVE_UPROBES, }; // NB: when adding new options, consider very carefully whether they diff --git a/main.cxx b/main.cxx index 5150a30..d8b1d98 100644 --- a/main.cxx +++ b/main.cxx @@ -994,6 +994,14 @@ passes_0_4 (systemtap_session &s) string module_dest_path = s.module_filename(); copy_file(module_src_path, module_dest_path, s.verbose > 1); } + + // Copy uprobes module to the current directory. + if (s.save_uprobes && s.built_uprobes && !pending_interrupts) + { + rc = create_dir("uprobes"); + if (! rc) + copy_file(s.uprobes_path, "uprobes/uprobes.ko", s.verbose > 1); + } } PROBE1(stap, pass4__end, &s); diff --git a/session.cxx b/session.cxx index 56198ef..201ae6c 100644 --- a/session.cxx +++ b/session.cxx @@ -134,6 +134,7 @@ systemtap_session::systemtap_session (): output_file = ""; // -o FILE tmpdir_opt_set = false; save_module = false; + save_uprobes = false; modname_given = false; keep_tmpdir = false; cmd = ""; @@ -145,6 +146,7 @@ systemtap_session::systemtap_session (): need_uprobes = false; need_unwind = false; need_symbols = false; + built_uprobes = false; uprobes_path = ""; load_only = false; skip_badvars = false; @@ -315,6 +317,7 @@ systemtap_session::systemtap_session (const systemtap_session& other, output_file = other.output_file; // XXX how should multiple remotes work? tmpdir_opt_set = false; save_module = other.save_module; + save_uprobes = other.save_uprobes; modname_given = other.modname_given; keep_tmpdir = other.keep_tmpdir; cmd = other.cmd; @@ -326,6 +329,7 @@ systemtap_session::systemtap_session (const systemtap_session& other, need_uprobes = false; need_unwind = false; need_symbols = false; + built_uprobes = false; uprobes_path = ""; load_only = other.load_only; skip_badvars = other.skip_badvars; @@ -637,6 +641,8 @@ systemtap_session::usage (int exitcode) " relative to the sysroot.\n" " --suppress-time-limits\n" " disable -DSTP_OVERLOAD, -DMAXACTION, and -DMAXTRYACTION limits\n" + " --save-uprobes\n" + " save uprobes.ko to current directory if it is built from source\n" , compatible.c_str()) << endl ; @@ -1390,6 +1396,10 @@ systemtap_session::parse_cmdline (int argc, char * const argv []) strcmp(getenv("TERM") ?: "notdumb", "dumb")); break; + case LONG_OPT_SAVE_UPROBES: + save_uprobes = true; + break; + case '?': // Invalid/unrecognized option given or argument required, but // not given. In both cases getopt_long() will have printed the diff --git a/session.h b/session.h index e95a26d..70f7909 100644 --- a/session.h +++ b/session.h @@ -197,6 +197,7 @@ public: unsigned verbose; bool timing; bool save_module; + bool save_uprobes; bool modname_given; bool keep_tmpdir; bool guru_mode; @@ -210,6 +211,7 @@ public: bool need_uprobes; bool need_unwind; bool need_symbols; + bool built_uprobes; std::string uprobes_path; std::string uprobes_hash; bool load_only; // flight recorder mode -- 1.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] initscript: add support for uprobes scripts 2014-08-11 13:25 [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi ` (2 preceding siblings ...) 2014-08-11 13:25 ` [PATCH v2 1/3] stap: add --save-uprobes Stefan Hajnoczi @ 2014-09-01 12:15 ` Stefan Hajnoczi 2014-09-02 15:03 ` Jonathan Lebon 3 siblings, 1 reply; 12+ messages in thread From: Stefan Hajnoczi @ 2014-09-01 12:15 UTC (permalink / raw) To: systemtap; +Cc: fche [-- Attachment #1: Type: text/plain, Size: 6 bytes --] Ping? [-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] initscript: add support for uprobes scripts 2014-09-01 12:15 ` [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi @ 2014-09-02 15:03 ` Jonathan Lebon 2014-09-03 12:46 ` Stefan Hajnoczi 0 siblings, 1 reply; 12+ messages in thread From: Jonathan Lebon @ 2014-09-02 15:03 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: systemtap, fche Hi Stefan, I'm wondering, could we do without the user needing to provide the -u option at all and make it completely transparent instead? I.e. the initscript could always use the new --save-uprobes switch, and remember whether uprobes.ko was built or not so that at runtime, it appropriately appends -u to the staprun command. This also works around overriding the '-u' option in the config file, which could previously be used to enable unoptimized mode. The only question is how/where this setting would be remembered. One way I can think of right now is to simply touch a blank file in the CACHE_PATH's uprobes directory for each script that requires it, but maybe there's a more elegant method. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] initscript: add support for uprobes scripts 2014-09-02 15:03 ` Jonathan Lebon @ 2014-09-03 12:46 ` Stefan Hajnoczi 2014-09-03 23:01 ` Jonathan Lebon 0 siblings, 1 reply; 12+ messages in thread From: Stefan Hajnoczi @ 2014-09-03 12:46 UTC (permalink / raw) To: Jonathan Lebon; +Cc: systemtap, fche [-- Attachment #1: Type: text/plain, Size: 832 bytes --] On Tue, Sep 02, 2014 at 11:03:26AM -0400, Jonathan Lebon wrote: > I'm wondering, could we do without the user needing to > provide the -u option at all and make it completely > transparent instead? That's a nice idea. > The only question is how/where this setting would be > remembered. One way I can think of right now is to simply > touch a blank file in the CACHE_PATH's uprobes directory for > each script that requires it, but maybe there's a more > elegant method. The $CACHE_PATH/$SCRIPT.opts file seems like the logical place to save it. Currently the format is: $(uname -a) stap compile opts We can add a new line extra options (in a backwards-compatible fashion to avoid breaking existing cache directories). If it contains --load-uprobes then staprun will be invoked with -u$CACHE_PATH/uprobes/uprobes.ko. Stefan [-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] initscript: add support for uprobes scripts 2014-09-03 12:46 ` Stefan Hajnoczi @ 2014-09-03 23:01 ` Jonathan Lebon 2014-09-03 23:22 ` Josh Stone 0 siblings, 1 reply; 12+ messages in thread From: Jonathan Lebon @ 2014-09-03 23:01 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: systemtap, fche Hi Stefan, I've pushed the original series for now (v2). Thanks for the contribution! :) I've added a follow-up commit to make use of --save-uprobes rather than -u so that we don't block the stap -u option while remaining compatible with the idea we mentioned. BTW, while testing, I've noticed that --save-uprobes only has an effect if uprobes.ko was not already built and cached in a previous run. Have you encountered that as well? We may need to tweak the buildrun.cxx logic a bit if that's the case. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] initscript: add support for uprobes scripts 2014-09-03 23:01 ` Jonathan Lebon @ 2014-09-03 23:22 ` Josh Stone 2014-09-03 23:44 ` [PATCH] Remove systemtap_session::built_uprobes Josh Stone 0 siblings, 1 reply; 12+ messages in thread From: Josh Stone @ 2014-09-03 23:22 UTC (permalink / raw) To: Jonathan Lebon, Stefan Hajnoczi; +Cc: systemtap, fche On 09/03/2014 04:01 PM, Jonathan Lebon wrote: > BTW, while testing, I've noticed that --save-uprobes only has an > effect if uprobes.ko was not already built and cached in a previous > run. Have you encountered that as well? > > We may need to tweak the buildrun.cxx logic a bit if that's the > case. I'd guess that get_cached_uprobes() ought to set s.built_uprobes when it assigns s.uprobes_path, just as make_uprobes() does. Actually, I think s.built_uprobes may not be needed at all, just check s.uprobes_path.empty(). ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] Remove systemtap_session::built_uprobes 2014-09-03 23:22 ` Josh Stone @ 2014-09-03 23:44 ` Josh Stone 2014-09-04 11:13 ` Stefan Hajnoczi 2014-09-04 15:20 ` Jonathan Lebon 0 siblings, 2 replies; 12+ messages in thread From: Josh Stone @ 2014-09-03 23:44 UTC (permalink / raw) To: systemtap; +Cc: jlebon, stefanha, fche, Josh Stone For the purpose of save_uprobes, it doesn't actually matter whether uprobes.ko was just built or was pulled from the cache. If we have the uprobes_path at all, go ahead and save it. --- buildrun.cxx | 4 +--- main.cxx | 2 +- session.cxx | 2 -- session.h | 1 - 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/buildrun.cxx b/buildrun.cxx index 67217c3459e5..5d3ab582c058 100644 --- a/buildrun.cxx +++ b/buildrun.cxx @@ -602,10 +602,8 @@ make_uprobes (systemtap_session& s) clog << _("uprobes rebuild exit code: ") << rc << endl; if (rc) s.set_try_server (); - else { + else s.uprobes_path = dir + "/uprobes.ko"; - s.built_uprobes = true; - } return rc; } diff --git a/main.cxx b/main.cxx index af4be20aa6bf..f88701cba1b8 100644 --- a/main.cxx +++ b/main.cxx @@ -990,7 +990,7 @@ passes_0_4 (systemtap_session &s) } // Copy uprobes module to the current directory. - if (s.save_uprobes && s.built_uprobes && !pending_interrupts) + if (s.save_uprobes && !s.uprobes_path.empty() && !pending_interrupts) { rc = create_dir("uprobes"); if (! rc) diff --git a/session.cxx b/session.cxx index e2a518ba0538..ee4e81b72e31 100644 --- a/session.cxx +++ b/session.cxx @@ -145,7 +145,6 @@ systemtap_session::systemtap_session (): need_uprobes = false; need_unwind = false; need_symbols = false; - built_uprobes = false; uprobes_path = ""; load_only = false; skip_badvars = false; @@ -327,7 +326,6 @@ systemtap_session::systemtap_session (const systemtap_session& other, need_uprobes = false; need_unwind = false; need_symbols = false; - built_uprobes = false; uprobes_path = ""; load_only = other.load_only; skip_badvars = other.skip_badvars; diff --git a/session.h b/session.h index e92fff167e88..63cacc997632 100644 --- a/session.h +++ b/session.h @@ -209,7 +209,6 @@ public: bool need_uprobes; bool need_unwind; bool need_symbols; - bool built_uprobes; std::string uprobes_path; std::string uprobes_hash; bool load_only; // flight recorder mode -- 1.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove systemtap_session::built_uprobes 2014-09-03 23:44 ` [PATCH] Remove systemtap_session::built_uprobes Josh Stone @ 2014-09-04 11:13 ` Stefan Hajnoczi 2014-09-04 15:20 ` Jonathan Lebon 1 sibling, 0 replies; 12+ messages in thread From: Stefan Hajnoczi @ 2014-09-04 11:13 UTC (permalink / raw) To: Josh Stone; +Cc: systemtap, jlebon, fche [-- Attachment #1: Type: text/plain, Size: 452 bytes --] On Wed, Sep 03, 2014 at 04:44:30PM -0700, Josh Stone wrote: > For the purpose of save_uprobes, it doesn't actually matter whether > uprobes.ko was just built or was pulled from the cache. If we have the > uprobes_path at all, go ahead and save it. > --- > buildrun.cxx | 4 +--- > main.cxx | 2 +- > session.cxx | 2 -- > session.h | 1 - > 4 files changed, 2 insertions(+), 7 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove systemtap_session::built_uprobes 2014-09-03 23:44 ` [PATCH] Remove systemtap_session::built_uprobes Josh Stone 2014-09-04 11:13 ` Stefan Hajnoczi @ 2014-09-04 15:20 ` Jonathan Lebon 1 sibling, 0 replies; 12+ messages in thread From: Jonathan Lebon @ 2014-09-04 15:20 UTC (permalink / raw) To: Josh Stone; +Cc: systemtap, stefanha, fche Hey Josh, This patch fixes the issue mentioned in the other thread for me. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-09-04 15:20 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-08-11 13:25 [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 2/3] initscript: copy uprobes.ko to cache directory Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 3/3] initscript: allow scripts to load uprobes Stefan Hajnoczi 2014-08-11 13:25 ` [PATCH v2 1/3] stap: add --save-uprobes Stefan Hajnoczi 2014-09-01 12:15 ` [PATCH v2 0/3] initscript: add support for uprobes scripts Stefan Hajnoczi 2014-09-02 15:03 ` Jonathan Lebon 2014-09-03 12:46 ` Stefan Hajnoczi 2014-09-03 23:01 ` Jonathan Lebon 2014-09-03 23:22 ` Josh Stone 2014-09-03 23:44 ` [PATCH] Remove systemtap_session::built_uprobes Josh Stone 2014-09-04 11:13 ` Stefan Hajnoczi 2014-09-04 15:20 ` Jonathan Lebon
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).