Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 May 2010 11:30:42 -0700
From:      Matthew Jacob <mj@feral.com>
To:        freebsd-scsi@freebsd.org
Subject:   Re: patches for CAM SCSI probing, etc.
Message-ID:  <4BFAC5D2.2070502@feral.com>
In-Reply-To: <4BEB87B8.1070104@feral.com>
References:  <4BEB87B8.1070104@feral.com>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------010300040707060501050509
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit


Any final comments on this?


> [ prefatory note: these patches incorporate the work done by Alexander 
> Motin for protecting the periph driver list. I am pretty eager for 
> comments on this. This ended up being a tougher nut to crack than I 
> thought. ]
>
> This is a set of patches that attempts to address the generalized
> problem of dealing with SCSI devices that are being unconfigured
> at the same time that they are being configured.
>
> There are essentially 3 problems that these pataches address.
>
> The first problem with race conditions with configuring and
> deconfiguring SCSI devices is that the periph_drivers list was not
> lock protected. The changes to address this are in cam_periph.c
> and the top of cam_xpt.c. and mostly come from Alexander Motin
> at FreeBSD.org. I found and fixed a potential problem for a
> periph refcount going negative and rearranged Alexander's patch
> slightly to get a bit more cohesion.  The general idea here is
> to protect the periph driver list with the cam XPT topology lock.
>
> I also added an assert that we can never look up a periph without
> the SIM mutex being owned. This (barely) covers us in case where a
> periph might want to go away in between cam_periph_find and
> cam_periph_acquire.
>
> The second problem has to do with making sure that the probe sequence
> for finding new devices correctly stops when it detects an error,
> and furthermore, leaves the device queues correctly unfrozen when
> this occurs. This is contained in the large number of changes in
> scsi/scsi_xpt.c:probedone.
>
> The gist of these changes is to stop trying to probe a device that
> has been found to have an uncorrectable error during probe and might
> in fact be deallocated already.
>
> I also more closely followed what Alexander did with ata/ata_xpt.c
> in that by the time you break out of the switch statement at the
> bottom of probedone, you're going to be tearing down the probe periph
> so it seemed to me to be dead code to call back to probestart.
>
> The third problem has to do with making sure that different
> subsystems that do things in an uncoordinated fashion don't get
> surprised by unexpected dealloaction of devices. The da driver
> (scsi_da.c) interacts both with the GEOM and SYSCTL subsystems
> when a device arrives. Unfortunately there is no shared locking
> that coordinate actions between callbacks from the GEOM and
> SYSCTL subsystems in an entirely safe way.
>
> For SYSCTL, increasing the refcount on the CAM periph structure is
> allowed because we know da will be called back later to create the
> SYSCTL tree for this da instance. This keeps the periph from going
> away unexpectedly.  At that point of time in a callback, the validity
> of the periph structure can be rechecked safely.
>
> For GEOM, I haven't specifically done anything, but there is a race
> condition where if you have specified a ioctl entry point in the disk
> structure, GEOM will call back on it during device tasting. There is
> just no existing good way to protect yourself from blowing up without
> adding some extra one-off barrier locking. This is not present in this
> patch set because the default FreeBSD disk driver does not, in fact,
> have an ioctl entry point (but the one at Panasas does).
>
> In order to really test this stuff, a "Fake HBA" and "Fake Disk"
> that I've come up with was used. Using the 'faulty' subclass for it,
> this fake HBA and fake DISK creates 512 fake disks (64 targets with
> 8 luns) but every 255 operations received 'fails' that operation with
> a SELECT TIMEOUT error.  Running repeated and simulataneous
> 'camcontrol rescans' over this bus exercises repeated
> probe/fail/probe/fail scenarios. I've had overnight runs with this
> testing now, whereas previously I would die within minutes.
>
>
> _______________________________________________
> freebsd-scsi@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-scsi
> To unsubscribe, send any mail to "freebsd-scsi-unsubscribe@freebsd.org"
>    


