Date: Tue, 10 Dec 2002 13:29:55 -0800 (PST)
From: Nate Lawson <nate@root.org>
To: Yar Tikhiy <yar@FreeBSD.ORG>
Cc: "Kenneth D. Merry" <ken@kdm.org>, freebsd-scsi@FreeBSD.ORG
Subject: Re: {da,sa,...}open bug?
Message-ID: <Pine.BSF.4.21.0212101325030.28346-100000@root.org>
In-Reply-To: <20021210171528.B27181@comp.chem.msu.su>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 10 Dec 2002, Yar Tikhiy wrote:
> On Mon, Dec 09, 2002 at 01:50:14PM -0800, Nate Lawson wrote:
> > NULL check should be moved inside the splsoftcam to ensure exclusive
> > access. In the future, we'll use mutex(9) for implementing lock and
> > acquire.
> >
> > --- /sys/cam/cam_periph.c 14 Nov 2002 05:35:57 -0000 1.43
> > +++ /sys/cam/cam_periph.c 9 Dec 2002 21:38:02 -0000
> > @@ -262,15 +262,17 @@
> > cam_periph_acquire(struct cam_periph *periph)
> > {
> > int s;
> > + cam_status status;
> >
> > - if (periph == NULL)
> > - return(CAM_REQ_CMP_ERR);
> > -
> > + cam_status = CAM_REQ_CMP;
> > s = splsoftcam();
> > - periph->refcount++;
> > + if (periph != NULL)
> > + periph->refcount++;
> > + else
> > + status = CAM_REQ_CMP_ERR;
> > splx(s);
> >
> > - return(CAM_REQ_CMP);
> > + return (status);
> > }
> >
> > void
>
> Excuse me, but I see little reason in this patch. "periph" is a
> function argument, so its value cannot be changed anywhere else,
> so there is no need for splsoftcam() before the NULL check.
I guess this makes sense since it's too late by the point where *periph
has been pushed on the stack and acquire called. My concern was the (very
small) possibility that between the NULL check and refcount++, an
interrupt could happen and free the periph. But my patch does not fix
this, as you point out.
-Nate
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?Pine.BSF.4.21.0212101325030.28346-100000>