Date: Sat, 2 Oct 1999 12:57:07 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Peter Wemm <peter@netplex.com.au> Cc: phk@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: BEWARE: CAM changes broke AHC! Message-ID: <Pine.BSF.4.10.9910021208310.2026-100000@alphplex.bde.org> In-Reply-To: <19991001220348.A90A71CA7@overcee.netplex.com.au>
next in thread | previous in thread | raw e-mail | index | archive | help
> I am particularly suspicious about this:
>
> @@ -284,26 +283,14 @@
> return (error); /* error code from tsleep */
> }
>
> - if ((softc->flags & DA_FLAG_OPEN) == 0) {
> - if (cam_periph_acquire(periph) != CAM_REQ_CMP)
> - return(ENXIO);
> - softc->flags |= DA_FLAG_OPEN;
> - }
> + if (cam_periph_acquire(periph) != CAM_REQ_CMP)
> + return(ENXIO);
> + softc->flags |= DA_FLAG_OPEN;
>
> At first glance, it would appear it's re-inquiring on each open instead of
> the first open, including while it's mounted. I wasn't sure, so rather than
> risk disks, I backed the lot out and it worked again.
daopen() is now supposed to be called only if the physical device is not
already open for some subdevice. However, some old races seem to be back.
Suppose there are concurrent opens on the same device. Both will find
dsisopen() == 0 in diskopen(), since dsopen() is not called until after
daopen() completes, so both will enter daopen(), but daopen() is no longer
designed to handle concurrent opens. The first one will obtain a lock
(slightly before the above code) and the second one will sleep. The second
one will continue in daopen() after the first daopen() completes, and will
apparently do some damage. One possible source of damage is clobbering the
label while the first of the two opens is still using it. There are a lot
of i/o's which will block and allow the opens to get in each others way.
The old version unnecessarily did a full physical open for all non-first
opens, not just for racing ones, but it had no problems with the label since
the label was a local variable.
Fix: move the locking up to diskopen().
There may be similar problems for last-closes. There should be locks to
prevent opens while last-closes are in progress. Note that checks of
vcount() in vfs don't provide sufficent locking for multiplexed physical
devices like disks and and sio/cua, since concurrent last-closes are
possible for different subdevices.
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" 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.10.9910021208310.2026-100000>