going rogue: incorporating REPORT_LUNS into the SCSI probe sequence

Matthew Jacob mj at feral.com
Mon May 17 19:09:14 UTC 2010


This one is a bit rougher than the others, but I'd like to encourage 
some discussion about this.

The problems presented by not using REPORT LUNS have been getting uglier 
and uglier. Any kind of sparse lun arrangement for large JBODs or RAID 
units just makes life very painful.

These changes manage REPORT LUNS for devices > SPC2, and even manage 
changes in the list. These changes do *not* address the edge case of 
having no attached LUN 0 (which has been known to happen- it's not illegal).

I haven't quite finished putting together an action based upon the LUN 
INVENTORY MAY HAVE CHANGED ASC/ASCQ, and testing has been somewhat light.

Still......

-------------- next part --------------
diff -r 85c0fa25a2fc sys/cam/cam_xpt.c
--- a/sys/cam/cam_xpt.c	Fri May 14 15:55:34 2010 -0700
+++ b/sys/cam/cam_xpt.c	Mon May 17 12:01:11 2010 -0700
@@ -4189,6 +4189,7 @@
 		target->target_id = target_id;
 		target->refcount = 1;
 		target->generation = 0;
+		target->luns = NULL;
 		timevalclear(&target->last_reset);
 		/*
 		 * Hold a reference to our parent bus so it
@@ -4220,6 +4221,8 @@
 		TAILQ_REMOVE(&target->bus->et_entries, target, links);
 		target->bus->generation++;
 		xpt_release_bus(target->bus);
+		if (target->luns)
+			free(target->luns, M_CAMXPT);
 		free(target, M_CAMXPT);
 	}
 }
diff -r 85c0fa25a2fc sys/cam/cam_xpt_internal.h
--- a/sys/cam/cam_xpt_internal.h	Fri May 14 15:55:34 2010 -0700
+++ b/sys/cam/cam_xpt_internal.h	Mon May 17 12:01:11 2010 -0700
@@ -135,6 +135,8 @@
 	u_int32_t	refcount;
 	u_int		generation;
 	struct		timeval last_reset;
+	u_int		rpl_size;
+	struct scsi_report_luns_data *luns;
 };
 
 /*
diff -r 85c0fa25a2fc sys/cam/scsi/scsi_xpt.c
--- a/sys/cam/scsi/scsi_xpt.c	Fri May 14 15:55:34 2010 -0700
+++ b/sys/cam/scsi/scsi_xpt.c	Mon May 17 12:01:11 2010 -0700
@@ -75,6 +75,7 @@
 #define	CAM_QUIRK_NOSERIAL	0x02
 #define	CAM_QUIRK_HILUNS	0x04
 #define	CAM_QUIRK_NOHILUNS	0x08
+#define	CAM_QUIRK_NORPTLUNS	0x10
 	u_int mintags;
 	u_int maxtags;
 };
@@ -88,6 +89,21 @@
     "allow search above LUN 7 for SCSI3 and greater devices");
 
 #define	CAM_SCSI2_MAXLUN	8
+#define	CAM_CAN_GET_SIMPLE_LUN(x, i)				\
+	((((x)->luns[i].lundata[0] & RPL_LUNDATA_ATYP_MASK) ==	\
+	RPL_LUNDATA_ATYP_PERIPH) ||				\
+	(((x)->luns[i].lundata[0] & RPL_LUNDATA_ATYP_MASK) ==	\
+	RPL_LUNDATA_ATYP_FLAT))
+#define	CAM_GET_SIMPLE_LUN(lp, i, lval)					\
+	if (((lp)->luns[(i)].lundata[0] & RPL_LUNDATA_ATYP_MASK) == 	\
+	    RPL_LUNDATA_ATYP_PERIPH) {					\
+		(lval) = (lp)->luns[(i)].lundata[1];			\
+	} else {							\
+		(lval) = (lp)->luns[(i)].lundata[0];			\
+		(lval) &= RPL_LUNDATA_FLAT_LUN_MASK;			\
+		(lval) <<= 8;						\
+		(lval) |=  (lp)->luns[(i)].lundata[1];			\
+	}
 /*
  * If we're not quirked to search <= the first 8 luns
  * and we are either quirked to search above lun 8,
@@ -120,6 +136,7 @@
 	PROBE_TUR,
 	PROBE_INQUIRY,	/* this counts as DV0 for Basic Domain Validation */
 	PROBE_FULL_INQUIRY,
+	PROBE_REPORT_LUNS,
 	PROBE_MODE_SENSE,
 	PROBE_SERIAL_NUM_0,
 	PROBE_SERIAL_NUM_1,
@@ -134,6 +151,7 @@
 	"PROBE_TUR",
 	"PROBE_INQUIRY",
 	"PROBE_FULL_INQUIRY",
+	"PROBE_REPORT_LUNS",
 	"PROBE_MODE_SENSE",
 	"PROBE_SERIAL_NUM_0",
 	"PROBE_SERIAL_NUM_1",
@@ -531,6 +549,8 @@
 static int       proberequestbackoff(struct cam_periph *periph,
 				     struct cam_ed *device);
 static void	 probedone(struct cam_periph *periph, union ccb *done_ccb);
+static void	 probe_purge_old(struct cam_path *path,
+				 struct scsi_report_luns_data *new);
 static void	 probecleanup(struct cam_periph *periph);
 static void	 scsi_find_quirk(struct cam_ed *device);
 static void	 scsi_scan_bus(struct cam_periph *periph, union ccb *ccb);
@@ -689,6 +709,7 @@
 
 	softc = (probe_softc *)periph->softc;
 	csio = &start_ccb->csio;
+again:
 
 	switch (softc->action) {
 	case PROBE_TUR:
@@ -777,6 +798,27 @@
 			     /*timeout*/60 * 1000);
 		break;
 	}
