Date: Tue, 25 May 2010 05:44:21 +0300 From: Alexander Motin <mav@FreeBSD.org> To: Matthew Jacob <mj@feral.com>, freebsd-scsi@freebsd.org Subject: Re: patches for CAM SCSI probing, etc. Message-ID: <4BFB3985.1030301@FreeBSD.org> In-Reply-To: <mailpost.1274725877.820236.58062.mailing.freebsd.scsi@FreeBSD.cs.nctu.edu.tw> References: <4BEB87B8.1070104@feral.com> <mailpost.1274725877.820236.58062.mailing.freebsd.scsi@FreeBSD.cs.nctu.edu.tw>
next in thread | previous in thread | raw e-mail | index | archive | help
Matthew Jacob wrote:
>> 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.
In this code
- 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);
}
you are slightly changing semantics of device probe. Previously, probe
call means warranty that device will be probed from the beginning after
the moment of request. It is very important for ATA, as probe function
also initializes device, that is mandatory if probe is called as result
of device reset. SCSI devices probably don't need that initialization,
but what if probe was called due to inquiry change status received
during probe sequence running? Are you sure you won't loose events here?
--
Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4BFB3985.1030301>