aux-syscalls change

aux-syscalls change

[Frank Ch. Eigler]

Hi -

I am a bit surprised by the subject patch (commit fd6324c).  Amongst
its effects is to always include these lookup tables in a generated
object code, if any other part of aux_syscalls is being called.
Algorithmically, it replaces a linear sequence of script if/thens with
a C linear search.

What aspect of the previous code did you deem unacceptable?  Why not
just use ordinary script-level lookup tables instead of yet more C
code?

- FChE

Re: aux-syscalls change

[Martin Hunt]

On Thu, 2007-09-27 at 14:21 -0400, Frank Ch. Eigler wrote:
> Hi -
> 
> I am a bit surprised by the subject patch (commit fd6324c).  Amongst
> its effects is to always include these lookup tables in a generated
> object code, if any other part of aux_syscalls is being called.

Yes, but they are small. And they save space overall.

> Algorithmically, it replaces a linear sequence of script if/thens with
> a C linear search.

Which saves some space.  

> What aspect of the previous code did you deem unacceptable?  Why not
> just use ordinary script-level lookup tables instead of yet more C
> code?

My intent was not to create more embedded C, it was to replace a bunch
of different embedded C with 2 well-tested functions.  The patch
replaces 3 embedded C functions and 4 script functions with the 2 new
functions and some lookup tables.  When compiling sys.stp from the
testsuite, this saved 12Kbytes in code size for the module.

Why use embedded C instead of scripts?  Because the values we need to
lookup are sometimes architecture or OS version dependent. And sometimes
we need to do these lookups from other embedded C functions. For
example, look at the old sources for get_mmap_args in aux_syscalls.
Ignoring the fact that the struct is incorrect and gives the wrong
results, and the sprintf() is almost certain to overflow, you can see
that the prot and mmap flags had to be decoded, despite there already
being functions that did the same thing.

I'm certainly open to suggestions on how to improve this.  Ultimately
I'd like to see most of the syscall tapset go away.  Why can't something
like $argstr be generated automatically? If there is an arg "struct
timeval __user *tvp" and a script tries to print $tvp, could it not
automatically be copied from userspace and decoded?  Decoding flags and
special values could not be automatically done, unless we somehow tag
them in specific functions. But if we rename and clean up a bit, it
should be easy for users to do something like print(fork_flags($flags))
if they want.

Re: aux-syscalls change

[Frank Ch. Eigler]

Hi -

> > I am a bit surprised by the subject patch (commit fd6324c).  Amongst
> > its effects is to always include these lookup tables in a generated
> > object code, if any other part of aux_syscalls is being called.
> 
> Yes, but they are small. And they save space overall.

No, they do not, if one compares a plain script-function reference to
aux_syscalls.  This part could be fixeed by moving the new lookup
array variable decls into the embedded-C functions bodies as static
variables.


> [...] Why use embedded C instead of scripts?  Because the values we
> need to lookup are sometimes architecture or OS version dependent.
> And sometimes we need to do these lookups from other embedded C
> functions.  For example, look at the old sources for get_mmap_args
> in aux_syscalls.  [...]

OK, these are not bad reasons, but there also does not seem to be a
good reason to keep rewriting this bunch of undesirable code either.
Let us work toward a more complete solution than tiny improvements.


> Why can't something like $argstr be generated automatically?

You're right - it probably can be, once bug #3672 is dealt with.

> If there is an arg "struct timeval __user *tvp" and a script tries
> to print $tvp, could it not automatically be copied from userspace
> and decoded?

As you know, the debugging data does not currently include attributes
like "__user".  OTOH, we may be able to apply heuristics that guess,
based upon the pointer value, whether it's user- or kernel-space.  Can
there be a version of kta() for data?


- FChE

Re: aux-syscalls change

[Martin Hunt]

> > If there is an arg "struct timeval __user *tvp" and a script tries
> > to print $tvp, could it not automatically be copied from userspace
> > and decoded?
> 
> As you know, the debugging data does not currently include attributes
> like "__user".  OTOH, we may be able to apply heuristics that guess,
> based upon the pointer value, whether it's user- or kernel-space.  Can
> there be a version of kta() for data?

__user only applies to kernel functions, so it might tie in nicely with
4262.  Have our debuginfo compression utility scan the kernel sources
for function prototypes and mark args with __user.  Output a compressed,
optimized format for systemtap that includes this info.

Martin