Date: Fri, 28 Feb 2003 07:57:45 -0800 (PST) From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 26076 for review Message-ID: <200302281557.h1SFvj6O047616@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200302281557.h1SFvj6O047616>