PERFORCE change 99528 for review

Scott Long scottl at FreeBSD.org
Sun Jun 18 15:23:53 UTC 2006


http://perforce.freebsd.org/chv.cgi?CH=99528

Change 99528 by scottl at scottl-wv1u on 2006/06/18 15:23:05

	Start working out the periph locking API:
	
	cam_periph_lock: Take a hard reference on the SIM via an interlock
		with the XPT.  Returns with the SIM lock held.
	cam_periph_unlock: removes a reference and returns with the SIM
		unlocked.
	cam_periph_acquire: Take a reference on the SIM.  Requires the SIM
		lock to already be held.
	cam_periph_release: Removes a reference a releases the periph if the
		refcount reaches 0.  Requires the SIM lock to already be held.
	
	Adapt the da, pass, and ses drivers to this API.  Also, this codifies
	the lock order as xpt_lock first and sim lock second, so rearrange
	locking in the camisr to honor this.

Affected files ...

.. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#11 edit
.. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#38 edit
.. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.h#4 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#11 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_pass.c#9 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ses.c#7 edit

Differences ...

==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#11 (text+ko) ====

@@ -51,6 +51,7 @@
 #include <cam/cam_xpt_periph.h>
 #include <cam/cam_periph.h>
 #include <cam/cam_debug.h>
+#include <cam/cam_sim.h>
 
 #include <cam/scsi/scsi_all.h>
 #include <cam/scsi/scsi_message.h>
@@ -277,6 +278,7 @@
 		return(CAM_REQ_CMP_ERR);
 
 	s = splsoftcam();
+	mtx_assert(periph->sim->mtx, MA_OWNED);
 	periph->refcount++;
 	splx(s);
 
@@ -292,6 +294,7 @@
 		return;
 
 	s = splsoftcam();
+	mtx_assert(periph->sim->mtx, MA_OWNED);
 	if ((--periph->refcount == 0)
 	 && (periph->flags & CAM_PERIPH_INVALID)) {
 		camperiphfree(periph);
@@ -500,23 +503,25 @@
 int
 cam_periph_lock(struct cam_periph *periph, int priority)
 {
-	int error;
+
+	/*
+	 * Interlock the SIM lock with the XPT lock to protect against the
+	 * SIM going away unexpectedly while we are trying reference it.
+	 */
+	xpt_lock_buses();
+
+	/*
+	 * Now it's safe to take the SIM lock.
+	 */
+	mtx_lock(periph->sim->mtx);
+	xpt_unlock_buses();
 
 	/*
-	 * Increment the reference count on the peripheral
-	 * while we wait for our lock attempt to succeed
-	 * to ensure the peripheral doesn't disappear out
-	 * from under us while we sleep.
+	 * Increment the reference count on the peripheral.
 	 */
-	if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+	if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
+		mtx_unlock(periph->sim->mtx);
 		return(ENXIO);
-
-	while ((periph->flags & CAM_PERIPH_LOCKED) != 0) {
-		periph->flags |= CAM_PERIPH_LOCK_WANTED;
-		if ((error = tsleep(periph, priority, "caplck", 0)) != 0) {
-			cam_periph_release(periph);
-			return error;
-		}
 	}
 
 	periph->flags |= CAM_PERIPH_LOCKED;
@@ -530,12 +535,9 @@
 cam_periph_unlock(struct cam_periph *periph)
 {
 	periph->flags &= ~CAM_PERIPH_LOCKED;
-	if ((periph->flags & CAM_PERIPH_LOCK_WANTED) != 0) {
-		periph->flags &= ~CAM_PERIPH_LOCK_WANTED;
-		wakeup(periph);
-	}
 
 	cam_periph_release(periph);
+	mtx_unlock(periph->sim->mtx);
 }
 
 /*

==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#38 (text+ko) ====

@@ -7181,6 +7181,18 @@
 {
 }
 
+void
+xpt_lock_buses(void)
+{
+	mtx_lock(&xsoftc.xpt_lock);
+}
+
+void
+xpt_unlock_buses(void)
+{
+	mtx_unlock(&xsoftc.xpt_lock);
+}
+
 static void
 camisr(void *V_queue)
 {
@@ -7205,8 +7217,6 @@
 		int	runq;
 
 		TAILQ_REMOVE(&queue, ccb_h, sim_links.tqe);
-		sim = ccb_h->path->bus->sim;
-		mtx_lock(sim->mtx);
 		ccb_h->pinfo.index = CAM_UNQUEUED_INDEX;
 		splx(s);
 
@@ -7228,7 +7238,6 @@
 			 * Increment the count since this command is done.
 			 */
 			xsoftc.num_highpower++;
-			mtx_unlock(&xsoftc.xpt_lock);
 
 			/* 
 			 * Any high powered commands queued up?
@@ -7236,11 +7245,17 @@
 			if (send_ccb != NULL) {
 
 				STAILQ_REMOVE_HEAD(hphead, xpt_links.stqe);
+				mtx_unlock(&xsoftc.xpt_lock);
 
 				xpt_release_devq(send_ccb->ccb_h.path,
 						 /*count*/1, /*runqueue*/TRUE);
-			}
+			} else
+				mtx_unlock(&xsoftc.xpt_lock);
 		}
