Mercurial > illumos > illumos-gate

changeset 5160:6a35c54999f3

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
6554813 Possible deadlock when enabling/disabling soft rings
author udpa
date Mon, 01 Oct 2007 16:07:13 -0700
parents 6cdd421a2458
children 6118969b7712
files usr/src/uts/common/io/dls/dls.c usr/src/uts/common/io/dls/dls_soft_ring.c usr/src/uts/common/sys/dls_soft_ring.h
diffstat 3 files changed, 81 insertions(+), 44 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/io/dls/dls.c	Mon Oct 01 15:49:09 2007 -0700
+++ b/usr/src/uts/common/io/dls/dls.c	Mon Oct 01 16:07:13 2007 -0700
@@ -308,12 +308,7 @@
 	dip->di_dvp = NULL;
 	dip->di_txinfo = NULL;
 
-	if (dip->di_soft_ring_list != NULL) {
-		soft_ring_set_destroy(dip->di_soft_ring_list,
-		    dip->di_soft_ring_size);
-		dip->di_soft_ring_list = NULL;
-	}
-	dip->di_soft_ring_size = 0;
+	ASSERT(dip->di_soft_ring_list == NULL);
 
 	kmem_cache_free(i_dls_impl_cachep, dip);
 
--- a/usr/src/uts/common/io/dls/dls_soft_ring.c	Mon Oct 01 15:49:09 2007 -0700
+++ b/usr/src/uts/common/io/dls/dls_soft_ring.c	Mon Oct 01 16:07:13 2007 -0700
@@ -69,7 +69,13 @@
 static void soft_ring_fire(void *);
 static void soft_ring_drain(soft_ring_t *, clock_t);
 static void soft_ring_worker(soft_ring_t *);
-static void soft_ring_stop_workers(soft_ring_t **, int);
+static void soft_ring_set_destroy(soft_ring_t **, int);
+static void dls_taskq_stop_soft_ring(void *);
+
+typedef struct soft_ring_taskq {
+	soft_ring_t	**ringp_list;
+	uint_t		ring_size;
+} soft_ring_taskq_t;
 
 kmem_cache_t *soft_ring_cache;
 