--------------010300040707060501050509
Content-Type: text/plain;
 name="BIG_CAM_PATCH"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="BIG_CAM_PATCH"

diff -ru sys/cam/cam_periph.c /space/FreeBSD/work/sys/cam/cam_periph.c
--- sys/cam/cam_periph.c	2010-04-04 09:35:26.000000000 -0700
+++ /space/FreeBSD/work/sys/cam/cam_periph.c	2010-05-12 21:28:18.000000000 -0700
@@ -185,17 +185,6 @@
 	
 	init_level++;
 
-	xpt_lock_buses();
-	for (p_drv = periph_drivers; *p_drv != NULL; p_drv++) {
-		if (strcmp((*p_drv)->driver_name, name) == 0)
-			break;
-	}
-	xpt_unlock_buses();
-	if (*p_drv == NULL) {
-		printf("cam_periph_alloc: invalid periph name '%s'\n", name);
-		free(periph, M_CAMPERIPH);
-		return (CAM_REQ_INVALID);
-	}
 
 	sim = xpt_path_sim(path);
 	path_id = xpt_path_path_id(path);
@@ -208,7 +197,6 @@
 	periph->periph_oninval = periph_oninvalidate;
 	periph->type = type;
 	periph->periph_name = name;
-	periph->unit_number = camperiphunit(*p_drv, path_id, target_id, lun_id);
 	periph->immediate_priority = CAM_PRIORITY_NONE;
 	periph->refcount = 0;
 	periph->sim = sim;
@@ -216,26 +204,39 @@
 	status = xpt_create_path(&path, periph, path_id, target_id, lun_id);
 	if (status != CAM_REQ_CMP)
 		goto failure;
-
 	periph->path = path;
-	init_level++;
-
-	status = xpt_add_periph(periph);
-
-	if (status != CAM_REQ_CMP)
-		goto failure;
 
+	xpt_lock_buses();
+	for (p_drv = periph_drivers; *p_drv != NULL; p_drv++) {
+		if (strcmp((*p_drv)->driver_name, name) == 0)
+			break;
+	}
+	if (*p_drv == NULL) {
+		printf("cam_periph_alloc: invalid periph name '%s'\n", name);
+		xpt_free_path(periph->path);
+		free(periph, M_CAMPERIPH);
+		xpt_unlock_buses();
+		return (CAM_REQ_INVALID);
+	}
+	periph->unit_number = camperiphunit(*p_drv, path_id, target_id, lun_id);
 	cur_periph = TAILQ_FIRST(&(*p_drv)->units);
 	while (cur_periph != NULL
 	    && cur_periph->unit_number < periph->unit_number)
 		cur_periph = TAILQ_NEXT(cur_periph, unit_links);
-
-	if (cur_periph != NULL)
+	if (cur_periph != NULL) {
+		KASSERT(cur_periph->unit_number != periph->unit_number, ("duplicate units on periph list"));
 		TAILQ_INSERT_BEFORE(cur_periph, periph, unit_links);
-	else {
+	} else {
 		TAILQ_INSERT_TAIL(&(*p_drv)->units, periph, unit_links);
 		(*p_drv)->generation++;
 	}
+	xpt_unlock_buses();
+
+	init_level++;
+
+	status = xpt_add_periph(periph);
+	if (status != CAM_REQ_CMP)
+		goto failure;
 
 	init_level++;
 
@@ -250,10 +251,12 @@
 		/* Initialized successfully */
 		break;
 	case 3:
-		TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links);
 		xpt_remove_periph(periph);
 		/* FALLTHROUGH */
 	case 2:
+		xpt_lock_buses();
+		TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links);
+		xpt_unlock_buses();
 		xpt_free_path(periph->path);
 		/* FALLTHROUGH */
 	case 1:
@@ -288,6 +291,7 @@
 		TAILQ_FOREACH(periph, &(*p_drv)->units, unit_links) {
 			if (xpt_path_comp(periph->path, path) == 0) {
 				xpt_unlock_buses();
+				mtx_assert(periph->sim->mtx, MA_OWNED);
 				return(periph);
 			}
 		}