+	case PROBE_REPORT_LUNS:
+	{
+		void *rp;
+
+		rp = malloc(periph->path->target->rpl_size,
+		    M_CAMXPT, M_NOWAIT | M_ZERO);
+		if (rp == NULL) {
+			struct scsi_inquiry_data *inq_buf;
+			inq_buf = &periph->path->device->inq_data;
+			xpt_print(periph->path,
+			    "Unable to alloc report luns storage\n");
+			if (INQ_DATA_TQ_ENABLED(inq_buf))
+				PROBE_SET_ACTION(softc, PROBE_MODE_SENSE);
+			else
+				PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_0);
+			goto again;
+		}
+		scsi_report_luns(csio, 4, probedone, MSG_SIMPLE_Q_TAG,
+		    RPL_REPORT_DEFAULT, rp, periph->path->target->rpl_size,
+		    SSD_FULL_SIZE, 60000); break;
+	}
 	case PROBE_MODE_SENSE:
 	{
 		void  *mode_buf;
@@ -1075,7 +1117,18 @@
 				scsi_find_quirk(path->device);
 
 				scsi_devise_transport(path);
-				if (INQ_DATA_TQ_ENABLED(inq_buf))
+
+				if (path->device->lun_id == 0 &&
+				    SID_ANSI_REV(inq_buf) > SCSI_REV_SPC2 &&
+				    (SCSI_QUIRK(path->device)->quirks &
+				     CAM_QUIRK_NORPTLUNS) == 0) {
+					PROBE_SET_ACTION(softc,
+					    PROBE_REPORT_LUNS);
+					/*
+					 * Start with room for *one* lun.
+					 */
+					periph->path->target->rpl_size = 16;
+				} else if (INQ_DATA_TQ_ENABLED(inq_buf))
 					PROBE_SET_ACTION(softc,
 					    PROBE_MODE_SENSE);
 				else
@@ -1117,6 +1170,83 @@
 		status = CAM_REQ_CMP_ERR;
 		break;
 	}
