Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 12 May 2010 22:01:44 -0700
From:      Matthew Jacob <mj@feral.com>
To:        freebsd-scsi@freebsd.org
Subject:   patches for CAM SCSI probing, etc.
Message-ID:  <4BEB87B8.1070104@feral.com>

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

[ 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.


--------------010007070401000608000906
Content-Type: text/plain;
 name="BIG_CAM_PATCH"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 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

--------------010007070401000608000906--



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