svn commit: r245228 - head/sys/cam/ctl

Kenneth D. Merry ken at FreeBSD.org
Wed Jan 9 17:02:09 UTC 2013


Author: ken
Date: Wed Jan  9 17:02:08 2013
New Revision: 245228
URL: http://svnweb.freebsd.org/changeset/base/245228

Log:
  Make CTL work a little better with loading and unloading drivers.
  
  Previously CTL would leave individual LUNs enabled in the target
  driver, whether or not the port as a whole was enabled.  It would
  also leave the wildcard LUN enabled indefinitely.
  
  This change means that CTL will enable and disable any active LUNs,
  as well as the wildcard LUN, when enabling and disabling a port.
  
  Also, fix a bug that could crop up due to an uninitialized CCB
  type.
  
  ctl.c:		Before calling ctl_frontend_online(), run through
  		the LUN list and enable all active LUNs.
  
  		After calling ctl_frontend_offline(), run through
  		the LUN list and disble all active LUNs.
  
  scsi_ctl.c:	Before bringing a port online, allocate the
  		wildcard peripheral for that bus.  And after taking
  		a port offline, invalidate the wildcard peripheral
  		for that bus.
  
  		Make sure that we hold the SIM lock around all
  		calls to xpt_action() and other transport layer
  		interfaces that require it.
  
  		Use CAM_SIM_{LOCK|UNLOCK} consistently to acquire
  		and release the SIM lock.
  
  		Update a number of outdated comments.  Some of
  		these should have been fixed long ago.
  
  		Actually do LUN disbables now.  The newer drivers
  		in the tree work correctly for this as far as I
  		know.
  
  		Initialize the CCB type to CTLFE_CCB_DEFAULT to
  		avoid a panic due to uninitialized memory.
  
  Submitted by:	Chuck Tuffli (partially)
  MFC after:	1 week

Modified:
  head/sys/cam/ctl/ctl.c
  head/sys/cam/ctl/scsi_ctl.c

