6889153 Dubious check in pmcs_unattach
authorJesse Butler <Jesse.Butler@Sun.COM>
Mon, 05 Apr 2010 17:22:47 -0600
changeset 12078 b3cbeb7a9c80
parent 12077 fb344a67eeb9
child 12079 13822b941977
6889153 Dubious check in pmcs_unattach 6936793 VTS diskmediatest causes system panic in pmcs_lock_phy 6855987 pmcs: need to revisit cv_timed_wait() and cv_wait() use 6937422 deadlock panic during storage poweron
usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_attach.c
usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_intr.c
usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_subr.c
usr/src/uts/common/sys/scsi/adapters/pmcs/pmcs.h
usr/src/uts/common/sys/scsi/adapters/pmcs/pmcs_def.h
--- a/usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_attach.c	Mon Apr 05 18:56:56 2010 -0400
+++ b/usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_attach.c	Mon Apr 05 17:22:47 2010 -0600
@@ -501,11 +501,13 @@
 	STAILQ_INIT(&pwp->cq);
 	STAILQ_INIT(&pwp->wf);
 	STAILQ_INIT(&pwp->pf);
+
 	/*
-	 * Create the list for iports
+	 * Create the list for iports and init its lock.
 	 */
 	list_create(&pwp->iports, sizeof (pmcs_iport_t),
 	    offsetof(pmcs_iport_t, list_node));
+	rw_init(&pwp->iports_lock, NULL, RW_DRIVER, NULL);
 
 	pwp->state = STATE_PROBING;
 
@@ -1010,11 +1012,6 @@
 	}
 	pwp->hba_attached = 1;
 
-	/*
-	 * Initialize the rwlock for the iport elements.
-	 */
-	rw_init(&pwp->iports_lock, NULL, RW_DRIVER, NULL);
-
 	/* Check all acc & dma handles allocated in attach */
 	if (pmcs_check_acc_dma_handle(pwp)) {
 		ddi_fm_service_impact(pwp->dip, DDI_SERVICE_LOST);
@@ -1370,24 +1367,6 @@
 		curstate = pwp->state;
 	}
 
