Gitweb:
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b533184fc353d4a2d07929b4ac424a6f1bf5a3b9
Commit: b533184fc353d4a2d07929b4ac424a6f1bf5a3b9
Parent: 9135f1901ee6449dfe338adf6e40e9c2025b8150
Author: J. Bruce Fields <[EMAIL PROTECTED]>
AuthorDate: Fri Oct 26 18:05:40 2007 -0400
Committer: J. Bruce Fields <[EMAIL PROTECTED]>
CommitDate: Sun Feb 3 17:51:36 2008 -0500
locks: clarify posix_locks_deadlock
For such a short function (with such a long comment),
posix_locks_deadlock() seems to cause a lot of confusion. Attempt to
make it a bit clearer:
- Remove the initial posix_same_owner() check, which can never
pass (since this is only called in the case that block_fl and
caller_fl conflict)
- Use an explicit loop (and a helper function) instead of a goto.
- Rewrite the comment, attempting a clearer explanation, and
removing some uninteresting historical detail.
Signed-off-by: J. Bruce Fields <[EMAIL PROTECTED]>
---
fs/locks.c | 64 ++++++++++++++++++++++++++++++++++-------------------------
1 files changed, 37 insertions(+), 27 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 8b8388e..c3eecb8 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -683,45 +683,55 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
EXPORT_SYMBOL(posix_test_lock);
-/* This function tests for deadlock condition before putting a process to
- * sleep. The detection scheme is no longer recursive. Recursive was neat,
- * but dangerous - we risked stack corruption if the lock data was bad, or
- * if the recursion was too deep for any other reason.
- *
- * We rely on the fact that a task can only be on one lock's wait queue
- * at a time. When we find blocked_task on a wait queue we can re-search
- * with blocked_task equal to that queue's owner, until either blocked_task
- * isn't found, or blocked_task is found on a queue owned by my_task.
- *
- * Note: the above assumption may not be true when handling lock requests
- * from a broken NFS client. But broken NFS clients have a lot more to
- * worry about than proper deadlock detection anyway... --okir
- *
- * However, the failure of this assumption (also possible in the case of
- * multiple tasks sharing the same open file table) also means there's no
- * guarantee that the loop below will terminate. As a hack, we give up
- * after a few iterations.
+/*
+ * Deadlock detection:
+ *
+ * We attempt to detect deadlocks that are due purely to posix file
+ * locks.
+ *
+ * We assume that a task can be waiting for at most one lock at a time.
+ * So for any acquired lock, the process holding that lock may be
+ * waiting on at most one other lock. That lock in turns may be held by
+ * someone waiting for at most one other lock. Given a requested lock
+ * caller_fl which is about to wait for a conflicting lock block_fl, we
+ * follow this chain of waiters to ensure we are not about to create a
+ * cycle.
+ *
+ * Since we do this before we ever put a process to sleep on a lock, we
+ * are ensured that there is never a cycle; that is what guarantees that
+ * the while() loop in posix_locks_deadlock() eventually completes.
+ *
+ * Note: the above assumption may not be true when handling lock
+ * requests from a broken NFS client. It may also fail in the presence
+ * of tasks (such as posix threads) sharing the same open file table.
+ *
+ * To handle those cases, we just bail out after a few iterations.
*/
#define MAX_DEADLK_ITERATIONS 10
+/* Find a lock that the owner of the given block_fl is blocking on. */
+static struct file_lock *what_owner_is_waiting_for(struct file_lock *block_fl)
+{
+ struct file_lock *fl;
+
+ list_for_each_entry(fl, &blocked_list, fl_link) {
+ if (posix_same_owner(fl, block_fl))
+ return fl->fl_next;
+ }
+ return NULL;
+}
+
static int posix_locks_deadlock(struct file_lock *caller_fl,
struct file_lock *block_fl)
{
- struct file_lock *fl;
int i = 0;
-next_task:
- if (posix_same_owner(caller_fl, block_fl))
- return 1;
- list_for_each_entry(fl, &blocked_list, fl_link) {
- if (posix_same_owner(fl, block_fl)) {
- if (i++ > MAX_DEADLK_ITERATIONS)
- return 0;
- fl = fl->fl_next;
- block_fl = fl;
- goto next_task;
- }
+ while ((block_fl = what_owner_is_waiting_for(block_fl))) {
+ if (i++ > MAX_DEADLK_ITERATIONS)
+ return 0;
+ if (posix_same_owner(caller_fl, block_fl))
+ return 1;
}
return 0;
}
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
