Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Sep 2012 06:47:59 GMT
From:      Venkat Duvvuru <venkatduvvuru.ml@gmail.com>
To:        freebsd-gnats-submit@FreeBSD.org
Subject:   misc/171838: Possible lock reversal and duplicate locks as reported by Witness
Message-ID:  <201209210647.q8L6lxSI003328@red.freebsd.org>
Resent-Message-ID: <201209210650.q8L6o8gb093183@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         171838
>Category:       misc
>Synopsis:       Possible lock reversal and duplicate locks as reported by Witness
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Sep 21 06:50:07 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Venkat Duvvuru
>Release:        9.0-RELEASE
>Organization:
Emulex
>Environment:
FreeBSD root@xxx FreeBsd 9.0-RELEASE FreeBSD 9.0-RELEASE #12: Thu Aug 23 23:42:39 IST 2012     root@root@xxx FreeBsd:/usr/obj/usr/src/sys/GENERIC  amd64
>Description:
This patch is for Freebsd nic "oce" driver (http://svn.freebsd.org/base/head/sys/dev/oce/)

I am attaching a patch which fixes some issues which were reported by witness. Specifically,
- Calling mtx_init with same name causes WITNESS to report possible lock reversal and duplicate lock issues.
- release mtx_lock before invoking taskqueue_drain.

>How-To-Repeat:

>Fix:
Fix is attached in the form of patch.

Patch attached with submission follows:

Index: oce_if.c
===================================================================
--- oce_if.c	(revision 233768)
+++ oce_if.c	(working copy)
@@ -1816,6 +1816,9 @@
 }
 
 
+/* NOTE : This should only be called holding
+ *        DEVICE_LOCK.
+*/
 static void
 oce_if_deactivate(POCE_SOFTC sc)
 {
@@ -1845,11 +1848,17 @@
 	/* Stop intrs and finish any bottom halves pending */
 	oce_hw_intr_disable(sc);
 
+    /* Since taskqueue_drain takes a Gaint Lock, We should not acquire
+       any other lock. So unlock device lock and require after
+       completing taskqueue_drain.
+    */
+    UNLOCK(&sc->dev_lock);
 	for (i = 0; i < sc->intr_count; i++) {
 		if (sc->intrs[i].tq != NULL) {
 			taskqueue_drain(sc->intrs[i].tq, &sc->intrs[i].task);
 		}
 	}
+    LOCK(&sc->dev_lock);
 
 	/* Delete RX queue in card with flush param */
 	oce_stop_rx(sc);
Index: oce_if.h
===================================================================
--- oce_if.h	(revision 233768)
+++ oce_if.h	(working copy)
@@ -493,7 +493,7 @@
 #define LOCK_CREATE(lock, desc) 		{ \
 	strncpy((lock)->name, (desc), MAX_LOCK_DESC_LEN); \
 	(lock)->name[MAX_LOCK_DESC_LEN] = '\0'; \
-	mtx_init(&(lock)->mutex, (lock)->name, MTX_NETWORK_LOCK, MTX_DEF); \
+	mtx_init(&(lock)->mutex, (lock)->name, NULL, MTX_DEF); \
 }
 #define LOCK_DESTROY(lock) 			\
 		if (mtx_initialized(&(lock)->mutex))\


>Release-Note:
>Audit-Trail:
>Unformatted:



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