@@ -177,38 +183,38 @@
 
 soft_ring_t **
 soft_ring_set_create(char *name, processorid_t bind, clock_t wait,
-    uint_t type, pri_t pri, int cnt)
+    uint_t type, pri_t pri, int ring_size)
 {
 	int 		i;
 	soft_ring_t	**ringp_list;
 
 	if ((ringp_list =
-	    (soft_ring_t **) kmem_zalloc(sizeof (soft_ring_t *) * cnt,
+	    (soft_ring_t **) kmem_zalloc(sizeof (soft_ring_t *) * ring_size,
 	    KM_NOSLEEP)) != NULL) {
-		for (i = 0; i < cnt; i++) {
+		for (i = 0; i < ring_size; i++) {
 			ringp_list[i] = soft_ring_create(name, bind, wait,
 			    type, pri);
 			if (ringp_list[i] == NULL)
 				break;
 		}
-		if (i != cnt) {
-			soft_ring_stop_workers(ringp_list, i);
-			soft_ring_set_destroy(ringp_list, i);
+		if (i != ring_size) {
+			soft_ring_set_destroy(ringp_list, ring_size);
 			ringp_list = NULL;
 		}
 	}
 	return (ringp_list);
 }
 
-static void
-soft_ring_stop_workers(soft_ring_t **ringp_set, int cnt)
+void
+soft_ring_set_destroy(soft_ring_t **ringp_set, int ring_size)
 {
 	int 		i;
+	mblk_t		*mp;
 	soft_ring_t	*ringp;
 	timeout_id_t 	tid;
 	kt_did_t	t_did = 0;
 
-	for (i = 0; i < cnt; i++) {
+	for (i = 0; (i < ring_size) && (ringp_set[i] != NULL); i++) {
 		ringp = ringp_set[i];
 
 		soft_ring_unbind((void *)ringp);
@@ -239,18 +245,6 @@
 		 */
 		if (t_did)
 			thread_join(t_did);
-	}
-}
-
-void
-soft_ring_set_destroy(soft_ring_t **ringp_set, int cnt)
-{
-	int 		i;
-	mblk_t		*mp;
-	soft_ring_t	*ringp;
-
-	for (i = 0; i < cnt; i++) {
-		ringp = ringp_set[i];
 
 		mutex_enter(&ringp->s_ring_lock);
 
@@ -272,7 +266,7 @@
 		kmem_cache_free(soft_ring_cache, ringp);
 		ringp_set[i] = NULL;
 	}
-	kmem_free(ringp_set, sizeof (soft_ring_t *) * cnt);
+	kmem_free(ringp_set, sizeof (soft_ring_t *) * ring_size);
 }
 
 /* ARGSUSED */
@@ -542,7 +536,7 @@
 	dls_impl_t	*dip = (dls_impl_t *)dc;
 	boolean_t ret = B_FALSE;
 
-	rw_enter(&(dip->di_lock), RW_WRITER);
+	rw_enter(&(dip->di_lock), RW_READER);
 	if (dip->di_soft_ring_list != NULL)
 		ret = B_TRUE;
 	rw_exit(&(dip->di_lock));
@@ -553,33 +547,82 @@
 dls_soft_ring_disable(dls_channel_t dc)
 {
 	dls_impl_t	*dip = (dls_impl_t *)dc;
+	soft_ring_t	**ringp_list = NULL;
+	int		ring_size;
 
 	rw_enter(&(dip->di_lock), RW_WRITER);
-	if (dip->di_soft_ring_list != NULL)
-		soft_ring_stop_workers(dip->di_soft_ring_list,
-		    dip->di_soft_ring_size);
+	if (dip->di_soft_ring_list != NULL) {
+		ringp_list = dip->di_soft_ring_list;
+		ring_size = dip->di_soft_ring_size;
+		dip->di_soft_ring_list = NULL;
+	}
+	dip->di_soft_ring_size = 0;
 	rw_exit(&(dip->di_lock));
+
+	if (ringp_list != NULL)
+		soft_ring_set_destroy(ringp_list, ring_size);
+}
+
+static void
+dls_taskq_stop_soft_ring(void *arg)
+{
+	soft_ring_taskq_t	*ring_taskq;
+	soft_ring_t		**ringp_list;
+	int			ring_size;
+
+	ring_taskq = (soft_ring_taskq_t *)arg;
+	ringp_list = ring_taskq->ringp_list;
+	ring_size = ring_taskq->ring_size;
+	kmem_free(ring_taskq, sizeof (soft_ring_taskq_t));
+
+	soft_ring_set_destroy(ringp_list, ring_size);
 }
 
 boolean_t
 dls_soft_ring_enable(dls_channel_t dc, dl_capab_dls_t *soft_ringp)
 {
-	dls_impl_t	*dip;
-	int		i;
-	soft_ring_t	**softring_set;
-	soft_ring_t	*softring;
-	mac_rx_fifo_t	mrf;
-	char		name[64];
+	dls_impl_t		*dip;
+	int			i;
+	soft_ring_t		**softring_set;
+	soft_ring_t		*softring;
+	mac_rx_fifo_t		mrf;
+	soft_ring_taskq_t	*ring_taskq;
+	char			name[64];
 
 	dip = (dls_impl_t *)dc;
 
 	rw_enter(&(dip->di_lock), RW_WRITER);
 
 	if (dip->di_soft_ring_list != NULL) {
-		soft_ring_stop_workers(dip->di_soft_ring_list,
-		    dip->di_soft_ring_size);
-		soft_ring_set_destroy(dip->di_soft_ring_list,
-		    dip->di_soft_ring_size);
+		/*
+		 * Both ds_lock and di_lock are held as writer.
+		 * As soft_ring_set_destroy() blocks for the
+		 * worker thread(s) to complete, there is a possibility
+		 * that the worker thread(s) could be in the process
+		 * of draining the queue and is blocked waiting for
+		 * either ds_lock or di_lock. Moreover the NIC interrupt
+		 * thread could be blocked in dls_accept().
+		 * To avoid deadlock condition, taskq thread would be
+		 * created to handle soft_ring_set_destroy() and
+		 * blocking if required which would avoid holding
+		 * both ds_lock and di_lock.
+		 * NOTE: we cannot drop either locks here, due to
+		 * weird race conditions seen.
+		 */
+		ring_taskq = (soft_ring_taskq_t *)
+		    kmem_zalloc(sizeof (soft_ring_taskq_t), KM_NOSLEEP);
+		if (ring_taskq == NULL) {
+			rw_exit(&(dip->di_lock));
+			return (B_FALSE);
+		}
+		ring_taskq->ringp_list = dip->di_soft_ring_list;
+		ring_taskq->ring_size = dip->di_soft_ring_size;
+		if (taskq_dispatch(system_taskq, dls_taskq_stop_soft_ring,
+		    ring_taskq, TQ_NOSLEEP) == NULL) {
+			rw_exit(&(dip->di_lock));
+			kmem_free(ring_taskq, sizeof (soft_ring_taskq_t));
+			return (B_FALSE);
+		}
 		dip->di_soft_ring_list = NULL;
 	}
 	dip->di_soft_ring_size = 0;
--- a/usr/src/uts/common/sys/dls_soft_ring.h	Mon Oct 01 15:49:09 2007 -0700
+++ b/usr/src/uts/common/sys/dls_soft_ring.h	Mon Oct 01 16:07:13 2007 -0700
@@ -82,7 +82,6 @@
     uint_t, pri_t);
 extern soft_ring_t **soft_ring_set_create(char *, processorid_t, clock_t,
     uint_t, pri_t, int);
-extern void soft_ring_set_destroy(soft_ring_t **, int);
 extern void soft_ring_bind(void *, processorid_t);
 extern void soft_ring_unbind(void *);
 extern void dls_soft_ring_fanout(void *, void *, mblk_t *, mac_header_info_t *);