@@ -322,8 +326,13 @@
 		return;
 
 	xpt_lock_buses();
-	if ((--periph->refcount == 0)
-	 && (periph->flags & CAM_PERIPH_INVALID)) {
+	if (periph->refcount != 0) {
+		periph->refcount--;
+	} else {
+		xpt_print(periph->path, "%s: release %p when refcount is zero\n ", __func__, periph);
+	}
+	if (periph->refcount == 0
+	    && (periph->flags & CAM_PERIPH_INVALID)) {
 		camperiphfree(periph);
 	}
 	xpt_unlock_buses();
diff -ru sys/cam/cam_xpt.c /space/FreeBSD/work/sys/cam/cam_xpt.c
--- sys/cam/cam_xpt.c	2010-05-02 04:16:42.000000000 -0700
+++ /space/FreeBSD/work/sys/cam/cam_xpt.c	2010-05-12 21:34:02.000000000 -0700
@@ -2115,6 +2115,7 @@
 
 	retval = 1;
 
+	xpt_lock_buses();
 	for (periph = (start_periph ? start_periph :
 	     TAILQ_FIRST(&(*pdrv)->units)); periph != NULL;
 	     periph = next_periph) {
@@ -2122,9 +2123,12 @@
 		next_periph = TAILQ_NEXT(periph, unit_links);
 
 		retval = tr_func(periph, arg);
-		if (retval == 0)
+		if (retval == 0) {
+			xpt_unlock_buses();
 			return(retval);
+		}
 	}
+	xpt_unlock_buses();
 	return(retval);
 }
 
@@ -2300,7 +2304,6 @@
 
 	CAM_DEBUG(start_ccb->ccb_h.path, CAM_DEBUG_TRACE, ("xpt_action_default\n"));
 