-	if (&pwp->iports != NULL) {
-		/* Destroy the iports lock */
-		rw_destroy(&pwp->iports_lock);
-		/* Destroy the iports list */
-		ASSERT(list_is_empty(&pwp->iports));
-		list_destroy(&pwp->iports);
-	}
-
-	if (pwp->hss_iportmap != NULL) {
-		/* Destroy the iportmap */
-		scsi_hba_iportmap_destroy(pwp->hss_iportmap);
-	}
-
-	if (pwp->hss_phymap != NULL) {
-		/* Destroy the phymap */
-		sas_phymap_destroy(pwp->hss_phymap);
-	}
-
 	/*
 	 * Make sure that any pending watchdog won't
 	 * be called from this point on out.
@@ -1450,6 +1429,21 @@
 		pmcs_wr_msgunit(pwp, PMCS_MSGU_OBDB_MASK, 0xffffffff);
 	}
 
+	if (pwp->hss_iportmap != NULL) {
+		/* Destroy the iportmap */
+		scsi_hba_iportmap_destroy(pwp->hss_iportmap);
+	}
+
+	if (pwp->hss_phymap != NULL) {
+		/* Destroy the phymap */
+		sas_phymap_destroy(pwp->hss_phymap);
+	}
+
+	/* Destroy the iports lock and list */
+	rw_destroy(&pwp->iports_lock);
+	ASSERT(list_is_empty(&pwp->iports));
+	list_destroy(&pwp->iports);
+
 	/* Destroy pwp's lock */
 	if (pwp->locks_initted) {
 		mutex_destroy(&pwp->lock);
@@ -1464,6 +1458,7 @@
 #ifdef	DEBUG
 		mutex_destroy(&pwp->dbglock);
 #endif
+		cv_destroy(&pwp->config_cv);
 		cv_destroy(&pwp->ict_cv);
 		cv_destroy(&pwp->drain_cv);
 		pwp->locks_initted = 0;
@@ -2549,6 +2544,7 @@
 #endif
 	cv_init(&pwp->ict_cv, NULL, CV_DRIVER, NULL);
 	cv_init(&pwp->drain_cv, NULL, CV_DRIVER, NULL);
+	cv_init(&pwp->config_cv, NULL, CV_DRIVER, NULL);
 	for (i = 0; i < PMCS_NIQ; i++) {
 		mutex_init(&pwp->iqp_lock[i], NULL,
 		    MUTEX_DRIVER, DDI_INTR_PRI(pwp->intr_pri));
--- a/usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_intr.c	Mon Apr 05 18:56:56 2010 -0400
+++ b/usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_intr.c	Mon Apr 05 17:22:47 2010 -0600
@@ -17,10 +17,9 @@
  * information: Portions Copyright [yyyy] [name of copyright owner]
  *
  * CDDL HEADER END
- *
- *
- * Copyright 2010 Sun Microsystems, Inc.  All rights reserved.
- * Use is subject to license terms.
+ */
+/*
+ * Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
  */
 
 /*
@@ -1468,17 +1467,23 @@
 {
 	pmcs_hw_t	*pwp = (pmcs_hw_t *)arg;
 	uint32_t	avg_nsecs;
+	clock_t		lbolt, ret;
 	pmcs_io_intr_coal_t *ici;
 
 	ici = &pwp->io_intr_coal;
 	mutex_enter(&pwp->ict_lock);
-
 	while (ici->stop_thread == B_FALSE) {
 		/*
 		 * Wait for next time quantum... collect stats
 		 */
-		(void) cv_timedwait(&pwp->ict_cv, &pwp->ict_lock,
-		    ddi_get_lbolt() + ici->quantum);
+		lbolt = ddi_get_lbolt();
+		while (ici->stop_thread == B_FALSE) {
+			ret = cv_timedwait(&pwp->ict_cv, &pwp->ict_lock,
+			    lbolt + ici->quantum);
+			if (ret == -1) {
+				break;
+			}
+		}
 
 		if (ici->stop_thread == B_TRUE) {
 			continue;
--- a/usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_subr.c	Mon Apr 05 18:56:56 2010 -0400
+++ b/usr/src/uts/common/io/scsi/adapters/pmcs/pmcs_subr.c	Mon Apr 05 17:22:47 2010 -0600
@@ -1830,6 +1830,13 @@
 	pmcs_register_dump_int(pwp);
 	mutex_exit(&pwp->lock);
 
+	/* Ensure discovery is not running before we proceed */
+	mutex_enter(&pwp->config_lock);
+	while (pwp->configuring) {
+		cv_wait(&pwp->config_cv, &pwp->config_lock);
+	}
+	mutex_exit(&pwp->config_lock);
+
 	/* Issue soft reset and clean up related softstate */
 	if (pmcs_soft_reset(pwp, B_FALSE)) {
 		/*
@@ -2565,6 +2572,7 @@
 		pwp->blocked = 0;
 	}
 	pwp->configuring = 0;
+	cv_signal(&pwp->config_cv);
 	mutex_exit(&pwp->config_lock);
 
 #ifdef DEBUG
@@ -2588,6 +2596,7 @@
 	pmcs_flush_observations(pwp);
 	mutex_enter(&pwp->config_lock);
 	pwp->configuring = 0;
+	cv_signal(&pwp->config_cv);
 	RESTART_DISCOVERY_LOCKED(pwp);
 	mutex_exit(&pwp->config_lock);
 }
@@ -7067,6 +7076,7 @@
 	 * with them.
 	 */
 	if (queues & PMCS_TGT_ACTIVE_QUEUE) {
+		mutex_exit(&tgt->statlock);
 		mutex_enter(&tgt->aqlock);
 		sp = STAILQ_FIRST(&tgt->aq);
 		while (sp) {
@@ -7093,7 +7103,6 @@
 			    "target 0x%p", __func__, (void *)sp, sp->cmd_tag,
 			    (void *)tgt);
 			mutex_exit(&tgt->aqlock);
-			mutex_exit(&tgt->statlock);
 			/*
 			 * Mark the work structure as dead and complete it
 			 */
@@ -7106,10 +7115,10 @@
 			STAILQ_INSERT_TAIL(&pwp->cq, sp, cmd_next);
 			mutex_exit(&pwp->cq_lock);
 			mutex_enter(&tgt->aqlock);
-			mutex_enter(&tgt->statlock);
 			sp = sp_next;
 		}
 		mutex_exit(&tgt->aqlock);
+		mutex_enter(&tgt->statlock);
 	}
 
 	if (queues & PMCS_TGT_SPECIAL_QUEUE) {
--- a/usr/src/uts/common/sys/scsi/adapters/pmcs/pmcs.h	Mon Apr 05 18:56:56 2010 -0400
+++ b/usr/src/uts/common/sys/scsi/adapters/pmcs/pmcs.h	Mon Apr 05 17:22:47 2010 -0600
@@ -610,23 +610,26 @@
 	/*
 	 * Discovery-related items.
 	 * config_lock: Protects config_changed and should never be held
-	 * outside of getting or setting the value of config_changed.
+	 * outside of getting or setting the value of config_changed or
+	 * configuring.
 	 * config_changed: Boolean indicating whether discovery needs to
 	 * be restarted.
 	 * configuring: 1 = discovery is running, 0 = discovery not running.
 	 * NOTE: configuring is now in the bitfield above.
-	 *
 	 * config_restart_time is set by the tgtmap_[de]activate callbacks each
 	 * time we decide we want SCSA to retry enumeration on some device.
 	 * The watchdog timer will not fire discovery unless it has reached
 	 * config_restart_time and config_restart is TRUE.  This ensures that
 	 * we don't ask SCSA to retry enumerating devices while it is still
 	 * running.
+	 * config_cv can be used by any thread waiting on the configuring
+	 * bit to clear.
 	 */
 	kmutex_t		config_lock;
 	volatile boolean_t	config_changed;
 	boolean_t		config_restart;
 	clock_t			config_restart_time;
+	kcondvar_t		config_cv;
 
 	/*
 	 * Work Related Stuff
--- a/usr/src/uts/common/sys/scsi/adapters/pmcs/pmcs_def.h	Mon Apr 05 18:56:56 2010 -0400
+++ b/usr/src/uts/common/sys/scsi/adapters/pmcs/pmcs_def.h	Mon Apr 05 17:22:47 2010 -0600
@@ -441,12 +441,12 @@
 	((atomic_and_ulong_nv(&hwp->work_flags, (ulong_t)-1) & (1 << wrk)) != 0)
 
 #define	WAIT_FOR(p, t, r)					\
+	clock_t	_lb = ddi_get_lbolt();				\
 	r = 0;							\
 	while (!PMCS_COMMAND_DONE(p)) {				\
-		clock_t tmp = cv_timedwait(&p->sleep_cv,	\
-		    &p->lock, ddi_get_lbolt() +			\
-		    drv_usectohz(t * 1000));			\
-		if (!PMCS_COMMAND_DONE(p) && tmp < 0) {		\
+		clock_t _ret = cv_timedwait(&p->sleep_cv,	\
+		    &p->lock, _lb + drv_usectohz(t * 1000));	\
+		if (!PMCS_COMMAND_DONE(p) && _ret < 0) {		\
 			r = 1;					\
 			break;					\
 		}						\