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

[K. Prasad]

On Tue, Apr 15, 2008 at 02:02:22PM +0200, Peter Zijlstra wrote:
> On Tue, 2008-04-15 at 17:23 +0530, K. Prasad wrote:
> 
> > +	trace_mark(futex_wait_called, "uaddr:%p fshared:%p val:%u "
> > +			"abs_time:%p bitset:%d",
> > +			uaddr, fshared, val, abs_time, bitset);
> 
> This is some seriuosly ugly looking gunk, why would we want stuff like
> that scattered across the code?
> 
> What is wrong with a few simple hooks like:
> 
>   trace_futex_wait(uaddr, fshares, val, abs_time, bitset);
> 
> and then deal with that.
> 
> Also, you seem to expose way too much futex internals; do you really
> need that? People will go use this marker crap like ABI and further
> restrain us from changing the code.
The idea is to export as much data as possible (considering that the
cost involved in doing so is less) and use them only when required.

If we were to log just the futex_ops, just as you had suggested,
"Just log:
 
  futex: <uaddr> wait
  futex: <uaddr> wakeup"
it would provide only cursory information about code-flow and not enough
debug information to help the user solve the issue he's trying to debug.

Say for e.g. in futex_wait you may want to know if abs_time had a
non-zero value but the primitive debugging information would end
up being a handicap from discovering that.

If you can specifically point me to information you think would be
absolutely unnecessary, I can get them out of the trace_mark().

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