-
 	switch (start_ccb->ccb_h.func_code) {
 	case XPT_SCSI_IO:
 	{
@@ -2647,7 +2650,9 @@
 			xptedtmatch(cdm);
 			break;
 		case CAM_DEV_POS_PDRV:
+			xpt_lock_buses();
 			xptperiphlistmatch(cdm);
+			xpt_unlock_buses();
 			break;
 		default:
 			cdm->status = CAM_DEV_MATCH_ERROR;
diff -ru sys/cam/scsi/scsi_da.c /space/FreeBSD/work/sys/cam/scsi/scsi_da.c
--- sys/cam/scsi/scsi_da.c	2010-04-02 12:08:45.000000000 -0800
+++ /space/FreeBSD/work/sys/cam/scsi/scsi_da.c	2010-05-12 21:28:18.000000000 -0700
@@ -997,11 +997,6 @@
 		xpt_print(periph->path, "can't remove sysctl context\n");
 	}
 
-	/*
-	 * Nullify our periph pointer here to try and catch
-	 * race conditions in callbacks/downcalls.
-	 */
-	softc->disk->d_drv1 = NULL;
 	disk_destroy(softc->disk);
 	callout_drain(&softc->sendordered_c);
 	free(softc, M_DEVBUF);
@@ -1080,8 +1075,13 @@
 	char tmpstr[80], tmpstr2[80];
 
 	periph = (struct cam_periph *)context;
-	if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+	/*
+	 * periph was held for us when this task was enqueued
+	 */
+	if (periph->flags & CAM_PERIPH_INVALID) {
+		cam_periph_release(periph);
 		return;
+	}
 
 	softc = (struct da_softc *)periph->softc;
 	snprintf(tmpstr, sizeof(tmpstr), "CAM DA unit %d", periph->unit_number);
@@ -1237,29 +1237,6 @@
 	 * Register this media as a disk
 	 */
 
-	mtx_unlock(periph->sim->mtx);
-	softc->disk = disk_alloc();
-	softc->disk->d_open = daopen;
-	softc->disk->d_close = daclose;
-	softc->disk->d_strategy = dastrategy;
-	softc->disk->d_dump = dadump;
-	softc->disk->d_name = "da";
-	softc->disk->d_drv1 = periph;
-	if (cpi.maxio == 0)
-		softc->disk->d_maxsize = DFLTPHYS;	/* traditional default */
-	else if (cpi.maxio > MAXPHYS)
-		softc->disk->d_maxsize = MAXPHYS;	/* for safety */
-	else
-		softc->disk->d_maxsize = cpi.maxio;
-	softc->disk->d_unit = periph->unit_number;
-	softc->disk->d_flags = 0;
-	if ((softc->quirks & DA_Q_NO_SYNC_CACHE) == 0)
-		softc->disk->d_flags |= DISKFLAG_CANFLUSHCACHE;
-	strlcpy(softc->disk->d_ident, cgd->serial_num,
-	    MIN(sizeof(softc->disk->d_ident), cgd->serial_num_len + 1));
-	disk_create(softc->disk, DISK_VERSION);
-	mtx_lock(periph->sim->mtx);
-
 	/*
 	 * Add async callbacks for bus reset and
 	 * bus device reset calls.  I don't bother
@@ -1277,7 +1254,6 @@
 	 * the end of probe.
 	 */
 	(void)cam_periph_hold(periph, PRIBIO);
-	xpt_schedule(periph, CAM_PRIORITY_DEV);
 
 	/*
 	 * Schedule a periodic event to occasionally send an
@@ -1288,6 +1264,31 @@
 	    (DA_DEFAULT_TIMEOUT * hz) / DA_ORDEREDTAG_INTERVAL,
 	    dasendorderedtag, softc);
 
+	mtx_unlock(periph->sim->mtx);
+	softc->disk = disk_alloc();
+	softc->disk->d_open = daopen;
+	softc->disk->d_close = daclose;
+	softc->disk->d_strategy = dastrategy;
+	softc->disk->d_dump = dadump;
+	softc->disk->d_name = "da";
+	softc->disk->d_drv1 = periph;
+	if (cpi.maxio == 0)
+		softc->disk->d_maxsize = DFLTPHYS;	/* traditional default */
+	else if (cpi.maxio > MAXPHYS)
+		softc->disk->d_maxsize = MAXPHYS;	/* for safety */
+	else
+		softc->disk->d_maxsize = cpi.maxio;
+	softc->disk->d_unit = periph->unit_number;
+	softc->disk->d_flags = 0;
+	if ((softc->quirks & DA_Q_NO_SYNC_CACHE) == 0)
+		softc->disk->d_flags |= DISKFLAG_CANFLUSHCACHE;
+	strlcpy(softc->disk->d_ident, cgd->serial_num,
+	    MIN(sizeof(softc->disk->d_ident), cgd->serial_num_len + 1));
+	disk_create(softc->disk, DISK_VERSION);
+	mtx_lock(periph->sim->mtx);
+
+	xpt_schedule(periph, CAM_PRIORITY_DEV);
+
 	return(CAM_REQ_CMP);
 }
 
@@ -1747,6 +1748,7 @@
 			 * Create our sysctl variables, now that we know
 			 * we have successfully attached.
 			 */
+			(void) cam_periph_acquire(periph);	/* increase the refcount */
 			taskqueue_enqueue(taskqueue_thread,&softc->sysctl_task);
 		}
 		softc->state = DA_STATE_NORMAL;	
diff -ru sys/cam/scsi/scsi_xpt.c /space/FreeBSD/work/sys/cam/scsi/scsi_xpt.c
--- sys/cam/scsi/scsi_xpt.c	2010-02-26 18:20:06.000000000 -0800
+++ /space/FreeBSD/work/sys/cam/scsi/scsi_xpt.c	2010-05-12 21:28:18.000000000 -0700
@@ -1005,53 +1005,49 @@
 	return (1);
 }
 
