From owner-p4-projects Fri Feb 28 7:57:53 2003 Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id E978837B405; Fri, 28 Feb 2003 07:57:47 -0800 (PST) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 67A2437B401 for ; Fri, 28 Feb 2003 07:57:47 -0800 (PST) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 52EA143FD7 for ; Fri, 28 Feb 2003 07:57:46 -0800 (PST) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.6/8.12.6) with ESMTP id h1SFvk0U047620 for ; Fri, 28 Feb 2003 07:57:46 -0800 (PST) (envelope-from jhb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.6/8.12.6/Submit) id h1SFvj6O047616 for perforce@freebsd.org; Fri, 28 Feb 2003 07:57:45 -0800 (PST) Date: Fri, 28 Feb 2003 07:57:45 -0800 (PST) Message-Id: <200302281557.h1SFvj6O047616@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jhb@freebsd.org using -f From: John Baldwin Subject: PERFORCE change 26076 for review To: Perforce Change Reviews Sender: owner-p4-projects@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG http://perforce.freebsd.org/chv.cgi?CH=26076 Change 26076 by jhb@jhb_laptop on 2003/02/28 07:57:07 - Document special rules about Giant and lock orders. - Properly implement those rules (well, except the first one). Axeing WITNESS_SLEEP broke the case of acquiring Giant after resuming from an msleep while holding a sleepable lock and witness would bogusly whine about it as a result. Affected files ... .. //depot/projects/smpng/sys/kern/subr_witness.c#71 edit Differences ... ==== //depot/projects/smpng/sys/kern/subr_witness.c#71 (text+ko) ==== @@ -56,6 +56,32 @@ * 6 capitalized : a member of the Jehovah's Witnesses */ +/* + * Special rules concerning Giant and lock orders: + * + * 1) Giant must be acquired before any other mutexes. Stated another way, + * no other mutex may be held when Giant is acquired. + * + * 2) Giant must be released when blocking on a sleepable lock. + * + * This rule is less obvious, but is a result of Giant providing the same + * semantics as spl(). Basically, when a thread sleeps, it must release + * Giant. When a thread blocks on a sleepable lock, it sleeps. Hence rule + * 2). + * + * 3) Giant may be acquired before or after sleepable locks. + * + * This rule is also not quite as obvious. Giant may be acquired after + * a sleepable lock because it is a non-sleepable lock and non-sleepable + * locks may always be acquired while holding a sleepable lock. The second + * case, Giant before a sleepable lock, follows from rule 2) above. Suppose + * you have two threads T1 and T2 and a sleepable lock X. Suppose that T1 + * acquires X and blocks on Giant. Then suppose that T2 acquires Giant and + * blocks on X. When T2 blocks on X, T2 will release Giant allowing T1 to + * execute. Thus, acquiring Giant both before and after a sleepable lock + * will not result in a lock order reversal. + */ + #include "opt_ddb.h" #include "opt_witness.h" @@ -636,6 +662,11 @@ mtx_unlock_spin(&w_mtx); goto out; } + /* + * If we know that the the lock we are acquiring comes after + * the lock we most recently acquired in the lock order tree, + * then there is no need for any further checks. + */ if (isitmydescendant(w1, w)) { mtx_unlock_spin(&w_mtx); goto out; @@ -657,24 +688,31 @@ continue; } /* - * If we are locking Giant and we slept with this + * If we are locking Giant and this is a sleepable * lock, then skip it. */ - if ((lock1->li_flags & LI_SLEPT) != 0 && + if ((lock1->li_lock->lo_flags & LO_SLEEPABLE) != 0 && lock == &Giant.mtx_object) continue; /* * If we are locking a sleepable lock and this lock - * isn't sleepable and isn't Giant, we want to treat - * it as a lock order violation to enfore a general - * lock order of sleepable locks before non-sleepable - * locks. Thus, we only bother checking the lock - * order hierarchy if we pass the initial test. + * is Giant, then skip it. + */ + if ((lock->lo_flags & LO_SLEEPABLE) != 0 && + lock1->li_lock != &Giant.mtx_object) + continue; + /* + * If we are locking a sleepable lock and this lock + * isn't sleepable, we want to treat it as a lock + * order violation to enfore a general lock order of + * sleepable locks before non-sleepable locks. */ if (!((lock->lo_flags & LO_SLEEPABLE) != 0 && - ((lock1->li_lock->lo_flags & LO_SLEEPABLE) == 0 && - lock1->li_lock != &Giant.mtx_object)) && - !isitmydescendant(w, w1)) + (lock1->li_lock->lo_flags & LO_SLEEPABLE) == 0)) + /* + * Check the lock order hierarchy for a reveresal. + */ + if (!isitmydescendant(w, w1)) continue; /* * We have a lock order violation, check to see if it To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe p4-projects" in the body of the message