Messages in this thread | | | Subject | Re: [RFC PATCH 1/2] Marker probes in futex.c | From | Peter Zijlstra <> | Date | Fri, 18 Apr 2008 08:56:07 +0200 |
| |
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/
| |