From owner-cvs-all  Wed Sep 25  3:57:49 2002
Delivered-To: cvs-all@freebsd.org
Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP
	id 28F5637B401; Wed, 25 Sep 2002 03:57:48 -0700 (PDT)
Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id A02DF43E77; Wed, 25 Sep 2002 03:57:47 -0700 (PDT)
	(envelope-from dl-freebsd@catspoiler.org)
Received: from mousie.catspoiler.org (mousie.catspoiler.org [192.168.101.2])
	by gw.catspoiler.org (8.12.5/8.12.5) with ESMTP id g8PAvamY016744;
	Wed, 25 Sep 2002 03:57:40 -0700 (PDT)
	(envelope-from dl-freebsd@catspoiler.org)
Message-Id: <200209251057.g8PAvamY016744@gw.catspoiler.org>
Date: Wed, 25 Sep 2002 03:57:36 -0700 (PDT)
From: Don Lewis <dl-freebsd@catspoiler.org>
Subject: Re: cvs commit: src/sys/sys lockmgr.h
To: jroberson@chesapeake.net
Cc: rwatson@FreeBSD.org, cvs-committers@FreeBSD.org,
	cvs-all@FreeBSD.org
In-Reply-To: <20020925003055.L97589-100000@mail.chesapeake.net>
MIME-Version: 1.0
Content-Type: TEXT/plain; charset=US-ASCII
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
List-ID: <cvs-all.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20cvs-all>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20cvs-all>
X-Loop: FreeBSD.ORG

On 25 Sep, Jeff Roberson wrote:

> There are some mp_fixmes, and some general cleanups that should happen.
> I'd like to be able to run with DEBUG_VFS_LOCKS enabled all the time.
> Right now there are still places that break on that.  After that, namei()
> and rename() are broken and unsafe wrt locking.  Right now all vfs locks
> are recursive.  I may take advantage of that to simplify the rename code.

kern_rename() and other callers of VOP_RENAME() should probably lock fvp
before calling VOP_RENAME() instead of leaving this up to the leaf
filesystems. ext2_rename() and ufs_rename() are already locking fvp, and
nfs_rename() needs to before it calls VOP_FSYNC(), so it would make more
sense to move this common code to the caller.

The current implementation doesn't properly handle the failure of
vn_lock(fvp, ...).  This also looks like it would be easier to handle if
the locking was done by kern_rename().  I believe that if the attempt to
lock fvp fails we should do something like:

                NDFREE(&tond, NDF_ONLY_PNBUF);
                if (tdvp == tvp)
                        vrele(tdvp);
                else
                        vput(tdvp);
                if (tvp)
                        vput(tvp);

and then to retry starting with:

        NDINIT(&tond, RENAME, LOCKPARENT | LOCKLEAF | NOCACHE | SAVESTART |
            NOOBJ, pathseg, to, td);
        if (fromnd.ni_vp->v_type == VDIR)
                tond.ni_cnd.cn_flags |= WILLBEDIR;
        if ((error = namei(&tond)) != 0) {

Maybe the NDFREE() and NDINIT() stuff could be skipped on the retry ...

I haven't looked at the other places in the source where VOP_RENAME() is
called to see if it would make sense to encapsulate some of this stuff
in a wrapper.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message