Messages in this thread | | | Date | Sun, 21 Mar 2010 18:49:40 +0900 | From | Hitoshi Mitake <> | Subject | Re: [PATCH RFC 00/11] lock monitor: Separate features related to lock |
| |
On 03/20/10 17:23, Hitoshi Mitake wrote: > On 03/20/10 14:56, Hitoshi Mitake wrote: > > On 03/19/10 06:16, Frederic Weisbecker wrote: > > >> And I have a question related to this dynamic patching approach for > > lockdep. > > >> If dynamic proving turning on/off is provided, > > >> lockdep will be confused by inconsistency of lock acquiring log. > > >> > > >> Will the sequence, > > >> > > >> lock_acquire(l) -> turning off -> lock_release(l) -> turning on -> > > >> lock_acquire(l) > > >> > > >> detected as double acquiring? > > >> > > >> Should turning on/off lockdep be done in the time > > >> when every processes have no lock? > > > > > > > > > There is almost always a process with a lock somewhere ;-) > > > > Yeah :) > > > > > > > > This is not a big deal, it's very similar to unfinished scenarios > > > due to the end of the tracing that can happen anytime and you miss > > > a lock_release or whatever. We can also begin the tracing anytime, > > > and you may receive orphan lock_release in the very beginning > > > because you missed the lock_acquire that happened before the tracing. > > > > > > Any locking scenario that doesn't fit into the state machine > > > or is incomplete must be considered as broken and then ignored. > > > > > > > > > > I see, thanks. > > I have to fix state machine of perf lock. > > Now it doesn't consider read, try and orphan events, > > it is very incompletely.. > > > > Ah, sorry, I've mentioned that these cases might be > a problem for validation part of lockdep, not for events. > > If the lock and turning on/off sequence like this happened, > lock_acquire(l) -> turning off -> lock_release(l) -> turning on -> > lock_acquire(l) > this will confuse validator of lockdep. > At least, task_struct.lockdep_depth will be corrupted. > > And I have a trivial question to Ingo. > In lockdep, held_locks of task_struct are accessed this arithmetical way > prev = curr->held_locks + i; > Of course this is valid way, but I feel it is more simple and natural way > prev = curr->held_locks[i]; >
Ah, sorry, prev = curr->held_locks[i]; is wrong. It's prev = &curr->held_locks[i];
| |