(In reply to comment #0)

These are valid concerns, but I think the solutions need further thought.  See
below.

One thing to keep in mind is that we don't expect uprobes.ko to be insmod-ed and
rmmod-ed every time somebody runs a stap script.  Rather, once uprobes is in the
kernel, it stays there.  (As mentioned in bugs #5079 and #5083, sometimes
uprobes has to hang around long after the last unregister.)  The typical
scenario is that when it's time to insmod a stap-generated module,  uprobes is
already in the kernel (which we can detect becauses the uprobes symbols show up
in /proc/kallsyms).

We can certainly handle somebody rmmod-ing uprobes (in which case we
automatically rebuild it and/or re-insmod it as necessary), but that's not the
typical situation.

The only times we try to build uprobes.ko are:
1. we're stopping after pass 4 (in case uprobes.ko is needed in the environment
where the script will eventually be run); or
2. we're proceeding to pass 5 and we don't see uprobes in the kernel...
... and the script needs uprobes, of course.

> We need to tweak some of the uprobes.ko build steps from bug #5079.
> Several problems: /usr/share/systemtap/runtime/uprobes is the wrong place
> to build an object file into; /usr could be read-only;

And a user from group stapdev doesn't have write access to the directory where
stap currently tries to build uprobes.ko.  You'd have to run stap as root for
the build to succeed.

> the file may not be
> easily accessible to staprun except in the local/immediate probing scenario;

Yes, if you move the stap-generated module to another system, you may need to
bring uprobes.ko with it.

> uprobes.ko is kernel-version/arch. dependent, just like the systemtap probe
> module.
> 
> Let's investigate building the uprobes.ko more similarly to probe modules.
> 
> It could be stored in the module cache directory ($SYSTEMTAP_DIR/uprobes/).

Since each stap user has his/her own cache directory (by default), this would
mean multiple copies of uprobes.ko (and possibly the uprobes source).

> It could be given a name that encodes the version/architecture parameters
> (so the cache directory itself can be shared between machines).  Instead
> of passing just "-u" to staprun and have it find uprobes.ko someplace,
> pass "UPROBEFILENAME.ko" itself in addition ot the "PROBEFILE.ko".
> "stap -p4" could itself print out both needed modules.

Jim

Further thoughts:

a) How about moving the uprobes source from
$(DESTDIR)/$(pkgdatadir)/runtime/uprobes to $(DESTDIR)/var/systemtap/uprobes? 
That would insure that the uprobes home lives in a writable filesystem, so
uprobes.ko can be built there.  I guess the preferred way to refer to
$(DESTDIR)/var/systemtap in Makefile.am is as
$(DESTDIR)/$(localstatedir)/var/$(PACKAGE).

b) I think it's correct that only root can build the version of uprobes.ko that
staprun insmods.  We don't want non-root users -- even users in  group stapdev
-- to mess with the uprobes source.  This also gets around some rather messy
problems that can arise if root builds uprobes.ko and then a non-root member of
stapdev tries to build it.

c) staprun will modprobe or insmod uprobes.ko, if necessary, for anybody who
invokes staprun with -u and specifies a permitted stap-generated module.  I
think this is OK.

(In reply to comment #2)
...
> 
> c) staprun will modprobe or insmod uprobes.ko, if necessary, for anybody who
> invokes staprun with -u and specifies a permitted stap-generated module.  I
> think this is OK.

Let me clarify the above, before anybody gets too worked up.  staprun runs
check_permissions() before trying to ensure that uprobes is available.  So if
you're not root, and not in stapdev or stapusr, no dice.  If you're not root and
not in stapdev, and the specified module isn't in /lib/modules/KVER/systemtap,
no dice.  (However, staprun doesn't currently verify that the module actually
uses uprobes.)

This does impact usability because of some oddities:
- A user in the stapdev group still has to sudo to get uprobes compiled and loaded.
  The error messages don't immediately make that clear. Especially confusing
after upgrading the kernel.
- When running a script that is cached no test is done to see if uprobes is loaded.
  Somewhat confusing after a reboot

(In reply to comment #4)
> This does impact usability because of some oddities:
...
> - When running a script that is cached no test is done to see if uprobes is
loaded.
>   Somewhat confusing after a reboot

That one is very annoying to me.  It makes cached modules of scripts that use
uprobes useless.

(In reply to comment #5)
> (In reply to comment #4)
> > This does impact usability because of some oddities:
> ...
> > - When running a script that is cached no test is done to see if uprobes is
> loaded.
> >   Somewhat confusing after a reboot
> 
> That one is very annoying to me.  It makes cached modules of scripts that use
> uprobes useless.

That particular nit should be improved now:

commit 8a03658eed54ff02494107f0ca55081f352df75b
Author: Josh Stone <jistone@redhat.com>
Date:   Fri Apr 3 14:50:18 2009 -0700

    PR5163: Assign need_uprobes during pass-2
    
    We were doing this in pass-3, which means it was never set for cached
    runs, and so staprun didn't get the -u flag.  Now need_probes is set as
    soon as a uprobe_derived_probe is saved into the session in pass-2.

Got another instance of this. With the recent update of uprobes I kept getting
failures loading stap modules. It turned out (in dmesg) because the new uprobes
defines two new symbols. But since the uprobes module is never unloaded the old
version without the new functions was still there. Only after a manual rmmod
uprobes did things start working again. That was a bit odd failure mode.

The build process changes probably wouldn't fix mjw's problem.
However, one thing we could do easily would be similar to the logic
recently added to staprun to automatically clean up junk leftover
stap_XXXX modules at startup.  It could gently attempt to module_remove
any existing uprobes first, then load the new copy.

(In reply to comment #8)
> The build process changes probably wouldn't fix mjw's problem.
> However, one thing we could do easily would be similar to the logic
> recently added to staprun to automatically clean up junk leftover
> stap_XXXX modules at startup.  It could gently attempt to module_remove
> any existing uprobes first, then load the new copy.

How about if "make install" unloads uprobes.ko?

For that matter, "make install" could attempt to build uprobes.ko, to help
prevent the situation where a stapdev or stapusr user can't run his script until
root builds uprobes.ko for him.  (It would have to be tolerant of the case where
there's already a kernel-built version of uprobes.)

Another thing we could do is add a script to /etc/init.d to (re)build
uprobes.ko, thus keeping it in sync with the running kernel.

(In reply to comment #9)
> How about if "make install" unloads uprobes.ko?
> For that matter, "make install" could attempt to build uprobes.ko

Good ideas; should help systemtap developers.

> Another thing we could do is add a script to /etc/init.d to (re)build
> uprobes.ko, thus keeping it in sync with the running kernel.

Should help systemtap users, except for security concerns (running
kernel make etc. at root during init).  dkms ...

We're thinking about this bug again, especially now that we have cases that must be responsible for multiple distinct targets (--remote and --use-server both).  I'd like to try leaving the uprobes source where it is in SYSTEMTAP_RUNTIME, but build the output in our tmpdir and cache it like all the rest.

Earlier concerns about even stapdev providing uprobes.ko seem moot, given that such users could do whatever they want in guru-mode anyway.

Created attachment 5811 [details]
implements cached uprobes

commit 7d26ee024