[Midnightbsd-cvs] src [9054] trunk/sys/cam/ctl: CTL locking issues.

laffer1 at midnightbsd.org laffer1 at midnightbsd.org
Sat Oct 1 06:03:55 EDT 2016


Revision: 9054
          http://svnweb.midnightbsd.org/src/?rev=9054
Author:   laffer1
Date:     2016-10-01 06:03:54 -0400 (Sat, 01 Oct 2016)
Log Message:
-----------
CTL locking issues.

Modified Paths:
--------------
    trunk/sys/cam/ctl/ctl.c
    trunk/sys/cam/ctl/ctl_frontend_cam_sim.c
    trunk/sys/cam/ctl/scsi_ctl.c

Modified: trunk/sys/cam/ctl/ctl.c
===================================================================
--- trunk/sys/cam/ctl/ctl.c	2016-10-01 10:03:02 UTC (rev 9053)
+++ trunk/sys/cam/ctl/ctl.c	2016-10-01 10:03:54 UTC (rev 9054)
@@ -2263,11 +2263,31 @@
 				 */
 				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: trunk/sys/cam/ctl/ctl_frontend_cam_sim.c
===================================================================
--- trunk/sys/cam/ctl/ctl_frontend_cam_sim.c	2016-10-01 10:03:02 UTC (rev 9053)
+++ trunk/sys/cam/ctl/ctl_frontend_cam_sim.c	2016-10-01 10:03:54 UTC (rev 9054)
@@ -275,7 +275,7 @@
 }
 
 static void
-cfcs_online(void *arg)
+cfcs_onoffline(void *arg, int online)
 {
 	struct cfcs_softc *softc;
 	union ccb *ccb;
@@ -283,13 +283,12 @@
 	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 @@
 			    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_online(void *arg)
+{
+	cfcs_onoffline(arg, /*online*/ 1);
+}
+
+static void
 cfcs_offline(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;
-	}
-
-	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);
+	cfcs_onoffline(arg, /*online*/ 0);
 }
 
 static int

Modified: trunk/sys/cam/ctl/scsi_ctl.c
===================================================================
--- trunk/sys/cam/ctl/scsi_ctl.c	2016-10-01 10:03:02 UTC (rev 9053)
+++ trunk/sys/cam/ctl/scsi_ctl.c	2016-10-01 10:03:54 UTC (rev 9054)
@@ -73,6 +73,7 @@
 #include <cam/ctl/ctl_error.h>
 
 typedef enum {
+	CTLFE_CCB_DEFAULT	= 0x00,
 	CTLFE_CCB_WAITING 	= 0x01
 } ctlfe_ccb_types;
 
@@ -304,10 +305,7 @@
 	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 @@
 						  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 @@
 			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;
+		break;
+	}
+	case AC_PATH_DEREGISTERED: {
+		struct ctlfe_softc *softc = NULL;
+
+		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;
+			}
 		}
-		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;
+		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);
 		}
-
-		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);
-
-		xpt_free_path(path);
-
 		break;
 	}
-	case AC_PATH_DEREGISTERED:
-		/* ctl_frontend_deregister() */
-		break;
 	case AC_CONTRACT: {
 		struct ac_contract *ac;
 
@@ -699,11 +683,14 @@
 	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 @@
 
 	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 @@
 
 			/*
 			 * 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 @@
 				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 @@
 				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,6 +1677,10 @@
 
 	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) {
@@ -1693,11 +1687,14 @@
 		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 @@
 
 		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 @@
 	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 @@
 
 	xpt_free_path(path);
 
-	CAM_SIM_UNLOCK(sim);
-
 	free(ccb, M_TEMP);
 
 	return;
@@ -1846,13 +1836,111 @@
 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 @@
 
 	
 	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 @@
 	}
 
 	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 @@
 
 	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 @@
 		}
 	}
 	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 @@
 
 	cam_periph_invalidate(lun_softc->periph);
 
-	mtx_unlock(softc->sim->mtx);
-#endif
+	CAM_SIM_UNLOCK(sim);
 
 	return (0);
 }
@@ -2131,7 +2214,7 @@
 
 	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 @@
 			xpt_schedule(periph, /*priority*/ 1);
 	}
 
-	mtx_unlock(sim->mtx);
+	CAM_SIM_UNLOCK(sim);
 }
 
 static void



More information about the Midnightbsd-cvs mailing list