From a0ecc1e7e5c739b173e535f5aa36fa604425f9ac Mon Sep 17 00:00:00 2001 From: Geoffrey Challen Date: Thu, 9 Feb 2017 09:52:18 -0500 Subject: [PATCH] Changes to use deadlock detection. --- kern/thread/hangman.c | 182 +++++++++++++++++++++++++++++++++++++++++ kern/thread/spinlock.c | 8 ++ kern/thread/synch.c | 11 +++ kern/thread/thread.c | 3 + 4 files changed, 204 insertions(+) create mode 100644 kern/thread/hangman.c diff --git a/kern/thread/hangman.c b/kern/thread/hangman.c new file mode 100644 index 0000000..127167a --- /dev/null +++ b/kern/thread/hangman.c @@ -0,0 +1,182 @@ +/* + * Copyright (c) 2015 + * The President and Fellows of Harvard College. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE UNIVERSITY AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE UNIVERSITY OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* + * Simple deadlock detector. + */ + +#include +#include +#include +#include +#include + +static struct spinlock hangman_lock = SPINLOCK_INITIALIZER; + +/* + * Look for a path through the waits-for graph that goes from START to + * TARGET. + * + * Because lockables can only be held by one actor, and actors can + * only be waiting for one thing at a time, this turns out to be + * quite simple. + */ +static +void +hangman_check(const struct hangman_lockable *start, + const struct hangman_actor *target) +{ + const struct hangman_actor *cur; + + cur = start->l_holding; + while (cur != NULL) { + if (cur == target) { + goto found; + } + if (cur->a_waiting == NULL) { + break; + } + cur = cur->a_waiting->l_holding; + } + return; + + found: + /* + * None of this can change while we print it (that's the point + * of it being a deadlock) so drop hangman_lock while + * printing; otherwise we can come back via kprintf_spinlock + * and that makes a mess. But force splhigh() explicitly so + * the console prints in polled mode and to discourage other + * things from running in the middle of the printout. + */ + splhigh(); + spinlock_release(&hangman_lock); + + kprintf("hangman: Detected lock cycle!\n"); + kprintf("hangman: in %s (%p);\n", target->a_name, target); + kprintf("hangman: waiting for %s (%p), but:\n", start->l_name, start); + kprintf(" lockable %s (%p)\n", start->l_name, start); + cur = start->l_holding; + while (cur != target) { + kprintf(" held by actor %s (%p)\n", cur->a_name, cur); + kprintf(" waiting for lockable %s (%p)\n", + cur->a_waiting->l_name, cur->a_waiting); + cur = cur->a_waiting->l_holding; + } + kprintf(" held by actor %s (%p)\n", cur->a_name, cur); + panic("Deadlock.\n"); +} + +/* + * Note that a is about to wait for l. + * + * Note that there's no point calling this if a isn't going to wait, + * because in that case l->l_holding will be null and the check + * won't go anywhere. + * + * One could also maintain in memory a graph of all requests ever + * seen, in order to detect lock order inversions that haven't + * actually deadlocked; but there are a lot of ways in which this is + * tricky and problematic. For now we'll settle for just detecting and + * reporting deadlocks that do happen. + */ +void +hangman_wait(struct hangman_actor *a, + struct hangman_lockable *l) +{ + if (l == &hangman_lock.splk_hangman) { + /* don't recurse */ + return; + } + + spinlock_acquire(&hangman_lock); + + if (a->a_waiting != NULL) { + spinlock_release(&hangman_lock); + panic("hangman_wait: already waiting for something?\n"); + } + + hangman_check(l, a); + a->a_waiting = l; + + spinlock_release(&hangman_lock); +} + +void +hangman_acquire(struct hangman_actor *a, + struct hangman_lockable *l) +{ + if (l == &hangman_lock.splk_hangman) { + /* don't recurse */ + return; + } + + spinlock_acquire(&hangman_lock); + + if (a->a_waiting != l) { + spinlock_release(&hangman_lock); + panic("hangman_acquire: not waiting for lock %s (%p)\n", + l->l_name, l); + } + if (l->l_holding != NULL) { + spinlock_release(&hangman_lock); + panic("hangman_acquire: lock %s (%p) still held by %s (%p)\n", + l->l_name, l, a->a_name, a); + } + + l->l_holding = a; + a->a_waiting = NULL; + + spinlock_release(&hangman_lock); +} + +void +hangman_release(struct hangman_actor *a, + struct hangman_lockable *l) +{ + if (l == &hangman_lock.splk_hangman) { + /* don't recurse */ + return; + } + + spinlock_acquire(&hangman_lock); + + if (a->a_waiting != NULL) { + spinlock_release(&hangman_lock); + panic("hangman_release: waiting for something?\n"); + } + if (l->l_holding != a) { + spinlock_release(&hangman_lock); + panic("hangman_release: not the holder\n"); + } + + l->l_holding = NULL; + + spinlock_release(&hangman_lock); +} diff --git a/kern/thread/spinlock.c b/kern/thread/spinlock.c index a7b50b1..0cdf9aa 100644 --- a/kern/thread/spinlock.c +++ b/kern/thread/spinlock.c @@ -52,6 +52,7 @@ spinlock_init(struct spinlock *splk) { spinlock_data_set(&splk->splk_lock, 0); splk->splk_holder = NULL; + HANGMAN_LOCKABLEINIT(&splk->splk_hangman, "spinlock"); } /* @@ -85,6 +86,8 @@ spinlock_acquire(struct spinlock *splk) panic("Deadlock on spinlock %p\n", splk); } mycpu->c_spinlocks++; + + HANGMAN_WAIT(&curcpu->c_hangman, &splk->splk_hangman); } else { mycpu = NULL; @@ -112,6 +115,10 @@ spinlock_acquire(struct spinlock *splk) membar_store_any(); splk->splk_holder = mycpu; + + if (CURCPU_EXISTS()) { + HANGMAN_ACQUIRE(&curcpu->c_hangman, &splk->splk_hangman); + } } /* @@ -125,6 +132,7 @@ spinlock_release(struct spinlock *splk) KASSERT(splk->splk_holder == curcpu->c_self); KASSERT(curcpu->c_spinlocks > 0); curcpu->c_spinlocks--; + HANGMAN_RELEASE(&curcpu->c_hangman, &splk->splk_hangman); } splk->splk_holder = NULL; diff --git a/kern/thread/synch.c b/kern/thread/synch.c index d083f4b..ad58934 100644 --- a/kern/thread/synch.c +++ b/kern/thread/synch.c @@ -154,6 +154,8 @@ lock_create(const char *name) return NULL; } + HANGMAN_LOCKABLEINIT(&lock->lk_hangman, lock->lk_name); + // add stuff here as needed return lock; @@ -173,14 +175,23 @@ lock_destroy(struct lock *lock) void lock_acquire(struct lock *lock) { + /* Call this (atomically) before waiting for a lock */ + //HANGMAN_WAIT(&curthread->t_hangman, &lock->lk_hangman); + // Write this (void)lock; // suppress warning until code gets written + + /* Call this (atomically) once the lock is acquired */ + //HANGMAN_ACQUIRE(&curthread->t_hangman, &lock->lk_hangman); } void lock_release(struct lock *lock) { + /* Call this (atomically) when the lock is released */ + //HANGMAN_RELEASE(&curthread->t_hangman, &lock->lk_hangman); + // Write this (void)lock; // suppress warning until code gets written diff --git a/kern/thread/thread.c b/kern/thread/thread.c index a41dc58..e6fe983 100644 --- a/kern/thread/thread.c +++ b/kern/thread/thread.c @@ -140,6 +140,7 @@ thread_create(const char *name) thread->t_context = NULL; thread->t_cpu = NULL; thread->t_proc = NULL; + HANGMAN_ACTORINIT(&thread->t_hangman, thread->t_name); /* Interrupt state fields */ thread->t_in_interrupt = false; @@ -239,6 +240,8 @@ cpu_create(unsigned hardware_number) curcpu->c_curthread = curthread; } + HANGMAN_ACTORINIT(&c->c_hangman, "cpu"); + result = proc_addthread(kproc, c->c_curthread); if (result) { panic("cpu_create: proc_addthread:: %s\n", strerror(result));