Re: [RFC PATCH 1/2] Marker probes in futex.c

["Frank Ch. Eigler" <fche@redhat.com>]

Hi -

On Wed, Apr 16, 2008 at 05:51:24PM +0200, Peter Zijlstra wrote:
> [...]
> > > > > What is wrong with a few simple hooks like:
> > > > >   trace_futex_wait(uaddr, fshares, val, abs_time, bitset);
> > > > > and then deal with that.
> [...]
> > Yes, but then you would have to create new code for each event you want
> > to trace. In the end, it would increase the icache footprint
> > considerably and would also make addition of new events cumbersome.
> > [...]

That, plus the new hand-written function (trace_futex_wait) would
still need to manage the packaging of the arguments for consumption by
separately compiled pieces.  It is desirable not to require such
hand-written functions to *also* be declared in headers for these
event consumers to compile against.


> So I'm not sure what adding all these character strings buy you.

The main thing is type checking by engaging gcc's printf format
checking logic.  In my original markers proposal, the types were
encoded into the function name, sort of as in C++:

  trace_mark_nnnnn(futex_wake_called, uaddr, fshares, val, abs_time, bitset);

where each "n" stands for some integral value, and could be chosen
amongst a small number of other types (say -- "s": char* string, "p":
void*, "l":64-bit long).  Then, type checking could be done by the
core compiler for both event producers and consumers.  One downside
was that the trace_mark_* permutations themselves would have to be
generated by some shell/perl script [1], and some deemed this probably
unacceptable.  I'm still not sure...

[1] some systemtap archaeology:
http://sourceware.org/git/?p=systemtap.git;a=commit;h=b171146c8e8d4fa749b8829c47750750dc19f11c


> >+       trace_mark(futex_wake_called, "uaddr:%p fshared:%p nr_wake:%d "
> > +                       "bitset:%d",
> > +                       uaddr, fshared, nr_wake, bitset);
> 
> > +       INIT_FUTEX_DEBUG_PROBE(futex_wake_called,
> > +                       "uaddr:%p fshared:%p nr_wake:%d bitset:%d"),
> 
> Why the need to duplicate it; that's utter madness.

This second instance is optional and is used as a consistency check
for the event consumer to hook up exactly to the intended producer.
The string could be empty.


- FChE