Modified: head/sys/cam/ctl/ctl.c
==============================================================================
--- head/sys/cam/ctl/ctl.c	Wed Jan  9 16:51:47 2013	(r245227)
+++ head/sys/cam/ctl/ctl.c	Wed Jan  9 17:02:08 2013	(r245228)
@@ -2257,11 +2257,31 @@ ctl_ioctl(struct cdev *dev, u_long cmd, 
 				 */
 				mtx_unlock(&softc->ctl_lock);
 
-				if (cmd == CTL_ENABLE_PORT)
+				if (cmd == CTL_ENABLE_PORT) {
+					struct ctl_lun *lun;
+
+					STAILQ_FOREACH(lun, &softc->lun_list,
+						       links) {
+						fe->lun_enable(fe->targ_lun_arg,
+						    lun->target,
+						    lun->lun);
+					}
+
 					ctl_frontend_online(fe);
-				else if (cmd == CTL_DISABLE_PORT)
+				} else if (cmd == CTL_DISABLE_PORT) {
+					struct ctl_lun *lun;
+
 					ctl_frontend_offline(fe);
 
+					STAILQ_FOREACH(lun, &softc->lun_list,
+						       links) {
+						fe->lun_disable(
+						    fe->targ_lun_arg,
+						    lun->target,
+						    lun->lun);
+					}
+				}
+
 				mtx_lock(&softc->ctl_lock);
 
 				if (cmd == CTL_SET_PORT_WWNS)

Modified: head/sys/cam/ctl/scsi_ctl.c
==============================================================================
--- head/sys/cam/ctl/scsi_ctl.c	Wed Jan  9 16:51:47 2013	(r245227)
+++ head/sys/cam/ctl/scsi_ctl.c	Wed Jan  9 17:02:08 2013	(r245228)
@@ -73,6 +73,7 @@ __FBSDID("$FreeBSD$");
 #include <cam/ctl/ctl_error.h>
 
 typedef enum {
+	CTLFE_CCB_DEFAULT	= 0x00,
 	CTLFE_CCB_WAITING 	= 0x01
 } ctlfe_ccb_types;
 
@@ -304,10 +305,7 @@ ctlfeasync(void *callback_arg, uint32_t 
 	case AC_PATH_REGISTERED: {
 		struct ctl_frontend *fe;
 		struct ctlfe_softc *bus_softc;
-		struct ctlfe_lun_softc *lun_softc;
-		struct cam_path *path;
 		struct ccb_pathinq *cpi;
-		cam_status status;
 		int retval;
 
 		cpi = (struct ccb_pathinq *)arg;
@@ -330,7 +328,6 @@ ctlfeasync(void *callback_arg, uint32_t 
 						  M_NOWAIT | M_ZERO);
 			if (ccb == NULL) {
 				printf("%s: unable to malloc CCB!\n", __func__);
-				xpt_free_path(path);
 				return;
 			}
 			xpt_setup_ccb(&ccb->ccb_h, cpi->ccb_h.path,
@@ -448,44 +445,31 @@ ctlfeasync(void *callback_arg, uint32_t 
 			mtx_unlock(&ctlfe_list_mtx);
 		}
 
-		status = xpt_create_path(&path, /*periph*/ NULL,
-					 bus_softc->path_id,CAM_TARGET_WILDCARD,
-					 CAM_LUN_WILDCARD);
-		if (status != CAM_REQ_CMP) {
-			printf("%s: unable to create path for wildcard "
-			       "periph\n", __func__);
-			break;
-		}
-		lun_softc = malloc(sizeof(*lun_softc), M_CTLFE,
-				   M_NOWAIT | M_ZERO);
-		if (lun_softc == NULL) {
-			xpt_print(path, "%s: unable to allocate softc for "
-				  "wildcard periph\n", __func__);
-			xpt_free_path(path);
-			break;
-		}
-
-		lun_softc->parent_softc = bus_softc;
-		lun_softc->flags |= CTLFE_LUN_WILDCARD;
-
-		status = cam_periph_alloc(ctlferegister,
-					  ctlfeoninvalidate,
-					  ctlfecleanup,
-					  ctlfestart,
-					  "ctl",
-					  CAM_PERIPH_BIO,
-					  path,
-					  ctlfeasync,
-					  0,
-					  lun_softc);
+		break;
+	}
+	case AC_PATH_DEREGISTERED: {
+		struct ctlfe_softc *softc = NULL;
 
-		xpt_free_path(path);
+		mtx_lock(&ctlfe_list_mtx);
+		STAILQ_FOREACH(softc, &ctlfe_softc_list, links) {
+			if (softc->path_id == xpt_path_path_id(path)) {
+				STAILQ_REMOVE(&ctlfe_softc_list, softc,
+						ctlfe_softc, links);
+				break;
+			}
+		}
+		mtx_unlock(&ctlfe_list_mtx);
 
+		if (softc != NULL) {
+			/*
+			 * XXX KDM are we certain at this point that there
+			 * are no outstanding commands for this frontend?
+			 */
+			ctl_frontend_deregister(&softc->fe);
+			free(softc, M_CTLFE);
+		}
 		break;
 	}
-	case AC_PATH_DEREGISTERED:
-		/* ctl_frontend_deregister() */
-		break;
 	case AC_CONTRACT: {
 		struct ac_contract *ac;
 
@@ -699,11 +683,14 @@ ctlfecleanup(struct cam_periph *periph)
 	softc = (struct ctlfe_lun_softc *)periph->softc;
 	bus_softc = softc->parent_softc;
 
-	STAILQ_REMOVE(&bus_softc->lun_softc_list, softc, ctlfe_lun_softc,links);
+	STAILQ_REMOVE(&bus_softc->lun_softc_list, softc, ctlfe_lun_softc, links);
 	
 	/*
 	 * XXX KDM is there anything else that needs to be done here?
 	 */
+
+	callout_stop(&softc->dma_callout);
+
 	free(softc, M_CTLFE);
 }
 
@@ -717,6 +704,8 @@ ctlfestart(struct cam_periph *periph, un
 
 	softc->ccbs_alloced++;
 
+	start_ccb->ccb_h.ccb_type = CTLFE_CCB_DEFAULT;
+
 	ccb_h = TAILQ_FIRST(&softc->work_queue);
 	if (periph->immediate_priority <= periph->pinfo.priority) {
 		panic("shouldn't get to the CCB waiting case!");
@@ -857,8 +846,6 @@ ctlfestart(struct cam_periph *periph, un
 
 			/*
 			 * Datamove call, we need to setup the S/G list. 
-			 * If we pass in a S/G list, the isp(4) driver at
-			 * least expects physical/bus addresses.
 			 */
 
 			cmd_info = (struct ctlfe_lun_cmd_info *)
@@ -933,12 +920,13 @@ ctlfestart(struct cam_periph *periph, un
 				int *ti;
 
 				/*
-				 * XXX KDM this is a temporary hack.  The
-				 * isp(4) driver can't deal with S/G lists
-				 * with virtual pointers, so we need to
-				 * go through and send down one virtual
-				 * pointer at a time.
+				 * If we have more S/G list pointers than
+				 * will fit in the available storage in the
+				 * cmd_info structure inside the ctl_io header,
+				 * then we need to send down the pointers
+				 * one element at a time.
 				 */
+
 				sglist = (struct ctl_sg_entry *)
 					io->scsiio.kern_data_ptr;
 				ti = &cmd_info->cur_transfer_index;
@@ -1405,13 +1393,15 @@ ctlfedone(struct cam_periph *periph, uni
 				break;
 			default:
 				/*
-				 * XXX KDM the isp(4) driver doesn't really
-				 * seem to send errors back for data
-				 * transfers that I can tell.  There is one
-				 * case where it'll send CAM_REQ_CMP_ERR,
-				 * but probably not that many more cases.
-				 * So set a generic data phase error here,
-				 * like the SXP driver sets.
+				 * XXX KDM we probably need to figure out a
+				 * standard set of errors that the SIM
+				 * drivers should return in the event of a
+				 * data transfer failure.  A data phase
+				 * error will at least point the user to a
+				 * data transfer error of some sort.
+				 * Hopefully the SIM printed out some
+				 * additional information to give the user
+				 * a clue what happened.
 				 */
 				io->io_hdr.port_status = 0xbad1;
 				ctl_set_data_phase_error(&io->scsiio);
@@ -1689,20 +1679,22 @@ ctlfe_onoffline(void *arg, int online)
 
 	sim = bus_softc->sim;
 
-	CAM_SIM_LOCK(sim);
+	mtx_assert(sim->mtx, MA_OWNED);
+
 	status = xpt_create_path(&path, /*periph*/ NULL, bus_softc->path_id,
 		CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD);
 	if (status != CAM_REQ_CMP) {
 		printf("%s: unable to create path!\n", __func__);
-		CAM_SIM_UNLOCK(sim);
 		return;
 	}
-	CAM_SIM_UNLOCK(sim);
-
-	ccb = (union ccb *)malloc(sizeof(*ccb), M_TEMP, M_WAITOK | M_ZERO);
+	ccb = (union ccb *)malloc(sizeof(*ccb), M_TEMP, M_NOWAIT | M_ZERO);
+	if (ccb == NULL) {
+		printf("%s: unable to malloc CCB!\n", __func__);
+		xpt_free_path(path);
+		return;
+	}
 	xpt_setup_ccb(&ccb->ccb_h, path, CAM_PRIORITY_NONE);
 
-
 	/*
 	 * Copan WWN format:
 	 *
@@ -1720,11 +1712,9 @@ ctlfe_onoffline(void *arg, int online)
 
 		ccb->ccb_h.func_code = XPT_GET_SIM_KNOB;
 
-		CAM_SIM_LOCK(sim);
 
 		xpt_action(ccb);
 
-		CAM_SIM_UNLOCK(sim);
 
 		if ((ccb->knob.xport_specific.valid & KNOB_VALID_ADDRESS) != 0){
 #ifdef RANDOM_WWNN
@@ -1822,9 +1812,6 @@ ctlfe_onoffline(void *arg, int online)
 	else
 		ccb->knob.xport_specific.fc.role = KNOB_ROLE_NONE;
 
-
-	CAM_SIM_LOCK(sim);
-
 	xpt_action(ccb);
 
 	if ((ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
@@ -1841,8 +1828,6 @@ ctlfe_onoffline(void *arg, int online)
 
 	xpt_free_path(path);
 
-	CAM_SIM_UNLOCK(sim);
-
 	free(ccb, M_TEMP);
 
 	return;
@@ -1851,13 +1836,111 @@ ctlfe_onoffline(void *arg, int online)
 static void
 ctlfe_online(void *arg)
 {
+	struct ctlfe_softc *bus_softc;
+	struct cam_path *path;
+	cam_status status;
+	struct ctlfe_lun_softc *lun_softc;
+	struct cam_sim *sim;
+
+	bus_softc = (struct ctlfe_softc *)arg;
+	sim = bus_softc->sim;
+
+	CAM_SIM_LOCK(sim);
+
+	/*
+	 * Create the wildcard LUN before bringing the port online.
+	 */
+	status = xpt_create_path(&path, /*periph*/ NULL,
+				 bus_softc->path_id, CAM_TARGET_WILDCARD,
+				 CAM_LUN_WILDCARD);
+	if (status != CAM_REQ_CMP) {
+		printf("%s: unable to create path for wildcard periph\n",
+				__func__);
+		CAM_SIM_UNLOCK(sim);
+		return;
+	}
+
+	lun_softc = malloc(sizeof(*lun_softc), M_CTLFE,
+			M_NOWAIT | M_ZERO);
+	if (lun_softc == NULL) {
+		xpt_print(path, "%s: unable to allocate softc for "
+				"wildcard periph\n", __func__);
+		xpt_free_path(path);
+		CAM_SIM_UNLOCK(sim);
+		return;
+	}
+
+	lun_softc->parent_softc = bus_softc;
+	lun_softc->flags |= CTLFE_LUN_WILDCARD;
+
+	STAILQ_INSERT_TAIL(&bus_softc->lun_softc_list, lun_softc, links);
+
+
+	status = cam_periph_alloc(ctlferegister,
+				  ctlfeoninvalidate,
+				  ctlfecleanup,
+				  ctlfestart,
+				  "ctl",
+				  CAM_PERIPH_BIO,
+				  path,
+				  ctlfeasync,
+				  0,
+				  lun_softc);
+
+	if ((status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
+		const struct cam_status_entry *entry;
+
+		entry = cam_fetch_status_entry(status);
+
+		printf("%s: CAM error %s (%#x) returned from "
+		       "cam_periph_alloc()\n", __func__, (entry != NULL) ?
+		       entry->status_text : "Unknown", status);
+	}
+
+	xpt_free_path(path);
+
 	ctlfe_onoffline(arg, /*online*/ 1);
+
+	CAM_SIM_UNLOCK(sim);
 }
 
 static void
 ctlfe_offline(void *arg)
 {
+	struct ctlfe_softc *bus_softc;
+	struct cam_path *path;
+	cam_status status;
+	struct cam_periph *periph;
+	struct cam_sim *sim;
+
+	bus_softc = (struct ctlfe_softc *)arg;
+	sim = bus_softc->sim;
+
+	CAM_SIM_LOCK(sim);
+
 	ctlfe_onoffline(arg, /*online*/ 0);
+
+	/*
+	 * Disable the wildcard LUN for this port now that we have taken
+	 * the port offline.
+	 */
+	status = xpt_create_path(&path, /*periph*/ NULL,
+				 bus_softc->path_id, CAM_TARGET_WILDCARD,
+				 CAM_LUN_WILDCARD);
+	if (status != CAM_REQ_CMP) {
+		CAM_SIM_UNLOCK(sim);
+		printf("%s: unable to create path for wildcard periph\n",
+		       __func__);
+		return;
+	}
+
+
+	if ((periph = cam_periph_find(path, "ctl")) != NULL)
+		cam_periph_invalidate(periph);
+
+	xpt_free_path(path);
+
+	CAM_SIM_UNLOCK(sim);
 }
 
 static int
@@ -1890,21 +1973,17 @@ ctlfe_lun_enable(void *arg, struct ctl_i
 	bus_softc = (struct ctlfe_softc *)arg;
 	sim = bus_softc->sim;
 
-	CAM_SIM_LOCK(sim);
-
-	status = xpt_create_path(&path, /*periph*/ NULL, bus_softc->path_id,
-				 targ_id.id, lun_id);
+	status = xpt_create_path_unlocked(&path, /*periph*/ NULL,
+					  bus_softc->path_id,
+					  targ_id.id, lun_id);
 	/* XXX KDM need some way to return status to CTL here? */
 	if (status != CAM_REQ_CMP) {
 		printf("%s: could not create path, status %#x\n", __func__,
 		       status);
-		CAM_SIM_UNLOCK(sim);
 		return (1);
 	}
-	CAM_SIM_UNLOCK(sim);
 
 	softc = malloc(sizeof(*softc), M_CTLFE, M_WAITOK | M_ZERO);
-
 	CAM_SIM_LOCK(sim);
 	periph = cam_periph_find(path, "ctl");
 	if (periph != NULL) {
@@ -1937,23 +2016,20 @@ ctlfe_lun_enable(void *arg, struct ctl_i
 }
 
 /*
- * XXX KDM we disable LUN removal here.  The problem is that the isp(4)
- * driver doesn't currently handle LUN removal properly.  We need to keep 
- * enough state here at the peripheral level even after LUNs have been
- * removed inside CTL.
- *
- * Once the isp(4) driver is fixed, this can be re-enabled.
+ * This will get called when the user removes a LUN to disable that LUN
+ * on every bus that is attached to CTL.  
  */
 static int
 ctlfe_lun_disable(void *arg, struct ctl_id targ_id, int lun_id)
 {
-#ifdef NOTYET
 	struct ctlfe_softc *softc;
 	struct ctlfe_lun_softc *lun_softc;
+	struct cam_sim *sim;
 
 	softc = (struct ctlfe_softc *)arg;
+	sim = softc->sim;
 
-	mtx_lock(softc->sim->mtx);
+	CAM_SIM_LOCK(sim);
 	STAILQ_FOREACH(lun_softc, &softc->lun_softc_list, links) {
 		struct cam_path *path;
 
@@ -1965,7 +2041,7 @@ ctlfe_lun_disable(void *arg, struct ctl_
 		}
 	}
 	if (lun_softc == NULL) {
-		mtx_unlock(softc->sim->mtx);
+		CAM_SIM_UNLOCK(sim);
 		printf("%s: can't find target %d lun %d\n", __func__,
 		       targ_id.id, lun_id);
 		return (1);
@@ -1973,8 +2049,7 @@ ctlfe_lun_disable(void *arg, struct ctl_
 
 	cam_periph_invalidate(lun_softc->periph);
 
-	mtx_unlock(softc->sim->mtx);
-#endif
+	CAM_SIM_UNLOCK(sim);
 
 	return (0);
 }
@@ -2139,7 +2214,7 @@ ctlfe_datamove_done(union ctl_io *io)
 
 	sim = xpt_path_sim(ccb->ccb_h.path);
 
-	mtx_lock(sim->mtx);
+	CAM_SIM_LOCK(sim);
 
 	periph = xpt_path_periph(ccb->ccb_h.path);
 
@@ -2186,7 +2261,7 @@ ctlfe_datamove_done(union ctl_io *io)
 			xpt_schedule(periph, /*priority*/ 1);
 	}
 
-	mtx_unlock(sim->mtx);
+	CAM_SIM_UNLOCK(sim);
 }
 
 static void


More information about the svn-src-all mailing list