Messages in this thread | | | Date | Sat, 26 Dec 2009 22:43:44 +0900 | Subject | Re: [PATCH 2/2] perf lock: Fix output of tracing lock events | From | Hitoshi Mitake <> |
| |
Sorry for my slow response...
>> diff --git a/include/trace/events/lock.h b/include/trace/events/lock.h >> index a870ba1..2f94e25 100644 >> --- a/include/trace/events/lock.h >> +++ b/include/trace/events/lock.h >> @@ -18,16 +18,19 @@ TRACE_EVENT(lock_acquire, >> TP_ARGS(lock, subclass, trylock, read, check, next_lock, ip), >> >> TP_STRUCT__entry( >> + __field(struct lockdep_map *, lockdep_addr) >> __field(unsigned int, flags) >> __string(name, lock->name) >> ), > > I feel a bit awkward explicitly leaking kernel pointers like that. All > this is accessible by root only (for now) so its not too harmfull, but > sitll. > > Also, I don't think we want to expose the struct lockdep_map thing, a > regular void * would be better.
Yeah, I agree with it. void * is enough.
> > As to removing the waittime, I'm not sure, in this case, yes, but if you > want some other processing that hooks straight into the tracepoints > instead of using a logging structure, it might be useful. > > Removing that do_div() from there and exposing waittime as u64 in nsec, > for sure, that do_div() is just silly. > > >
I was too egoist. perf lock is not an only one user of lock events.
And I have a suggestion. Adding name of source files and lines of lock instances may be good thing for human's readability. How do you think?
I have some pending works for these (not made into patches). If you agree, I'll send these later. -- 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/
| |