State machine of perf lock requires "nested" field of lock_release(),
so this patch adds it to event.
Signed-off-by: Hitoshi Mitake <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
---
include/trace/events/lock.h | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/lock.h b/include/trace/events/lock.h
index 5c1dcfc..4f489b5 100644
--- a/include/trace/events/lock.h
+++ b/include/trace/events/lock.h
@@ -44,15 +44,17 @@ TRACE_EVENT(lock_release,
TP_STRUCT__entry(
__string(name, lock->name)
__field(void *, lockdep_addr)
+ __field(int, nested)
),
TP_fast_assign(
__assign_str(name, lock->name);
__entry->lockdep_addr = lock;
+ __entry->nested = nested;
),
- TP_printk("%p %s",
- __entry->lockdep_addr, __get_str(name))
+ TP_printk("%p %s %d",
+ __entry->lockdep_addr, __get_str(name), __entry->nested)
);
#ifdef CONFIG_LOCK_STAT
--
1.6.5.2
On Sat, 2010-04-10 at 19:41 +0900, Hitoshi Mitake wrote:
> State machine of perf lock requires "nested" field of lock_release(),
> so this patch adds it to event.
>
> Signed-off-by: Hitoshi Mitake <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
The nested flag only indicates if it could be nested, not if it was
actually nested. So no, this doesn't make any sense at all.
(in fact, pretty much every lock_release out there has nested=1)
On 04/10/10 22:59, Peter Zijlstra wrote:
> On Sat, 2010-04-10 at 19:41 +0900, Hitoshi Mitake wrote:
>> State machine of perf lock requires "nested" field of lock_release(),
>> so this patch adds it to event.
>>
>> Signed-off-by: Hitoshi Mitake<[email protected]>
>> Cc: Peter Zijlstra<[email protected]>
>> Cc: Paul Mackerras<[email protected]>
>> Cc: Arnaldo Carvalho de Melo<[email protected]>
>> Cc: Frederic Weisbecker<[email protected]>
>
> The nested flag only indicates if it could be nested, not if it was
> actually nested. So no, this doesn't make any sense at all.
>
> (in fact, pretty much every lock_release out there has nested=1)
>
Ah, sorry, I misunderstood.
When looked at rwlock_release(), I judged that nested is required,
because double or more read_lock() is allowed.
Thanks for your correction :)
Hitoshi
On Sat, 2010-04-10 at 23:50 +0900, Hitoshi Mitake wrote:
> On 04/10/10 22:59, Peter Zijlstra wrote:
> > On Sat, 2010-04-10 at 19:41 +0900, Hitoshi Mitake wrote:
> >> State machine of perf lock requires "nested" field of lock_release(),
> >> so this patch adds it to event.
> >>
> >> Signed-off-by: Hitoshi Mitake<[email protected]>
> >> Cc: Peter Zijlstra<[email protected]>
> >> Cc: Paul Mackerras<[email protected]>
> >> Cc: Arnaldo Carvalho de Melo<[email protected]>
> >> Cc: Frederic Weisbecker<[email protected]>
> >
> > The nested flag only indicates if it could be nested, not if it was
> > actually nested. So no, this doesn't make any sense at all.
> >
> > (in fact, pretty much every lock_release out there has nested=1)
> >
Just to clarify, nested=0 means the released lock needs to be at the top
of the lock stack. This means all locking needs to be perfectly
balanced:
lock A
lock B
unlock A
unlock B
Would be an invalid sequence in that scenario. I think when lockdep was
started it was thought it would be nice to be able to clean up locking
to be always perfectly balanced, but it was found quite early to be too
much, so it got relaxed to always nested=1, allowing the above sequence.