diff options
Diffstat (limited to 'kernel')
| -rw-r--r-- | kernel/locking/lockdep.c | 56 | ||||
| -rw-r--r-- | kernel/workqueue.c | 23 |
2 files changed, 58 insertions, 21 deletions
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 66011c9f5df3..f73ca595b81e 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -4629,7 +4629,7 @@ asmlinkage __visible void lockdep_sys_exit(void) * the index to point to the last entry, which is already invalid. */ crossrelease_hist_end(XHLOCK_PROC); - crossrelease_hist_start(XHLOCK_PROC); + crossrelease_hist_start(XHLOCK_PROC, false); } void lockdep_rcu_suspicious(const char *file, const int line, const char *s) @@ -4725,25 +4725,25 @@ static inline void invalidate_xhlock(struct hist_lock *xhlock) /* * Lock history stacks; we have 3 nested lock history stacks: * - * Hard IRQ - * Soft IRQ - * History / Task + * HARD(IRQ) + * SOFT(IRQ) + * PROC(ess) * - * The thing is that once we complete a (Hard/Soft) IRQ the future task locks - * should not depend on any of the locks observed while running the IRQ. + * The thing is that once we complete a HARD/SOFT IRQ the future task locks + * should not depend on any of the locks observed while running the IRQ. So + * what we do is rewind the history buffer and erase all our knowledge of that + * temporal event. * - * So what we do is rewind the history buffer and erase all our knowledge of - * that temporal event. - */ - -/* - * We need this to annotate lock history boundaries. Take for instance - * workqueues; each work is independent of the last. The completion of a future - * work does not depend on the completion of a past work (in general). - * Therefore we must not carry that (lock) dependency across works. + * The PROCess one is special though; it is used to annotate independence + * inside a task. + * + * Take for instance workqueues; each work is independent of the last. The + * completion of a future work does not depend on the completion of a past work + * (in general). Therefore we must not carry that (lock) dependency across + * works. * * This is true for many things; pretty much all kthreads fall into this - * pattern, where they have an 'idle' state and future completions do not + * pattern, where they have an invariant state and future completions do not * depend on past completions. Its just that since they all have the 'same' * form -- the kthread does the same over and over -- it doesn't typically * matter. @@ -4751,15 +4751,31 @@ static inline void invalidate_xhlock(struct hist_lock *xhlock) * The same is true for system-calls, once a system call is completed (we've * returned to userspace) the next system call does not depend on the lock * history of the previous system call. + * + * They key property for independence, this invariant state, is that it must be + * a point where we hold no locks and have no history. Because if we were to + * hold locks, the restore at _end() would not necessarily recover it's history + * entry. Similarly, independence per-definition means it does not depend on + * prior state. */ -void crossrelease_hist_start(enum xhlock_context_t c) +void crossrelease_hist_start(enum xhlock_context_t c, bool force) { struct task_struct *cur = current; - if (cur->xhlocks) { - cur->xhlock_idx_hist[c] = cur->xhlock_idx; - cur->hist_id_save[c] = cur->hist_id; + if (!cur->xhlocks) + return; + + /* + * We call this at an invariant point, no current state, no history. + */ + if (c == XHLOCK_PROC) { + /* verified the former, ensure the latter */ + WARN_ON_ONCE(!force && cur->lockdep_depth); + invalidate_xhlock(&xhlock(cur->xhlock_idx)); } + + cur->xhlock_idx_hist[c] = cur->xhlock_idx; + cur->hist_id_save[c] = cur->hist_id; } void crossrelease_hist_end(enum xhlock_context_t c) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 8ad214dc15a9..c0331891dec1 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -2093,7 +2093,28 @@ __acquires(&pool->lock) lock_map_acquire(&pwq->wq->lockdep_map); lock_map_acquire(&lockdep_map); - crossrelease_hist_start(XHLOCK_PROC); + /* + * Strictly speaking we should do start(PROC) without holding any + * locks, that is, before these two lock_map_acquire()'s. + * + * However, that would result in: + * + * A(W1) + * WFC(C) + * A(W1) + * C(C) + * + * Which would create W1->C->W1 dependencies, even though there is no + * actual deadlock possible. There are two solutions, using a + * read-recursive acquire on the work(queue) 'locks', but this will then + * hit the lockdep limitation on recursive locks, or simly discard + * these locks. + * + * AFAICT there is no possible deadlock scenario between the + * flush_work() and complete() primitives (except for single-threaded + * workqueues), so hiding them isn't a problem. + */ + crossrelease_hist_start(XHLOCK_PROC, true); trace_workqueue_execute_start(work); worker->current_func(work); /* |
