From owner-freebsd-scsi@FreeBSD.ORG Mon May 24 18:30:59 2010 Return-Path: Delivered-To: freebsd-scsi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3BE471065672 for ; Mon, 24 May 2010 18:30:59 +0000 (UTC) (envelope-from mj@feral.com) Received: from ns1.feral.com (ns1.feral.com [192.67.166.1]) by mx1.freebsd.org (Postfix) with ESMTP id 5BB058FC13 for ; Mon, 24 May 2010 18:30:58 +0000 (UTC) Received: from [192.168.221.2] (remotevpn [192.168.221.2]) by ns1.feral.com (8.14.3/8.14.3) with ESMTP id o4OIUlvC090361 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO) for ; Mon, 24 May 2010 11:30:57 -0700 (PDT) (envelope-from mj@feral.com) Message-ID: <4BFAC5D2.2070502@feral.com> Date: Mon, 24 May 2010 11:30:42 -0700 From: Matthew Jacob Organization: Feral Software User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: freebsd-scsi@freebsd.org References: <4BEB87B8.1070104@feral.com> In-Reply-To: <4BEB87B8.1070104@feral.com> Content-Type: multipart/mixed; boundary="------------010300040707060501050509" X-Greylist: Sender DNS name whitelisted, not delayed by milter-greylist-4.2.3 (ns1.feral.com [192.168.221.1]); Mon, 24 May 2010 11:30:58 -0700 (PDT) X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Subject: Re: patches for CAM SCSI probing, etc. X-BeenThere: freebsd-scsi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SCSI subsystem List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 May 2010 18:30:59 -0000 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--