PERFORCE change 110667 for review

Scott Long scottl at FreeBSD.org
Wed Nov 29 00:39:20 PST 2006


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

Change 110667 by scottl at scottl-x64 on 2006/11/29 08:26:07

	When traversing buses and devices, grab the sim lock at the highest
	point, i.e. xptbustraverse(), instead of doing it in disjointed ways
	in the lower layers.  One side effect of this is that async callbacks
	will be called with the sim/bus lock held already.  Another side
	effect is that pass device enumeration that originated in
	xpt_finishedconfig() is now decoupled into a taskqueue.

Affected files ...

.. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#45 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#17 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_pass.c#14 edit

Differences ...

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

@@ -2684,9 +2684,11 @@
 		next_bus = TAILQ_NEXT(bus, links);
 
 		mtx_unlock(&xsoftc.xpt_lock);
+		mtx_lock(bus->sim->mtx);
 		retval = tr_func(bus, arg);
 		if (retval == 0)
 			return(retval);
+		mtx_unlock(bus->sim->mtx);
 		mtx_lock(&xsoftc.xpt_lock);
 	}
 	mtx_unlock(&xsoftc.xpt_lock);
@@ -7004,7 +7006,9 @@
 static int
 xptconfigbuscountfunc(struct cam_eb *bus, void *arg)
 {
-	mtx_lock(bus->sim->mtx);
+
+	mtx_assert(bus->sim->mtx, MA_OWNED);
+
 	if (bus->path_id != CAM_XPT_PATH_ID) {
 		struct cam_path path;
 		struct ccb_pathinq cpi;
@@ -7023,7 +7027,6 @@
 			busses_to_reset++;
 		xpt_release_path(&path);
 	}
-	mtx_unlock(bus->sim->mtx);
 
 	return(1);
 }
@@ -7034,7 +7037,8 @@
 	struct	cam_path *path;
 	union	ccb *work_ccb;
 
-	mtx_lock(bus->sim->mtx);
+	mtx_assert(bus->sim->mtx, MA_OWNED);
+
 	if (bus->path_id != CAM_XPT_PATH_ID) {
 		cam_status status;
 		int can_negotiate;
@@ -7049,7 +7053,6 @@
 			xpt_free_ccb(work_ccb);
 			busses_to_config--;
 			xpt_finishconfig(xpt_periph, NULL);
-			mtx_unlock(bus->sim->mtx);
 			return(0);
 		}
 		xpt_setup_ccb(&work_ccb->ccb_h, path, /*priority*/1);
@@ -7060,7 +7063,6 @@
 			       "with status %d\n", bus->path_id,
 			       work_ccb->ccb_h.status);
 			xpt_finishconfig(xpt_periph, work_ccb);
-			mtx_unlock(bus->sim->mtx);
 			return(1);
 		}
 
@@ -7082,7 +7084,6 @@
 		}
 	}
 
-	mtx_unlock(bus->sim->mtx);
 	return(1);
 }
 
@@ -7157,11 +7158,40 @@
 }
 
 static void
-xpt_finishconfig(struct cam_periph *periph, union ccb *done_ccb)
+xpt_finishconfig_task(void *context, int pending)
 {
 	struct	periph_driver **p_drv;
 	int	i;
 
+	if (busses_to_config == 0) {
+		/* Register all the peripheral drivers */
+		/* XXX This will have to change when we have loadable modules */
+		p_drv = periph_drivers;
+		for (i = 0; p_drv[i] != NULL; i++) {
+			(*p_drv[i]->init)();
+		}
+
+		/*
+		 * Check for devices with no "standard" peripheral driver
+		 * attached.  For any devices like that, announce the
+		 * passthrough driver so the user will see something.
+		 */
+		xpt_for_all_devices(xptpassannouncefunc, NULL);
+
+		/* Release our hook so that the boot can continue. */
+		config_intrhook_disestablish(xsoftc.xpt_config_hook);
+		free(xsoftc.xpt_config_hook, M_TEMP);
+		xsoftc.xpt_config_hook = NULL;
+	}
+
+	free(context, M_CAMXPT);
+}
+
+static void
+xpt_finishconfig(struct cam_periph *periph, union ccb *done_ccb)
+{
+	struct	task *task;
+
 	if (done_ccb != NULL) {
 		CAM_DEBUG(done_ccb->ccb_h.path, CAM_DEBUG_TRACE,
 			  ("xpt_finishconfig\n"));
@@ -7183,26 +7213,12 @@
 		}
 	}
 
-	if (busses_to_config == 0) {
-		/* Register all the peripheral drivers */
-		/* XXX This will have to change when we have loadable modules */
-		p_drv = periph_drivers;
-		for (i = 0; p_drv[i] != NULL; i++) {
-			(*p_drv[i]->init)();
-		}
+	task = malloc(sizeof(struct task), M_CAMXPT, M_NOWAIT);
+	if (task != NULL) {
+		TASK_INIT(task, 0, xpt_finishconfig_task, task);
+		taskqueue_enqueue(taskqueue_thread, task);
+	}
 
-		/*
-		 * Check for devices with no "standard" peripheral driver
-		 * attached.  For any devices like that, announce the
-		 * passthrough driver so the user will see something.
-		 */
-		xpt_for_all_devices(xptpassannouncefunc, NULL);
-
-		/* Release our hook so that the boot can continue. */
-		config_intrhook_disestablish(xsoftc.xpt_config_hook);
-		free(xsoftc.xpt_config_hook, M_TEMP);
-		xsoftc.xpt_config_hook = NULL;
-	}
 	if (done_ccb != NULL)
 		xpt_free_ccb(done_ccb);
 }

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

@@ -974,13 +974,11 @@
 		 * process.
 		 */
 		sim = xpt_path_sim(cgd->ccb_h.path);
-		mtx_lock(sim->mtx);
 		status = cam_periph_alloc(daregister, daoninvalidate,
 					  dacleanup, dastart,
 					  "da", CAM_PERIPH_BIO,
 					  cgd->ccb_h.path, daasync,
 					  AC_FOUND_DEVICE, cgd);
-		mtx_unlock(sim->mtx);
 
 		if (status != CAM_REQ_CMP
 		 && status != CAM_REQ_INPROG)

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

@@ -224,12 +224,10 @@
 		 * process.
 		 */
 		sim = xpt_path_sim(cgd->ccb_h.path);
-		mtx_lock(sim->mtx);
 		status = cam_periph_alloc(passregister, passoninvalidate,
 					  passcleanup, passstart, "pass",
 					  CAM_PERIPH_BIO, cgd->ccb_h.path,
 					  passasync, AC_FOUND_DEVICE, cgd);
-		mtx_unlock(sim->mtx);
 
 		if (status != CAM_REQ_CMP
 		 && status != CAM_REQ_INPROG) {


More information about the p4-projects mailing list