From owner-freebsd-scsi Sun Dec 8 7:27:13 2002 Delivered-To: freebsd-scsi@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9E3EA37B401 for ; Sun, 8 Dec 2002 07:27:11 -0800 (PST) Received: from comp.chem.msu.su (comp-ext.chem.msu.su [158.250.32.157]) by mx1.FreeBSD.org (Postfix) with ESMTP id D4FE643EA9 for ; Sun, 8 Dec 2002 07:27:09 -0800 (PST) (envelope-from yar@comp.chem.msu.su) Received: from comp.chem.msu.su (localhost [127.0.0.1]) by comp.chem.msu.su (8.12.3/8.12.3) with ESMTP id gB8FR7m2080085; Sun, 8 Dec 2002 18:27:07 +0300 (MSK) (envelope-from yar@comp.chem.msu.su) Received: (from yar@localhost) by comp.chem.msu.su (8.12.3/8.12.3/Submit) id gB8FR6kp080084; Sun, 8 Dec 2002 18:27:06 +0300 (MSK) Date: Sun, 8 Dec 2002 18:27:06 +0300 From: Yar Tikhiy To: Nate Lawson Cc: freebsd-scsi@freebsd.org Subject: Re: {da,sa,...}open bug? Message-ID: <20021208182706.B75509@comp.chem.msu.su> References: <20021206145942.I80257@comp.chem.msu.su> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from nate@root.org on Fri, Dec 06, 2002 at 11:23:56AM -0800 Sender: owner-freebsd-scsi@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org On Fri, Dec 06, 2002 at 11:23:56AM -0800, Nate Lawson wrote: > On Fri, 6 Dec 2002, Yar Tikhiy wrote: > > Yet another daopen() issue came to my attention. The DA_OPEN_FLAG bit > > won't be reset back to 0 if daopen() has failed, thus leaving the device > > marked as open. Isn't the below patch necessary? > > > > The rest of scsi_* modules seem to not have this bug. > > > > -- > > Yar > > > > Index: scsi_da.c > > =================================================================== > > RCS file: /home/ncvs/src/sys/cam/scsi/scsi_da.c,v > > retrieving revision 1.116 > > diff -u -r1.116 scsi_da.c > > --- scsi_da.c 29 Nov 2002 15:40:10 -0000 1.116 > > +++ scsi_da.c 6 Dec 2002 11:49:57 -0000 > > @@ -612,6 +612,7 @@ > > if ((softc->flags & DA_FLAG_PACK_REMOVABLE) != 0) > > daprevent(periph, PR_PREVENT); > > } else { > > + softc->flags &= ~DA_FLAG_OPEN; > > cam_periph_release(periph); > > } > > cam_periph_unlock(periph); > > You are correct. Ok to commit this w/ re@ approval The other periphs > don't use this flag. While writing a message to re@, I thought if we were propagating bad style with such patches. Wouldn't it be better to set the open flag and acquire the periph if and only if returning success? The open flag seems to be of no use before the return from daopen(). I.e., Index: scsi_da.c =================================================================== RCS file: /home/ncvs/src/sys/cam/scsi/scsi_da.c,v retrieving revision 1.116 diff -u -r1.116 scsi_da.c --- scsi_da.c 29 Nov 2002 15:40:10 -0000 1.116 +++ scsi_da.c 8 Dec 2002 15:12:51 -0000 @@ -545,10 +545,6 @@ if ((error = cam_periph_lock(periph, PRIBIO|PCATCH)) != 0) return (error); /* error code from tsleep */ - if (cam_periph_acquire(periph) != CAM_REQ_CMP) - return(ENXIO); - softc->flags |= DA_FLAG_OPEN; - if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) { /* Invalidate our pack information. */ disk_invalidate(&softc->disk); @@ -607,12 +603,15 @@ softc->device_stats.flags &= ~DEVSTAT_BS_UNAVAILABLE; } } + + if (error == 0) + if (cam_periph_acquire(periph) != CAM_REQ_CMP) + error = ENXIO; if (error == 0) { + softc->flags |= DA_FLAG_OPEN; if ((softc->flags & DA_FLAG_PACK_REMOVABLE) != 0) daprevent(periph, PR_PREVENT); - } else { - cam_periph_release(periph); } cam_periph_unlock(periph); return (error); =================================================================== Notice how we kill two birds with one stone: The "no unlock on error" bug is gone as well. -- Yar To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-scsi" in the body of the message