Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 25 Nov 2002 13:43:02 +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:  <20021125134302.D14452@comp.chem.msu.su>
In-Reply-To: <Pine.BSF.4.21.0211221113280.71270-100000@root.org>; from nate@root.org on Fri, Nov 22, 2002 at 11:14:47AM -0800
References:  <20021122212846.C60810@comp.chem.msu.su> <Pine.BSF.4.21.0211221113280.71270-100000@root.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Nate,

On Fri, Nov 22, 2002 at 11:14:47AM -0800, Nate Lawson wrote:
> On Fri, 22 Nov 2002, Yar Tikhiy wrote:
> > The XXopen() methods of the da, sa etc drivers get a reference to
> > the peripheral ("periph") through cam_periph_acquire() and don't
> > release it on error exit.  I suspect this leads to CAM wedge if the
> > device is removed when its driver is in XXopen().  That is because
> > the "periph" stays acquired (refcount > 0), but the XXopen() caller
> > wouldn't invoke XXclose() subsequently since it sees XXopen() having
> > failed.  Thus CAM won't ever "forget" the device removed.
> > 
> > Moreover, after I had patched daopen() to release the peripheral
> > if returning non-zero error code, the CAM wedge has gone.
> 
> Your approach sounds correct to me.  open should call cam_periph_release
> if it errors out after cam_periph_acquire.  Care to submit a patch?

Certainly!  The patch is at the end of this message.  I can commit
it if it looks right to you.

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.

The current code can tolerate the above problems, so I'd rather not
fix working code when facing the upcoming release.  That's why the
patch is for scsi_da.c only, which appears the only module suffering
from the problem discussed before.  However, I'd like to get rid of
the mentioned oddities after 5.0-RELEASE is out.

-- 
Yar

--- ../scsi.orig/scsi_da.c	Sat Nov 16 12:07:30 2002
+++ scsi_da.c	Sat Nov 23 14:36:43 2002
@@ -609,7 +609,8 @@
 	if (error == 0) {
 		if ((softc->flags & DA_FLAG_PACK_REMOVABLE) != 0)
 			daprevent(periph, PR_PREVENT);
-	}
+	} else
+		cam_periph_release(periph);
 	cam_periph_unlock(periph);
 	return (error);
 }

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?20021125134302.D14452>