svn commit: r249167 - stable/9/sys/cam/ctl

Kenneth D. Merry ken at FreeBSD.org
Fri Apr 5 19:33:33 UTC 2013


Author: ken
Date: Fri Apr  5 19:33:31 2013
New Revision: 249167
URL: http://svnweb.freebsd.org/changeset/base/249167

Log:
  MFC r244052 and r245288:
  
    r244052 | ken | 2012-12-09 12:53:21 -0700 (Sun, 09 Dec 2012) | 20 lines
  
    Fix a couple of CTL locking issues and clean up some duplicated code.
  
    ctl_frontend_cam_sim.c: Coalesce cfcs_online() and cfcs_offline()
    			into a single function since these were
    			identical except for one line.
  
    			Make sure we hold the SIM lock around path
    			creation, and calling xpt_rescan().
  
    scsi_ctl.c:		In ctlfe_onoffline(), make sure we hold the
    			SIM lock around path creation and free
    			calls, as well as xpt_action().
  
    			In ctlfe_lun_enable(), hold the SIM lock
    			around path and peripheral operations that
    			require it.
  
    Sponsored by:	Spectra Logic Corporation
    MFC after:	1 week
  
    ------------------------------------------------------------------------
    r245228 | ken | 2013-01-09 10:02:08 -0700 (Wed, 09 Jan 2013) | 43 lines
  
    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:
  stable/9/sys/cam/ctl/ctl.c
  stable/9/sys/cam/ctl/ctl_frontend_cam_sim.c
  stable/9/sys/cam/ctl/scsi_ctl.c
Directory Properties:
  stable/9/sys/   (props changed)

Modified: stable/9/sys/cam/ctl/ctl.c
==============================================================================
--- stable/9/sys/cam/ctl/ctl.c	Fri Apr  5 18:09:43 2013	(r249166)
+++ stable/9/sys/cam/ctl/ctl.c	Fri Apr  5 19:33:31 2013	(r249167)
@@ -2263,11 +2263,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: stable/9/sys/cam/ctl/ctl_frontend_cam_sim.c
==============================================================================
--- stable/9/sys/cam/ctl/ctl_frontend_cam_sim.c	Fri Apr  5 18:09:43 2013	(r249166)
+++ stable/9/sys/cam/ctl/ctl_frontend_cam_sim.c	Fri Apr  5 19:33:31 2013	(r249167)
@@ -275,7 +275,7 @@ cfcs_shutdown(void)
 }
 
 static void
-cfcs_online(void *arg)
+cfcs_onoffline(void *arg, int online)
 {
 	struct cfcs_softc *softc;
 	union ccb *ccb;
@@ -283,13 +283,12 @@ cfcs_online(void *arg)
 	softc = (struct cfcs_softc *)arg;
 
 	mtx_lock(&softc->lock);
-	softc->online = 1;
-	mtx_unlock(&softc->lock);
+	softc->online = online;
 
 	ccb = xpt_alloc_ccb_nowait();
 	if (ccb == NULL) {
 		printf("%s: unable to allocate CCB for rescan\n", __func__);
-		return;
+		goto bailout;
 	}
 
 	if (xpt_create_path(&ccb->ccb_h.path, xpt_periph,
@@ -297,37 +296,24 @@ cfcs_online(void *arg)
 			    CAM_LUN_WILDCARD) != CAM_REQ_CMP) {
 		printf("%s: can't allocate path for rescan\n", __func__);
 		xpt_free_ccb(ccb);
-		return;
+		goto bailout;
 	}
 	xpt_rescan(ccb);
+
+bailout:
+	mtx_unlock(&softc->lock);
 }
 
 static void
-cfcs_offline(void *arg)
+cfcs_online(void *arg)
 {
-	struct cfcs_softc *softc;
-	union ccb *ccb;
-
-	softc = (struct cfcs_softc *)arg;
-
-	mtx_lock(&softc->lock);
-	softc->online = 0;
-	mtx_unlock(&softc->lock);
-
-	ccb = xpt_alloc_ccb_nowait();
-	if (ccb == NULL) {
-		printf("%s: unable to allocate CCB for rescan\n", __func__);
-		return;
-	}
+	cfcs_onoffline(arg, /*online*/ 1);
+}
 
-	if (xpt_create_path(&ccb->ccb_h.path, xpt_periph,
-			    cam_sim_path(softc->sim), CAM_TARGET_WILDCARD,
-			    CAM_LUN_WILDCARD) != CAM_REQ_CMP) {
-		printf("%s: can't allocate path for rescan\n", __func__);
-		xpt_free_ccb(ccb);
-		return;
-	}
-	xpt_rescan(ccb);
+static void
+cfcs_offline(void *arg)
+{
+	cfcs_onoffline(arg, /*online*/ 0);
 }
 
 static int

Modified: stable/9/sys/cam/ctl/scsi_ctl.c
==============================================================================
--- stable/9/sys/cam/ctl/scsi_ctl.c	Fri Apr  5 18:09:43 2013	(r249166)
+++ stable/9/sys/cam/ctl/scsi_ctl.c	Fri Apr  5 19:33:31 2013	(r249167)
@@ -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);
@@ -1687,17 +1677,24 @@ ctlfe_onoffline(void *arg, int online)
 
 	set_wwnn = 0;
 
+	sim = bus_softc->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__);
 		return;
 	}
-	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);
 
-	sim = xpt_path_sim(path);
-
 	/*
 	 * Copan WWN format:
 	 *
@@ -1715,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
@@ -1817,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) {
@@ -1836,8 +1828,6 @@ ctlfe_onoffline(void *arg, int online)
 
 	xpt_free_path(path);
 
-	CAM_SIM_UNLOCK(sim);
-
 	free(ccb, M_TEMP);
 
 	return;
@@ -1846,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
@@ -1883,11 +1971,11 @@ ctlfe_lun_enable(void *arg, struct ctl_i
 
 	
 	bus_softc = (struct ctlfe_softc *)arg;
+	sim = bus_softc->sim;
 
 	status = xpt_create_path_unlocked(&path, /*periph*/ NULL,
 					  bus_softc->path_id,
-					  targ_id.id,
-					  lun_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__,
@@ -1896,14 +1984,13 @@ ctlfe_lun_enable(void *arg, struct ctl_i
 	}
 
 	softc = malloc(sizeof(*softc), M_CTLFE, M_WAITOK | M_ZERO);
-	sim = xpt_path_sim(path);
-	mtx_lock(sim->mtx);
+	CAM_SIM_LOCK(sim);
 	periph = cam_periph_find(path, "ctl");
 	if (periph != NULL) {
 		/* We've already got a periph, no need to alloc a new one. */
 		xpt_free_path(path);
 		free(softc, M_CTLFE);
-		mtx_unlock(sim->mtx);
+		CAM_SIM_UNLOCK(sim);
 		return (0);
 	}
 
@@ -1923,29 +2010,26 @@ ctlfe_lun_enable(void *arg, struct ctl_i
 
 	xpt_free_path(path);
 
-	mtx_unlock(sim->mtx);
+	CAM_SIM_UNLOCK(sim);
 
 	return (0);
 }
 
 /*
- * 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;
 
@@ -1957,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);
@@ -1965,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);
 }
@@ -2131,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);
 
@@ -2178,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-stable mailing list