+#define	PCHK(d, f)	cam_periph_error((d), 0, (f), NULL) == ERESTART
+#define	PCHK_S(d, f, s)	\
+	cam_periph_error((d), 0, (f), &(s)->saved_ccb) == ERESTART
+
 static void
 probedone(struct cam_periph *periph, union ccb *done_ccb)
 {
 	probe_softc *softc;
 	struct cam_path *path;
-	u_int32_t  priority;
+	cam_status status;
+	int frozen;
+	u_int32_t priority;
 
 	CAM_DEBUG(done_ccb->ccb_h.path, CAM_DEBUG_TRACE, ("probedone\n"));
 
 	softc = (probe_softc *)periph->softc;
 	path = done_ccb->ccb_h.path;
 	priority = done_ccb->ccb_h.pinfo.priority;
+	status = done_ccb->ccb_h.status & CAM_STATUS_MASK;
+	frozen = (done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0;
 
 	switch (softc->action) {
 	case PROBE_TUR:
-	{
-		if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
-
-			if (cam_periph_error(done_ccb, 0,
-					     SF_NO_PRINT, NULL) == ERESTART)
-				return;
-			else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
-				/* Don't wedge the queue */
-				xpt_release_devq(done_ccb->ccb_h.path,
-						 /*count*/1,
-						 /*run_queue*/TRUE);
-		}
-		PROBE_SET_ACTION(softc, PROBE_INQUIRY);
-		xpt_release_ccb(done_ccb);
-		xpt_schedule(periph, priority);
-		return;
-	}
+		if (status == CAM_REQ_CMP) {
+			PROBE_SET_ACTION(softc, PROBE_INQUIRY);
+			xpt_release_ccb(done_ccb);
+			xpt_schedule(periph, priority);
+			return;
+		}
+		if (PCHK(done_ccb, SF_NO_PRINT))
+			return;
+		status = CAM_REQ_CMP_ERR;
+		break;
 	case PROBE_INQUIRY:
 	case PROBE_FULL_INQUIRY:
 	{
-		if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
+		if (status == CAM_REQ_CMP) {
 			struct scsi_inquiry_data *inq_buf;
-			u_int8_t periph_qual;
 
 			path->device->flags |= CAM_DEV_INQUIRY_DATA_VALID;
 			inq_buf = &path->device->inq_data;
 
-			periph_qual = SID_QUAL(inq_buf);
-
-			switch(periph_qual) {
-			case SID_QUAL_LU_CONNECTED:
-			{
+			if (SID_QUAL(inq_buf) == SID_QUAL_LU_CONNECTED) {
 				u_int8_t len;
 
 				/*
@@ -1069,7 +1065,8 @@
                                                additional_length) + 1;
 				if (softc->action == PROBE_INQUIRY
 				    && len > SHORT_INQUIRY_LENGTH) {
-					PROBE_SET_ACTION(softc, PROBE_FULL_INQUIRY);
+					PROBE_SET_ACTION(softc,
+					    PROBE_FULL_INQUIRY);
 					xpt_release_ccb(done_ccb);
 					xpt_schedule(periph, priority);
 					return;
@@ -1079,31 +1076,29 @@
 
 				scsi_devise_transport(path);
 				if (INQ_DATA_TQ_ENABLED(inq_buf))
-					PROBE_SET_ACTION(softc, PROBE_MODE_SENSE);
+					PROBE_SET_ACTION(softc,
+					    PROBE_MODE_SENSE);
 				else
-					PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_0);
+					PROBE_SET_ACTION(softc,
+					    PROBE_SERIAL_NUM_0);
 
-				if (path->device->flags & CAM_DEV_UNCONFIGURED) {
-					path->device->flags &= ~CAM_DEV_UNCONFIGURED;
+				if (path->device->flags &
+				    CAM_DEV_UNCONFIGURED) {
+					path->device->flags &=
+					    ~CAM_DEV_UNCONFIGURED;
 					xpt_acquire_device(path->device);
 				}
 				xpt_release_ccb(done_ccb);
 				xpt_schedule(periph, priority);
 				return;
 			}
-			default:
-				break;
+		} else {
+			int flags = SF_RETRY_UA;
+			if (done_ccb->ccb_h.target_lun > 0)
+			    flags |= SF_QUIET_IR;
+			if (PCHK_S(done_ccb, flags, softc)) {
+				return;
 			}
-		} else if (cam_periph_error(done_ccb, 0,
-					    done_ccb->ccb_h.target_lun > 0
-					    ? SF_RETRY_UA|SF_QUIET_IR
-					    : SF_RETRY_UA,
-					    &softc->saved_ccb) == ERESTART) {
-			return;
-		} else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
-			/* Don't wedge the queue */
-			xpt_release_devq(done_ccb->ccb_h.path, /*count*/1,
-					 /*run_queue*/TRUE);
 		}
 		/*
 		 * If we get to this point, we got an error status back
@@ -1119,7 +1114,7 @@
 			/* Send the async notification. */
 			xpt_async(AC_LOST_DEVICE, path, NULL);
 
-		xpt_release_ccb(done_ccb);
+		status = CAM_REQ_CMP_ERR;
 		break;
 	}
 	case PROBE_MODE_SENSE:
@@ -1129,7 +1124,7 @@
 
 		csio = &done_ccb->csio;
 		mode_hdr = (struct scsi_mode_header_6 *)csio->data_ptr;
-		if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
+		if (status == CAM_REQ_CMP) {
 			struct scsi_control_page *page;
 			u_int8_t *offset;
 
@@ -1137,20 +1132,18 @@
 			    + mode_hdr->blk_desc_len;
 			page = (struct scsi_control_page *)offset;
 			path->device->queue_flags = page->queue_flags;
-		} else if (cam_periph_error(done_ccb, 0,
-					    SF_RETRY_UA|SF_NO_PRINT,
-					    &softc->saved_ccb) == ERESTART) {
+		} else if (PCHK_S(done_ccb, SF_RETRY_UA|SF_NO_PRINT, softc)) {
 			return;
-		} else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
-			/* Don't wedge the queue */
-			xpt_release_devq(done_ccb->ccb_h.path,
-					 /*count*/1, /*run_queue*/TRUE);
 		}
-		xpt_release_ccb(done_ccb);
 		free(mode_hdr, M_CAMXPT);
-		PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_0);
-		xpt_schedule(periph, priority);
-		return;
+		if (status == CAM_REQ_CMP) {
+			PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_0);
+			xpt_release_ccb(done_ccb);
+			xpt_schedule(periph, priority);
+			return;
+		}
+		status = CAM_REQ_CMP_ERR;
+		break;
 	}
 	case PROBE_SERIAL_NUM_0:
 	{
@@ -1167,8 +1160,7 @@
 			/*
 			 * Don't process the command as it was never sent
 			 */
-		} else if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP
-		    && (page_list->length > 0)) {
+		} else if (status == CAM_REQ_CMP && page_list->length > 0) {
 			length = min(page_list->length,
 			    SVPD_SUPPORTED_PAGES_SIZE);
 			for (i = 0; i < length; i++) {
@@ -1178,22 +1170,16 @@
 					break;
 				}
 			}
