From owner-freebsd-scsi@FreeBSD.ORG Tue May 25 02:44:41 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 B4A5B1065673 for ; Tue, 25 May 2010 02:44:41 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id 3FF358FC1D for ; Tue, 25 May 2010 02:44:40 +0000 (UTC) Received: by fxm4 with SMTP id 4so3958968fxm.13 for ; Mon, 24 May 2010 19:44:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:subject:references:in-reply-to :x-enigmail-version:content-type:content-transfer-encoding; bh=cLPqdaHTUWSg5BSTG4GSJtLrQMmdg0WUedbyJmuStrg=; b=KvQS8688wo+7v7ClKfwbTGlAbuMsjjAty5kuXJO8jRG47II/nRUe4Or4xj8Id3EC35 7eg9bLdP52iClmbkHD9qrR9U4hYFubKEVsFZkFAE8mEEM+cFb+5KIO4X3PzGj8SDxK2a w9ZQGIY3qjRAWwZMNEpYl15z5ZHaEciDxteI8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:x-enigmail-version:content-type :content-transfer-encoding; b=Z/CsqtYDEIQBSFmBpGUDMlNvXGetI4YpTgb53MhkOGv6GJI570WIirNLxBNGywzr7i V8jdcyRRmOSLqzMDweLBUWnKpjbOtB75awSQvQrf3vyUv1i8L808ON+CWZ6jPjZ7zBBw TV8+rpwZhTDRgkFQ1BOB6gMoFUNOpfpPofKnI= Received: by 10.223.50.68 with SMTP id y4mr5518874faf.51.1274755480227; Mon, 24 May 2010 19:44:40 -0700 (PDT) Received: from mavbook.mavhome.dp.ua (pc.mavhome.dp.ua [212.86.226.226]) by mx.google.com with ESMTPS id u12sm22499343fah.16.2010.05.24.19.44.39 (version=SSLv3 cipher=RC4-MD5); Mon, 24 May 2010 19:44:39 -0700 (PDT) Sender: Alexander Motin Message-ID: <4BFB3985.1030301@FreeBSD.org> Date: Tue, 25 May 2010 05:44:21 +0300 From: Alexander Motin User-Agent: Thunderbird 2.0.0.24 (X11/20100402) MIME-Version: 1.0 To: Matthew Jacob , freebsd-scsi@freebsd.org References: <4BEB87B8.1070104@feral.com> In-Reply-To: X-Enigmail-Version: 0.96.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: 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: Tue, 25 May 2010 02:44:41 -0000 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