Date: Fri, 29 Nov 2002 22:38:17 +0300
From: Yar Tikhiy <yar@freebsd.org>
To: Nate Lawson <nate@root.org>
Cc: freebsd-scsi@freebsd.org
Subject: Re: {da,sa,...}open bug?
Message-ID: <20021129223817.D34288@comp.chem.msu.su>
In-Reply-To: <Pine.BSF.4.21.0211251208350.83036-100000@root.org>; from nate@root.org on Mon, Nov 25, 2002 at 12:10:14PM -0800
References: <20021125134302.D14452@comp.chem.msu.su> <Pine.BSF.4.21.0211251208350.83036-100000@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 25, 2002 at 12:10:14PM -0800, Nate Lawson wrote:
> On Mon, 25 Nov 2002, Yar Tikhiy wrote:
>
> > While preparing the fix, I noticed an additional couple of oddities.
> > First, files under sys/cam/scsi are inconsistent as to the order of
> > calling cam_periph_release() and cam_periph_unlock(): Some of them
> > will call cam_periph_release() first, and the others will call it second.
> > Then, there's a number of places in the code where cam_periph_unlock()
> > won't be called before return on a cam_periph_acquire() error, though
> > the "periph" has been locked.
>
> I think this should be fixed. Please submit a patch for this.
Here it is. It a) reorders unlock()'s and release()'s where
necessary, b) adds missing unlock()'s, and finally c) changes
"return(error)" to "return(0)" where "error" will be always 0.
The latter is essentially a style fix, but it is important
WRT the discussed necessity to release a peripheral on errors.
Having no "if (error) cam_periph_release(periph)" before such
returns would be confusing.
To Nate: If the patch looks good to you, please just say OK, and
I'll do the dirty work of obtaining the high approval and committing.
--
Yar
Index: scsi_cd.c
===================================================================
RCS file: /home/ncvs/src/sys/cam/scsi/scsi_cd.c,v
retrieving revision 1.68
diff -u -r1.68 scsi_cd.c
--- scsi_cd.c 23 Nov 2002 22:51:50 -0000 1.68
+++ scsi_cd.c 29 Nov 2002 19:16:19 -0000
@@ -903,16 +903,18 @@
splx(s);
- if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+ if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
+ cam_periph_unlock(periph);
return(ENXIO);
+ }
cdprevent(periph, PR_PREVENT);
/* find out the size */
if ((error = cdsize(dev, &size)) != 0) {
cdprevent(periph, PR_ALLOW);
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return(error);
}
@@ -931,7 +933,7 @@
CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("leaving cdopen\n"));
- return (error);
+ return (0);
}
static int
@@ -959,8 +961,8 @@
*/
softc->device_stats.flags |= DEVSTAT_BS_UNAVAILABLE;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return (0);
}
Index: scsi_ch.c
===================================================================
RCS file: /home/ncvs/src/sys/cam/scsi/scsi_ch.c,v
retrieving revision 1.31
diff -u -r1.31 scsi_ch.c
--- scsi_ch.c 28 Sep 2002 17:14:05 -0000 1.31
+++ scsi_ch.c 29 Nov 2002 19:16:19 -0000
@@ -438,8 +438,10 @@
splx(s);
if ((softc->flags & CH_FLAG_OPEN) == 0) {
- if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+ if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
+ cam_periph_unlock(periph);
return(ENXIO);
+ }
softc->flags |= CH_FLAG_OPEN;
}
@@ -448,14 +450,14 @@
*/
if ((error = chgetparams(periph)) != 0) {
softc->flags &= ~CH_FLAG_OPEN;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return(error);
}
cam_periph_unlock(periph);
- return(error);
+ return(0);
}
static int
@@ -478,8 +480,8 @@
softc->flags &= ~CH_FLAG_OPEN;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return(0);
}
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 29 Nov 2002 19:16:20 -0000
@@ -545,8 +545,10 @@
if ((error = cam_periph_lock(periph, PRIBIO|PCATCH)) != 0)
return (error); /* error code from tsleep */
- if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+ if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
+ cam_periph_unlock(periph);
return(ENXIO);
+ }
softc->flags |= DA_FLAG_OPEN;
if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) {
@@ -696,8 +698,8 @@
}
softc->flags &= ~DA_FLAG_OPEN;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return (0);
}
Index: scsi_pass.c
===================================================================
RCS file: /home/ncvs/src/sys/cam/scsi/scsi_pass.c,v
retrieving revision 1.34
diff -u -r1.34 scsi_pass.c
--- scsi_pass.c 15 Aug 2002 20:54:03 -0000 1.34
+++ scsi_pass.c 29 Nov 2002 19:16:20 -0000
@@ -384,14 +384,16 @@
splx(s);
if ((softc->flags & PASS_FLAG_OPEN) == 0) {
- if (cam_periph_acquire(periph) != CAM_REQ_CMP)
+ if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
+ cam_periph_unlock(periph);
return(ENXIO);
+ }
softc->flags |= PASS_FLAG_OPEN;
}
cam_periph_unlock(periph);
- return (error);
+ return (0);
}
static int
@@ -412,8 +414,8 @@
softc->flags &= ~PASS_FLAG_OPEN;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return (0);
}
Index: scsi_pt.c
===================================================================
RCS file: /home/ncvs/src/sys/cam/scsi/scsi_pt.c,v
retrieving revision 1.33
diff -u -r1.33 scsi_pt.c
--- scsi_pt.c 15 Aug 2002 20:54:03 -0000 1.33
+++ scsi_pt.c 29 Nov 2002 19:16:20 -0000
@@ -198,8 +198,8 @@
return (error); /* error code from tsleep */
softc->flags &= ~PT_FLAG_OPEN;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return (0);
}
Index: scsi_sa.c
===================================================================
RCS file: /home/ncvs/src/sys/cam/scsi/scsi_sa.c,v
retrieving revision 1.84
diff -u -r1.84 scsi_sa.c
--- scsi_sa.c 14 Nov 2002 05:03:11 -0000 1.84
+++ scsi_sa.c 29 Nov 2002 19:16:20 -0000
@@ -640,8 +640,8 @@
if ((softc->flags & SA_FLAG_TAPE_MOUNTED) == 0)
sareservereleaseunit(periph, FALSE);
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return (error);
}
Index: scsi_ses.c
===================================================================
RCS file: /home/ncvs/src/sys/cam/scsi/scsi_ses.c,v
retrieving revision 1.23
diff -u -r1.23 scsi_ses.c
--- scsi_ses.c 9 Nov 2002 12:55:04 -0000 1.23
+++ scsi_ses.c 29 Nov 2002 19:16:21 -0000
@@ -488,8 +488,8 @@
softc->ses_flags &= ~SES_FLAG_OPEN;
- cam_periph_unlock(periph);
cam_periph_release(periph);
+ cam_periph_unlock(periph);
return (0);
}
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-scsi" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021129223817.D34288>