-		} else if (cam_periph_error(done_ccb, 0,
-					    SF_RETRY_UA|SF_NO_PRINT,
-					    &softc->saved_ccb) == ERESTART) {
+		} else if (PCHK_S(done_ccb, SF_RETRY_UA|SF_NO_PRINT, softc)) {
 			return;
-		} else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
-			/* Don't wedge the queue */
-			xpt_release_devq(done_ccb->ccb_h.path, /*count*/1,
-					 /*run_queue*/TRUE);
 		}
 
 		if (page_list != NULL)
 			free(page_list, M_CAMXPT);
 
 		if (serialnum_supported) {
-			xpt_release_ccb(done_ccb);
 			PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_1);
+			xpt_release_ccb(done_ccb);
 			xpt_schedule(periph, priority);
 			return;
 		}
@@ -1201,7 +1187,6 @@
 		csio->data_ptr = NULL;
 		/* FALLTHROUGH */
 	}
-
 	case PROBE_SERIAL_NUM_1:
 	{
 		struct ccb_scsiio *csio;
@@ -1210,7 +1195,11 @@
 		int changed;
 		int have_serialnum;
 
-		changed = 1;
+		if (status == CAM_REQ_CMP)
+			changed = 1;
+		else {
+			changed = 0;
+		}
 		have_serialnum = 0;
 		csio = &done_ccb->csio;
 		priority = done_ccb->ccb_h.pinfo.priority;
@@ -1228,9 +1217,7 @@
 			/*
 			 * Don't process the command as it was never sent
 			 */
-		} else if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP
-			&& (serial_buf->length > 0)) {
-
+		} else if (status == CAM_REQ_CMP && serial_buf->length > 0) {
 			have_serialnum = 1;
 			path->device->serial_num =
 				(u_int8_t *)malloc((serial_buf->length + 1),
@@ -1244,20 +1231,14 @@
 				path->device->serial_num[serial_buf->length]
 				    = '\0';
 			}
-		} else if (cam_periph_error(done_ccb, 0,
-					    SF_RETRY_UA|SF_NO_PRINT,
-					    &softc->saved_ccb) == ERESTART) {
+		} else if (PCHK_S(done_ccb, SF_RETRY_UA|SF_NO_PRINT, softc)) {
 			return;
-		} else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
-			/* Don't wedge the queue */
-			xpt_release_devq(done_ccb->ccb_h.path, /*count*/1,
-					 /*run_queue*/TRUE);
 		}
 
 		/*
 		 * Let's see if we have seen this device before.
 		 */