+
+		sim = ccb_h->path->bus->sim;
+		mtx_lock(sim->mtx);
+
 		if ((ccb_h->func_code & XPT_FC_USER_CCB) == 0) {
 			struct cam_ed *dev;
 

==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.h#4 (text+ko) ====

@@ -71,6 +71,8 @@
 struct cam_periph	*xpt_path_periph(struct cam_path *path);
 void			xpt_async(u_int32_t async_code, struct cam_path *path,
 				  void *async_arg);
+void			xpt_lock_buses(void);
+void			xpt_unlock_buses(void);
 #endif /* _KERNEL */
 
 #endif /* _CAM_CAM_XPT_H */

==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#11 (text+ko) ====

@@ -531,11 +531,6 @@
 		splx(s);
 		return (ENXIO);	
 	}
-	mtx_lock(periph->sim->mtx);
-
-	unit = periph->unit_number;
-
-	softc = (struct da_softc *)periph->softc;
 
 	CAM_DEBUG(periph->path, CAM_DEBUG_TRACE,
 	    ("daopen: disk=%s%d (unit %d)\n", dp->d_name, dp->d_unit,
@@ -548,6 +543,9 @@
 		mtx_unlock(periph->sim->mtx);
 		return(ENXIO);
 	}
+
+	unit = periph->unit_number;
+	softc = (struct da_softc *)periph->softc;
 	softc->flags |= DA_FLAG_OPEN;
 
 	if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) {
@@ -578,7 +576,6 @@
 		cam_periph_release(periph);
 	}
 	cam_periph_unlock(periph);
-	mtx_unlock(periph->sim->mtx);
 	return (error);
 }
 
@@ -593,14 +590,12 @@
 	if (periph == NULL)
 		return (ENXIO);	
 
-	mtx_lock(periph->sim->mtx);
-	softc = (struct da_softc *)periph->softc;
-
 	if ((error = cam_periph_lock(periph, PRIBIO)) != 0) {
-		mtx_unlock(periph->sim->mtx);
 		return (error); /* error code from tsleep */
 	}
 
+	softc = (struct da_softc *)periph->softc;
+
 	if ((softc->quirks & DA_Q_NO_SYNC_CACHE) == 0) {
 		union	ccb *ccb;
 
@@ -665,7 +660,6 @@
 	softc->flags &= ~DA_FLAG_OPEN;
 	cam_periph_unlock(periph);
 	cam_periph_release(periph);
-	mtx_unlock(periph->sim->mtx);
 	return (0);	
 }
 
@@ -1201,7 +1195,7 @@
 	 * Lock this peripheral until we are setup.
 	 * This first call can't block
 	 */
-	(void)cam_periph_lock(periph, PRIBIO);
+	(void)cam_periph_acquire(periph);
 	xpt_schedule(periph, /*priority*/5);
 
 	return(CAM_REQ_CMP);
@@ -1661,7 +1655,7 @@
 		 * operation.
 		 */
 		xpt_release_ccb(done_ccb);
-		cam_periph_unlock(periph);
+		cam_periph_release(periph);
 		return;
 	}
 	case DA_CCB_WAITING:

==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_pass.c#9 (text+ko) ====

