On 2019/01/10 19:21, Dmitry Vyukov wrote: >> @@ -3535,6 +3535,9 @@ static int __lock_downgrade(struct lockdep_map *lock, >> unsigned long ip) >> unsigned int depth; >> int i; >> >> + if (unlikely(!debug_locks)) >> + return 0; >> + > > Are we sure this resolves the problem rather than makes the > inconsistency window smaller?
As far as I know, this should resolve the problem. > I don't understand all surrounding code, but looking just at this > function it looks like it may just pepper over the problem. Say, we > pass this check when lockdep was still turned on. Then this thread is > preempted for some time (e.g. a virtual CPU), then another thread > started reporting a warning, turned lockdep off, some information > wasn't collected, and this this task resumes and reports a false > warning. What this function checks is whether current thread is holding rw_semaphore for write. Since the information of held locks are per "struct task_struct" record, if lockdep is still enabled as of entry of this function, there must be a lockdep record that current thread is holding rw_semaphore for write if current thread is actually holding rw_semaphore for write. Therefore, preemption/interrupts can't erase the lockdep record that current thread is holding rw_semaphore, even if lockdep is turned off after passing this check. > Or we are holding the mutex here, and the fact that we are holding it > ensures that no other task will take it and no information will be > lost? > Quite a tricky moment, perhaps deserves a comment. I think many other functions check debug_locks upon entry of functions. > >> depth = curr->lockdep_depth; >> /* >> * This function is about (re)setting the class of a held lock,