-		if ((softc->flags & PROBE_INQUIRY_CKSUM) != 0) {
+		if (changed && (softc->flags & PROBE_INQUIRY_CKSUM) != 0) {
 			MD5_CTX context;
 			u_int8_t digest[16];
 
@@ -1298,33 +1279,33 @@
 			 * settings to set the device up.
 			 */
 			proberequestdefaultnegotiation(periph);
-			xpt_release_ccb(done_ccb);
 
 			/*
 			 * Perform a TUR to allow the controller to
 			 * perform any necessary transfer negotiation.
 			 */
 			PROBE_SET_ACTION(softc, PROBE_TUR_FOR_NEGOTIATION);
+			xpt_release_ccb(done_ccb);
 			xpt_schedule(periph, priority);
 			return;
 		}
-		xpt_release_ccb(done_ccb);
+		status = CAM_REQ_CMP_ERR;
 		break;
 	}
 	case PROBE_TUR_FOR_NEGOTIATION:
-		if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
+		if (status != CAM_REQ_CMP) {
 			DELAY(500000);
-			if (cam_periph_error(done_ccb, 0, SF_RETRY_UA,
-			    NULL) == ERESTART)
+			if (PCHK(done_ccb, SF_RETRY_UA))
 				return;
 		}
-	/* FALLTHROUGH */
+		/* FALLTHROUGH */
 	case PROBE_DV_EXIT:
-		if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
-			/* Don't wedge the queue */
-			xpt_release_devq(done_ccb->ccb_h.path, /*count*/1,
-					 /*run_queue*/TRUE);
+
+		if (status != CAM_REQ_CMP) {
+			status = CAM_REQ_CMP_ERR;
+			break;
 		}
+
 		/*
 		 * Do Domain Validation for lun 0 on devices that claim
 		 * to support Synchronous Transfer modes.
@@ -1336,8 +1317,8 @@
 			CAM_DEBUG(periph->path, CAM_DEBUG_INFO,
 			    ("Begin Domain Validation\n"));
 			path->device->flags |= CAM_DEV_IN_DV;
-			xpt_release_ccb(done_ccb);
 			PROBE_SET_ACTION(softc, PROBE_INQUIRY_BASIC_DV1);
+			xpt_release_ccb(done_ccb);
 			xpt_schedule(periph, priority);
 			return;
 		}
@@ -1358,7 +1339,6 @@
 			xpt_async(AC_FOUND_DEVICE, done_ccb->ccb_h.path,
 				  done_ccb);
 		}
-		xpt_release_ccb(done_ccb);
 		break;
 	case PROBE_INQUIRY_BASIC_DV1:
 	case PROBE_INQUIRY_BASIC_DV2:
@@ -1366,20 +1346,21 @@
 		struct scsi_inquiry_data *nbuf;
 		struct ccb_scsiio *csio;
 
-		if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
-			/* Don't wedge the queue */
-			xpt_release_devq(done_ccb->ccb_h.path, /*count*/1,
-					 /*run_queue*/TRUE);
-		}
 		csio = &done_ccb->csio;
 		nbuf = (struct scsi_inquiry_data *)csio->data_ptr;
