Date: Wed, 25 Sep 2002 03:57:36 -0700 (PDT) From: Don Lewis <dl-freebsd@catspoiler.org> To: jroberson@chesapeake.net Cc: rwatson@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/sys lockmgr.h Message-ID: <200209251057.g8PAvamY016744@gw.catspoiler.org> In-Reply-To: <20020925003055.L97589-100000@mail.chesapeake.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200209251057.g8PAvamY016744>