[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1208501767.7115.67.camel@twins>
Date: Fri, 18 Apr 2008 08:56:07 +0200
From: Peter Zijlstra <a.p.zijlstra@...llo.nl>
To: "Frank Ch. Eigler" <fche@...hat.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>,
prasad@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
tglx@...utronix.de, mingo@...e.hu,
Christoph Hellwig <hch@...radead.org>
Subject: Re: [RFC PATCH 1/2] Marker probes in futex.c
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists