Date: Sun, 27 Apr 1997 16:14:52 +0100 (BST) From: Doug Rabson <dfr@nlsystems.com> To: KATO Takenori <kato@eclogite.eps.nagoya-u.ac.jp> Cc: CVS-committers@FreeBSD.org, cvs-all@FreeBSD.org, cvs-sys@FreeBSD.org Subject: Re: cvs commit: src/sys/miscfs/union union_vnops.c Message-ID: <Pine.BSF.3.95q.970427160802.346H-100000@herring.nlsystems.com> In-Reply-To: <199704271044.TAA03480@gneiss.eps.nagoya-u.ac.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 27 Apr 1997, KATO Takenori wrote:
> From: Doug Rabson <dfr@nlsystems.com>
> Subject: Re: cvs commit: src/sys/miscfs/union union_vnops.c
> Date: Sat, 26 Apr 1997 19:06:50 +0100 (BST)
>
> > I don't think this is right. I just had a lockmgr panic using the 1.28
> > version of this file (the one which played with the flags). It turned out
> > that update had the uppervp locked and the union node should have locked
> > the uppervp itself which would have waited until update had finished
> > before continuing. In the same situation for 1.29 we would not lock
> > uppervp at all which just seems wrong.
>
> I will undo 1.29. I will replace 1.28 with new change after I make
> new fix.
>
> I consider that the problems related to 1.28 are that (1) 4.4BSD-Lite2
> assumes that vn_lock (or VOP_LOCK) is not called after VOP_UNLOCK in
> same vnode related function. (2) Vnode operations in FreeBSD
> sometimes call vn_lock after VOP_UNLOCK(). VOP_UNLOCK() keep upper
> vnode locked and clear UN_ULOCK flag when UN_KLOCK is set in
> union_unlock. The vn_lock after VOP_UNLOCK calls union_lock, and
> uppervp is locked because UN_ULOCK is not set.
>
> My current idea is that:
>
> (1) union_unlock: do not clear uppervp, clear UN_ULOCK flag (same as
> current), and do not clear UN_KLOCK, when UN_KLOCK is set.
> (2) union_lock: do not access upper vp, do not clear UN_KLOCK, do not
> panic, when UN_KLOCK is set.
> (3) Each function in which UN_KLOCK is set should explicitly clear
> UN_KLOCK after calling vnode related functions.
I don't think I understand why the code wants to lock uppervp whenever the
union node is locked. Why not leave both uppervp and lowervp unlocked,
make the lock on the union node local (i.e. just lock the union node and
leave upper/lowervp alone). Then when a VOP in the underlying filesystems
needs to be called, take the underlying lock at the time. For example,
union_readlink would become:
int
union_readlink(ap)
struct vop_readlink_args /* {
struct vnode *a_vp;
struct uio *a_uio;
struct ucred *a_cred;
} */ *ap;
{
int error;
struct uio *uio = ap->a_uio;
struct proc *p = uio->uio_procp;
struct vnode *vp = OTHERVP(ap->a_vp);
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
ap->a_vp = vp;
error = VCALL(vp, VOFFSET(vop_readlink), ap);
VOP_UNLOCK(vp, 0, p);
return (error);
}
--
Doug Rabson Mail: dfr@nlsystems.com
Nonlinear Systems Ltd. Phone: +44 181 951 1891
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.3.95q.970427160802.346H-100000>