Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Nov 2006 08:26:41 GMT
From:      Scott Long <scottl@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 110667 for review
Message-ID:  <200611290826.kAT8Qf13036044@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=110667

Change 110667 by scottl@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) {



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