@@ -332,13 +332,17 @@
 	if (periph == NULL)
 		return (ENXIO);
 
+	if ((error = cam_periph_lock(periph, PRIBIO | PCATCH)) != 0) {
+		splx(s);
+		return (error);
+	}
+
 	softc = (struct pass_softc *)periph->softc;
 
 	s = splsoftcam();
-	mtx_lock(periph->sim->mtx);
 	if (softc->flags & PASS_FLAG_INVALID) {
 		splx(s);
-		mtx_unlock(periph->sim->mtx);
+		cam_periph_unlock(periph);
 		return(ENXIO);
 	}
 
@@ -348,7 +352,7 @@
 	error = securelevel_gt(td->td_ucred, 1);
 	if (error) {
 		splx(s);
-		mtx_unlock(periph->sim->mtx);
+		cam_periph_unlock(periph);
 		return(error);
 	}
 
@@ -357,7 +361,7 @@
 	 */
 	if (((flags & FWRITE) == 0) || ((flags & FREAD) == 0)) {
 		splx(s);
-		mtx_unlock(periph->sim->mtx);
+		cam_periph_unlock(periph);
 		return(EPERM);
 	}
 
@@ -368,26 +372,21 @@
 		xpt_print_path(periph->path);
 		printf("can't do nonblocking accesss\n");
 		splx(s);
-		mtx_unlock(periph->sim->mtx);
+		cam_periph_unlock(periph);
 		return(EINVAL);
 	}
 
-	if ((error = cam_periph_lock(periph, PRIBIO | PCATCH)) != 0) {
-		splx(s);
-		mtx_unlock(periph->sim->mtx);
-		return (error);
-	}
-
 	splx(s);
 
 	if ((softc->flags & PASS_FLAG_OPEN) == 0) {
-		if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+		if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
+			cam_periph_unlock(periph);
 			return(ENXIO);
+		}
 		softc->flags |= PASS_FLAG_OPEN;
 	}
 
 	cam_periph_unlock(periph);
-	mtx_unlock(periph->sim->mtx);
 
 	return (error);
 }
@@ -403,19 +402,15 @@
 	if (periph == NULL)
 		return (ENXIO);	
 
-	softc = (struct pass_softc *)periph->softc;
-
-	mtx_lock(periph->sim->mtx);
 	if ((error = cam_periph_lock(periph, PRIBIO)) != 0) {
-		mtx_unlock(periph->sim->mtx);
 		return (error);
 	}
 
+	softc = (struct pass_softc *)periph->softc;
 	softc->flags &= ~PASS_FLAG_OPEN;
 
 	cam_periph_unlock(periph);
 	cam_periph_release(periph);
-	mtx_unlock(periph->sim->mtx);
 
 	return (0);
 }

==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ses.c#7 (text+ko) ====

@@ -420,17 +420,14 @@
 		splx(s);
 		return (ENXIO);
 	}
-	mtx_lock(periph->sim->mtx);
 	if ((error = cam_periph_lock(periph, PRIBIO | PCATCH)) != 0) {
 		splx(s);
-		mtx_unlock(periph->sim->mtx);
 		return (error);
 	}
 	splx(s);
 
 	if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
 		cam_periph_unlock(periph);
-		mtx_unlock(periph->sim->mtx);
 		return (ENXIO);
 	}
 
@@ -463,7 +460,6 @@
 		cam_periph_release(periph);
 	}
 	cam_periph_unlock(periph);
-	mtx_unlock(periph->sim->mtx);
 	return (error);
 }
 
@@ -480,19 +476,15 @@
 	if (periph == NULL)
 		return (ENXIO);
 
-	softc = (struct ses_softc *)periph->softc;
-
-	mtx_lock(periph->sim->mtx);
 	if ((error = cam_periph_lock(periph, PRIBIO)) != 0) {
-		mtx_unlock(periph->sim->mtx);
 		return (error);
 	}
 
+	softc = (struct ses_softc *)periph->softc;
 	softc->ses_flags &= ~SES_FLAG_OPEN;
 
 	cam_periph_unlock(periph);
 	cam_periph_release(periph);
-	mtx_unlock(periph->sim->mtx);
 
 	return (0);
 }


More information about the p4-projects mailing list