Bug 4281 - two problems with "stap -m"
Summary: two problems with "stap -m"
Status: RESOLVED FIXED
Alias: None
Product: systemtap
Classification: Unclassified
Component: translator (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: David Smith
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-03-26 17:22 UTC by Martin Hunt
Modified: 2007-03-29 16:27 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Hunt 2007-03-26 17:22:31 UTC
Problem 1. "stap -m close close.stp"
Where's close.ko?  I shouldn't need to also use -k.

Problem 2. "stap -m close.ko close.stp"
This creates a module named "close.ko.ko" which will not be removed at cleanup.
Cannot be removed easily either.
> /sbin/lsmod | grep close
close.ko               46252  0 
> sudo /sbin/rmmod close.ko
ERROR: Module close does not exist in /proc/modules
> sudo /sbin/rmmod close.ko.ko
ERROR: Module close does not exist in /proc/modules
Comment 1 David Smith 2007-03-26 17:37:18 UTC
Problem 1.  There is at least one reason for using '-m NAME' without '-k' - to
disable caching.  Admittedly there is another way to disable caching
("SYSTEMTAP_DIR=/dev/null stap close.stp"), but it is uglier.

Problem 2.  Does this happen with any extension or just '.ko'?  Is any extension
valid?  For instance, are any of the following valid?

stap -m close.c close.stp
stap -m close.o close.stp
stap -m my.close.module close.stp

We should probably worry about other special characters in the module name, such
as '/'.
Comment 2 Martin Hunt 2007-03-26 18:09:41 UTC
(In reply to comment #1)
> Problem 1.  There is at least one reason for using '-m NAME' without '-k' - to
> disable caching.  Admittedly there is another way to disable caching
> ("SYSTEMTAP_DIR=/dev/null stap close.stp"), but it is uglier.

I don't see how its a good idea to combine to options like this. Instead of
relying on a side-effect of the way "-m" is currently implemented to disable
cache, why not give it its own option?

Regardless, you can still disable cache and compile a module, then place a copy
of it in the current directory. That is what I think it should do.

> Problem 2.  Does this happen with any extension or just '.ko'?  Is any extension
> valid?  For instance, are any of the following valid?
> 
> stap -m close.c close.stp
> stap -m close.o close.stp
> stap -m my.close.module close.stp
> 
> We should probably worry about other special characters in the module name, such
> as '/'.

Consulting the source code in rmmod, "." and "-" (dash) are not used in module
names. Anything after the dot is ignored.  There might be other characters that,
if quoted just right, would make it through the systemtap command line and
compilation process but even if they end up in the module name probably won't be
a problem.

I recommend detecting names ending in ".ko" and silently doing the right thing.
Otherwise if the module name contains "." or "-", print an error and exit.
Comment 3 Frank Ch. Eigler 2007-03-26 18:34:36 UTC
(In reply to comment #2)
> I don't see how its a good idea to combine to options like this. Instead of
> relying on a side-effect of the way "-m" is currently implemented to disable
> cache, why not give it its own option?

It need not actually bypass the cache.  We could cache the resulting probe.ko
file and reuse it next time.
 
> Regardless, you can still disable cache and compile a module, then place a copy
> of it in the current directory. That is what I think it should do.

Surely that's only appropriate in -p4 mode, for which we now print the resulting
module file name on stdout.  We don't normally put files into pwd at all, though
that is a plausible fallback if neither -k was given nor the cache is available.
Comment 4 Martin Hunt 2007-03-26 18:54:45 UTC
(In reply to comment #3)
> 
> It need not actually bypass the cache.  We could cache the resulting probe.ko
> file and reuse it next time.

Or if it is already cached under another name (stap...) just copy it to the
destination and rename it.  This would be one way to enable running multiple
copies of the same script without disabling the cache.

> > Regardless, you can still disable cache and compile a module, then place a copy
> > of it in the current directory. That is what I think it should do.
> 
> Surely that's only appropriate in -p4 mode, for which we now print the resulting
> module file name on stdout.  We don't normally put files into pwd at all, though
> that is a plausible fallback if neither -k was given nor the cache is available.

> stap -p4 -m close examples/small_demos/close.stp
Warning: using '-m' disables cache support.

> ls -l close*
/bin/ls: close*: No such file or directory

At the very least, we need some kind of warning that you need to use "-k" with
"-m" and look for your module in the temporary directory. But wouldn't it be
better to not require "-k" at all and maybe just put the module in the current
directory.  Wouldn't that make scripting things easier to?  Right now you have
to compile modules and parse the output from stap to get the temporary
directory, then copy the module from there to where you want it, then rm -rf the
temp directory.

Comment 5 David Smith 2007-03-26 19:07:58 UTC
The reason why the use of '-m' disables the cache is that we do cache lookups
based on filenames.  The hash of the compile environment, systemtap options, and
pass 2 output becomes the module name and filename.  If we can find that
filename in the cache directory, we reuse it.

Let's say you run "stap -m foo script1.stp" and we somehow cache the result. 
Then you run "stap -m foo script2.stp" - obviously you don't want the cached
result, but we'd have no way of knowing that.

We could theoretically rename the module filename to its hash name when storing
it in the cache, but then you couldn't use the module directly from the cache -
you'd have to know to rename it back to the original '-m' name.
Comment 6 Josh Stone 2007-03-26 19:34:52 UTC
(In reply to comment #4)
> Or if it is already cached under another name (stap...) just copy it to the
> destination and rename it.  This would be one way to enable running multiple
> copies of the same script without disabling the cache.

I looked at this before -- I think the kernel gets the module name from some
compiled variable rather than the name of the actual ko.  So simply renaming the
binary isn't enough to allow multiple instances.
Comment 7 Martin Hunt 2007-03-26 19:42:10 UTC
(In reply to comment #5)
> 
> Let's say you run "stap -m foo script1.stp" and we somehow cache the result. 
> Then you run "stap -m foo script2.stp" - obviously you don't want the cached
> result, but we'd have no way of knowing that.

This is beyond the scope of this BZ. I am aware of the basics how stap caching
works. But I don't think that rules out using the cache with -m.
> stap script1.stp
( puts compiled script in long cached stap* name)
> stap -m foo script1.stp
(copies and renames the cached stap* module and loads that)
Comment 8 Martin Hunt 2007-03-26 19:45:18 UTC
(In reply to comment #6)

> I looked at this before -- I think the kernel gets the module name from some
> compiled variable rather than the name of the actual ko.  So simply renaming the
> binary isn't enough to allow multiple instances.

No, you would have to read in the module header and modify the name in place.
It's a fixed size static buffer so it isn't hard. This is what "modprobe -o" does.
Comment 9 David Smith 2007-03-26 19:52:21 UTC
(In reply to comment #7)

> This is beyond the scope of this BZ. I am aware of the basics how stap caching
> works. But I don't think that rules out using the cache with -m.
> > stap script1.stp
> ( puts compiled script in long cached stap* name)
> > stap -m foo script1.stp
> (copies and renames the cached stap* module and loads that)

True.  But, that disallows the user from using the cached file directly.

# stap -p4 begin.stp 
/home/dsmith/.systemtap/cache/fc/stap_fc1322dcb34cdb7c96679435f22df754_264.ko
# staprun
/home/dsmith/.systemtap/cache/fc/stap_fc1322dcb34cdb7c96679435f22df754_264.ko
... script runs ...

The above will work correctly.  However, if we do the same thing with '-m', it
would fail, since the cached file couldn't be used directly - it has to be
renamed first.
Comment 10 David Smith 2007-03-26 19:54:47 UTC
(In reply to comment #2)
> I recommend detecting names ending in ".ko" and silently doing the right thing.
> Otherwise if the module name contains "." or "-", print an error and exit.

How about we only allow characters [_a-zA-Z0-9] in the '-m' argument (and
otherwise error out)?  Seems the safest plan.
Comment 11 David Smith 2007-03-29 17:27:04 UTC
Original problem 1 ("stap -m" shouldn't need '-k') has been refiled as bug #4295.

Original problem 2 ("stap -m close.ko" doesn't work) has been fixed.  Any
trailing '.ko' is chopped off.  In addition, a module name must be composed of
characters [_a-zA-Z0-9].