+	case PROBE_REPORT_LUNS:
+	{
+		struct ccb_scsiio *csio;
+		struct scsi_report_luns_data *lp;
+
+		csio = &done_ccb->csio;
+
+		lp = (struct scsi_report_luns_data *)csio->data_ptr;
+
+		if (status != CAM_REQ_CMP) {
+			if (PCHK_S(done_ccb, SF_RETRY_UA|SF_NO_PRINT, softc))
+				return;
+			free(lp, M_CAMXPT);
+			lp = NULL;
+		} else {
+			u_int nlun, maxlun;
+			struct scsi_report_luns_data *newlp = NULL;
+			lun_id_t lun;
+
+ 			nlun = scsi_4btoul(lp->length) / 8;
+			maxlun = (csio->dxfer_len / 8) - 1;
+
+			/*
+			 * If we have one lun, and it's lun 0, just
+			 * do the standard probe sequence.
+			 */
+			if (nlun < 1) {
+				free(lp, M_CAMXPT);
+				lp = NULL;
+			} else if (nlun == 1 &&
+			    CAM_CAN_GET_SIMPLE_LUN(lp, 0)) {
+				CAM_GET_SIMPLE_LUN(lp, 0, lun);
+				if (lun != 0) {
+					newlp = lp;
+				} else {
+					free(lp, M_CAMXPT);
+					lp = NULL;
+				}
+			} else if (nlun > maxlun) {
+				/*
+				 * Retry and cover all luns
+				 */
+				free(lp, M_CAMXPT);
+				path->target->rpl_size = (nlun << 3) + 8;
+				xpt_release_ccb(done_ccb);
+				xpt_schedule(periph, priority);
+				return;
+			} else {
+				newlp = lp;
+			}
+			if (newlp) {
+				/*
+				 * If we have an old lun list, We can either retest luns
+				 * that appear to have been dropped, or just nuke them.
+				 * We'll opt for the latter. This function will also
+				 * install the new list in the target structure.
+				 */
+				probe_purge_old(path, newlp);
+			}
+		}
+		if (path->device->flags & CAM_DEV_INQUIRY_DATA_VALID) {
+			struct scsi_inquiry_data *inq_buf;
+			inq_buf = &path->device->inq_data;
+			if (INQ_DATA_TQ_ENABLED(inq_buf))
+				PROBE_SET_ACTION(softc, PROBE_MODE_SENSE);
+			else
+				PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_0);
+			xpt_release_ccb(done_ccb);
+			xpt_schedule(periph, priority);
+			return;
+		}
+		if (lp) {
+			free(lp, M_CAMXPT);
+		}
+		status = CAM_REQ_CMP_ERR;
+		break;
+	}
 	case PROBE_MODE_SENSE:
 	{
 		struct ccb_scsiio *csio;
@@ -1426,6 +1556,66 @@
 }
 
 static void
+probe_purge_old(struct cam_path *path, struct scsi_report_luns_data *new)
+{
+	struct cam_path *tp;
+	struct scsi_report_luns_data *old;
+	u_int idx1, idx2, nlun_old, nlun_new, this_lun;
+	u_int8_t *ol, *nl;
+
+	if (path->target == NULL) {
+		return;
+	}
+	if (path->target->luns == NULL) {
+		path->target->luns = new;
+		return;
+	}
+	old = path->target->luns;
+	nlun_old = scsi_4btoul(old->length) / 8;
+	nlun_new = scsi_4btoul(new->length) / 8;
+
+	/*
+	 * We are not going to assume sorted lists. Deal.
+	 */
+	for (idx1 = 0; idx1 < nlun_old; idx1++) {
+		ol = old->luns[idx1].lundata;
+		for (idx2 = 0; idx2 < nlun_new; idx2++) {
+			nl = new->luns[idx2].lundata;
+			if (memcmp(nl, ol, 8) == 0) {
+				break;
+			}
+		}
+		if (idx2 < nlun_new) {
+			continue;
+		}
+		/*
+		 * An 'old' item not in the 'new' list.
+		 * Nuke it. Except that if it is lun 0,
+		 * that would be what the probe state
+		 * machine is currently working on,
+		 * so we won't do that.
+		 *
+		 * We also cannot nuke it if it is
+		 * not in a lun format we understand.
+		 */
+		if (!CAM_CAN_GET_SIMPLE_LUN(old, idx1)) {
+			continue;
+		}
+		CAM_GET_SIMPLE_LUN(old, idx1, this_lun);
+		if (this_lun == 0) {
+			continue;
+		}
+		if (xpt_create_path(&tp, NULL, xpt_path_path_id(path),
+		    xpt_path_target_id(path), this_lun) == CAM_REQ_CMP) {
+			xpt_async(AC_LOST_DEVICE, tp, NULL);
+			xpt_free_path(tp);
+		}
+	}
+	free(old, M_CAMXPT);
+	path->target->luns = new;
+}
+
+static void
 probecleanup(struct cam_periph *periph)
 {
 	free(periph->softc, M_CAMXPT);
@@ -1473,6 +1663,7 @@
 	union	ccb *request_ccb;
 	struct 	ccb_pathinq *cpi;
 	int	counter;
+	int	lunindex;
 } scsi_scan_bus_info;
 
 /*
@@ -1554,6 +1745,7 @@
 		}
 		scan_info->request_ccb = request_ccb;
 		scan_info->cpi = &work_ccb->cpi;
+		scan_info->lunindex = 0;
 
 		/* Cache on our stack so we can work asynchronously */
 		max_target = scan_info->cpi->max_target;