+		if (status != CAM_REQ_CMP) {
+			status = CAM_REQ_CMP_ERR;
+			free(nbuf, M_CAMXPT);
+			break;
+		}
 		if (bcmp(nbuf, &path->device->inq_data, SHORT_INQUIRY_LENGTH)) {
 			xpt_print(path,
 			    "inquiry data fails comparison at DV%d step\n",
 			    softc->action == PROBE_INQUIRY_BASIC_DV1 ? 1 : 2);
 			if (proberequestbackoff(periph, path->device)) {
 				path->device->flags &= ~CAM_DEV_IN_DV;
-				PROBE_SET_ACTION(softc, PROBE_TUR_FOR_NEGOTIATION);
+				PROBE_SET_ACTION(softc,
+				    PROBE_TUR_FOR_NEGOTIATION);
 			} else {
 				/* give up */
 				PROBE_SET_ACTION(softc, PROBE_DV_EXIT);
@@ -1413,27 +1394,35 @@
 			xpt_async(AC_FOUND_DEVICE, done_ccb->ccb_h.path,
 				  done_ccb);
 		}
-		xpt_release_ccb(done_ccb);
 		break;
 	}
 	case PROBE_INVALID:
-		CAM_DEBUG(done_ccb->ccb_h.path, CAM_DEBUG_INFO,
-		    ("probedone: invalid action state\n"));
+		status = CAM_REQ_CMP_ERR;
+		xpt_print(done_ccb->ccb_h.path, "%s: invalid action state\n",
+		    __func__);
+		break;
 	default:
+		status = CAM_REQ_CMP_ERR;
+		xpt_print(done_ccb->ccb_h.path, "%s: unknown action state %x\n",
+		    __func__, softc->action);
 		break;
 	}
-	done_ccb = (union ccb *)TAILQ_FIRST(&softc->request_ccbs);
-	TAILQ_REMOVE(&softc->request_ccbs, &done_ccb->ccb_h, periph_links.tqe);
-	done_ccb->ccb_h.status = CAM_REQ_CMP;
-	xpt_done(done_ccb);
-	if (TAILQ_FIRST(&softc->request_ccbs) == NULL) {
-		cam_release_devq(periph->path,
-		    RELSIM_RELEASE_RUNLEVEL, 0, CAM_RL_XPT + 1, FALSE);
-		cam_periph_invalidate(periph);
-		cam_periph_release_locked(periph);
-	} else {
-		probeschedule(periph);
-	}
+	if (frozen) {
+		xpt_release_devq_rl(done_ccb->ccb_h.path,
+		    CAM_PRIORITY_TO_RL(done_ccb->ccb_h.pinfo.priority),
+		    1, TRUE);
+	}
+	xpt_release_ccb(done_ccb);
+	while ((done_ccb = (union ccb *)TAILQ_FIRST(&softc->request_ccbs))) {
+		TAILQ_REMOVE(&softc->request_ccbs,
+		    &done_ccb->ccb_h, periph_links.tqe);
+		done_ccb->ccb_h.status = status;
+		xpt_done(done_ccb);
+	}
+	cam_release_devq(periph->path,
+	    RELSIM_RELEASE_RUNLEVEL, 0, CAM_RL_XPT + 1, FALSE);
+	cam_periph_invalidate(periph);
+	cam_periph_release_locked(periph);
 }
 
 static void

--------------010300040707060501050509--



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