lkml.org 
[lkml]   [2008]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH 1/2] Marker probes in futex.c
From
Date
On Thu, 2008-04-17 at 15:19 -0400, Frank Ch. Eigler wrote:
> 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.

*blink* so all this is so you don't have to put a declarion in a header
file?

How about we put these premanent markers in a header - Mathieu says
there are <200. Surely that's not too much trouble.

Then you can keep this trace_mark() (perhaps trace_printf() is a better
name) around for the ad-hoc debug hacks.

> > 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.

So instead of writing normal C code and placing a declarion in a header,
you've come up with a scheme that needs to duplicate a text string to
check integrity. Sounds like a real good way to confuse people.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2008-04-18 08:59    [W:0.610 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site