@@ -1615,6 +1807,9 @@
 		cam_status status;
 		struct cam_path *path;
 		scsi_scan_bus_info *scan_info;
+		struct cam_et *target;
+		struct cam_ed *device;
+		int next_target;
 		path_id_t path_id;
 		target_id_t target_id;
 		lun_id_t lun_id;
@@ -1630,9 +1825,34 @@
 		lun_id = request_ccb->ccb_h.target_lun;
 		xpt_action(request_ccb);
 
-		if (request_ccb->ccb_h.status != CAM_REQ_CMP) {
-			struct cam_ed *device;
-			struct cam_et *target;
+		target = request_ccb->ccb_h.path->target;
+		next_target = 1;
+
+		if (target->luns) {
+			u_int nluns = scsi_4btoul(target->luns->length) / 8;
+			/*
+			 * We could check to see whether or not the current probe
+			 * finishing and is lun 0. If it were marked as failed,
+			 * we could check against the first lunindex- we just (barely)
+			 * might have a target with nothing at lun 0. This is really
+			 * contorted, but oddly enough a valid edge case. However,
+			 * we really can't get here w/o changing the probedone
+			 * handling of inquiry so that it will accept non-connected
+			 * luns. It's just too much of an edge case to worry about.
+			 *
+			 * Just see if we have more luns to scan. We're not going to
+			 * care about success or failure here for any one probe because
+			 * we're just simply poking at the list the target gave us.
+			 */
+			if (scan_info->lunindex++ < nluns) {
+				if (CAM_CAN_GET_SIMPLE_LUN(target->luns,
+				    scan_info->lunindex)) {
+					CAM_GET_SIMPLE_LUN(target->luns,
+					    scan_info->lunindex, lun_id);
+					next_target = 0;
+				}
+			}
+		} else if (request_ccb->ccb_h.status != CAM_REQ_CMP) {
 			int phl;
 
 			/*
@@ -1641,7 +1861,6 @@
 			 * target that might have "gone away", go onto
 			 * the next lun.
 			 */
-			target = request_ccb->ccb_h.path->target;
 			/*
 			 * We may touch devices that we don't
 			 * hold references too, so ensure they
@@ -1657,11 +1876,12 @@
 					device = TAILQ_NEXT(device, links);
 			}
 			if ((lun_id != 0) || (device != NULL)) {
-				if (lun_id < (CAM_SCSI2_MAXLUN-1) || phl)
+				if (lun_id < (CAM_SCSI2_MAXLUN-1) || phl) {
 					lun_id++;
+					next_target = 0;
+				}
 			}
 		} else {
-			struct cam_ed *device;
 
 			device = request_ccb->ccb_h.path->device;
 
@@ -1669,8 +1889,10 @@
 			    CAM_QUIRK_NOLUNS) == 0) {
 				/* Try the next lun */
 				if (lun_id < (CAM_SCSI2_MAXLUN-1)
-				  || CAN_SRCH_HI_DENSE(device))
+				  || CAN_SRCH_HI_DENSE(device)) {
 					lun_id++;
+					next_target = 0;
+				}
 			}
 		}
 
@@ -1682,10 +1904,10 @@
 		/*
 		 * Check to see if we scan any further luns.
 		 */
-		if (lun_id == request_ccb->ccb_h.target_lun
-                 || lun_id > scan_info->cpi->max_lun) {
+		if (next_target) {
 			int done;
 
+			scan_info->lunindex = 0;
  hop_again:
 			done = 0;
 			if (scan_info->cpi->hba_misc & PIM_SEQSCAN) {


More information about the freebsd-scsi mailing list