Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Jul 2008 11:03:59 GMT
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 144415 for review
Message-ID:  <200807011103.m61B3x2b079270@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=144415

Change 144415 by hselasky@hselasky_laptop001 on 2008/07/01 11:03:31

	
	To allow USB drivers using the Giant mutex, like ukbd
	and the tty layer (ucom), condition variable functions
	like mtx_sleep() and cv_wait() needs to support the Giant
	mutex. Previously using the Giant mutex with these functions
	resulted in a panic due to an unlock race between the 
	GIANT_DROP macro and the internal mutex unlock in the
	condition variable function. This patch will try to
	resolve that race.

Affected files ...

.. //depot/projects/usb/src/sys/kern/kern_condvar.c#7 edit
.. //depot/projects/usb/src/sys/kern/kern_synch.c#9 edit
.. //depot/projects/usb/src/sys/sys/mutex.h#8 edit

Differences ...

==== //depot/projects/usb/src/sys/kern/kern_condvar.c#7 (text+ko) ====

@@ -123,7 +123,7 @@
 	sleepq_lock(cvp);
 
 	cvp->cv_waiters++;
-	DROP_GIANT();
+	DROP_GIANT(lock);
 
 	sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0);
 	if (class->lc_flags & LC_SLEEPABLE)
@@ -176,7 +176,7 @@
 	sleepq_lock(cvp);
 
 	cvp->cv_waiters++;
-	DROP_GIANT();
+	DROP_GIANT(lock);
 
 	sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0);
 	if (class->lc_flags & LC_SLEEPABLE)
@@ -233,7 +233,7 @@
 	sleepq_lock(cvp);
 
 	cvp->cv_waiters++;
-	DROP_GIANT();
+	DROP_GIANT(lock);
 
 	sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR |
 	    SLEEPQ_INTERRUPTIBLE, 0);
@@ -293,7 +293,7 @@
 	sleepq_lock(cvp);
 
 	cvp->cv_waiters++;
-	DROP_GIANT();
+	DROP_GIANT(lock);
 
 	sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0);
 	sleepq_set_timeout(cvp, timo);
@@ -356,7 +356,7 @@
 	sleepq_lock(cvp);
 
 	cvp->cv_waiters++;
-	DROP_GIANT();
+	DROP_GIANT(lock);
 
 	sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR |
 	    SLEEPQ_INTERRUPTIBLE, 0);

==== //depot/projects/usb/src/sys/kern/kern_synch.c#9 (text+ko) ====

@@ -181,7 +181,7 @@
 	CTR5(KTR_PROC, "sleep: thread %ld (pid %ld, %s) on %s (%p)",
 	    td->td_tid, p->p_pid, td->td_name, wmesg, ident);
 
-	DROP_GIANT();
+	DROP_GIANT(lock);
 	if (lock != NULL && !(class->lc_flags & LC_SLEEPABLE)) {
 		WITNESS_SAVE(lock, lock_witness);
 		lock_state = class->lc_unlock(lock);

==== //depot/projects/usb/src/sys/sys/mutex.h#8 (text+ko) ====

@@ -366,17 +366,44 @@
  *
  * Note that DROP_GIANT*() needs to be paired with PICKUP_GIANT() 
  * The #ifndef is to allow lint-like tools to redefine DROP_GIANT.
+ *
+ * Note that by default DROP_GIANT takes no argument. Optionally you
+ * can specify an argument which explicitly has the name "lock" and
+ * type "struct lock_object *". If this "lock" pointer is equal to
+ * "&Giant", the DROP_GIANT macro will not do the final drop on the
+ * Giant mutex, but expects the calling code to do so. This feature is
+ * used by condition variables to allow sleeping on Giant. The
+ * condition variable code will then do the final drop!
  */
 #ifndef DROP_GIANT
-#define DROP_GIANT()							\
+#define	DROP_GIANT(arg) DROP_GIANT_SUB_##arg(arg)
+#define	DROP_GIANT_SUB_lock(arg) DROP_GIANT_SUB(arg) /* "lock" argument */
+#define	DROP_GIANT_SUB_(arg) DROP_GIANT_SUB(NULL) /* no argument */
+#define	DROP_GIANT_SUB(lock)						\
 do {									\
-	int _giantcnt = 0;						\
+	unsigned int _giantcnt;						\
 	WITNESS_SAVE_DECL(Giant);					\
 									\
 	if (mtx_owned(&Giant)) {					\
-		WITNESS_SAVE(&Giant.lock_object, Giant);		\
-		for (_giantcnt = 0; mtx_owned(&Giant); _giantcnt++)	\
-			mtx_unlock(&Giant);				\
+		unsigned int _giantn;					\
+		if (((void *)(lock)) == ((void *)&Giant)) {		\
+			/* special case */				\
+			_giantn = Giant.mtx_recurse;			\
+		} else {						\
+			/* default case */				\
+			_giantn = Giant.mtx_recurse + 1;		\
+		}							\
+		if (_giantn != 0) {					\
+			WITNESS_SAVE(&Giant.lock_object, Giant);	\
+			_giantcnt = _giantn;				\
+			do {						\
+				mtx_unlock(&Giant);			\
+			} while (--_giantn);				\
+		} else {						\
+			_giantcnt = 0;					\
+		}							\
+	} else {							\
+		_giantcnt = 0;						\
 	}
 
 #define PICKUP_GIANT()							\



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200807011103.m61B3x2b079270>