Date: Tue, 10 Sep 2002 18:26:42 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Don Lewis <dl-freebsd@catspoiler.org> Cc: rwatson@FreeBSD.ORG, <current@FreeBSD.ORG>, <jeff@FreeBSD.ORG> Subject: Re: vnode lock assertion problem in nfs_link() Message-ID: <20020910182523.D367-100000@gamplex.bde.org> In-Reply-To: <200209100419.g8A4JTwr093971@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> The locking changes in union_link() need a thorough review,
> though the light testing of that I performed didn't turn up any
> glaring problems.
The changes are obviously just cleanups for leaf file systems, but I
wonder why everything wasn't always locked at the top. Could it have
been because locking all the way down is harmful?
> Index: ufs/ufs/ufs_vnops.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v
> retrieving revision 1.204
> diff -u -r1.204 ufs_vnops.c
> --- ufs/ufs/ufs_vnops.c 15 Aug 2002 20:55:08 -0000 1.204
> +++ ufs/ufs/ufs_vnops.c 10 Sep 2002 03:08:48 -0000
> ...
> @@ -825,19 +824,16 @@
> #endif
> if (tdvp->v_mount != vp->v_mount) {
> error = EXDEV;
> - goto out2;
> - }
> - if (tdvp != vp && (error = vn_lock(vp, LK_EXCLUSIVE, td))) {
> - goto out2;
> + goto out;
> }
> ip = VTOI(vp);
> if ((nlink_t)ip->i_nlink >= LINK_MAX) {
> error = EMLINK;
> - goto out1;
> + goto out;
> }
> if (ip->i_flags & (IMMUTABLE | APPEND)) {
> error = EPERM;
> - goto out1;
> + goto out;
> }
> ip->i_effnlink++;
> ip->i_nlink++;
> @@ -859,10 +855,7 @@
> if (DOINGSOFTDEP(vp))
> softdep_change_linkcnt(ip);
> }
> -out1:
> - if (tdvp != vp)
> - VOP_UNLOCK(vp, 0, td);
> -out2:
> +out:
> VN_KNOTE(vp, NOTE_LINK);
> VN_KNOTE(tdvp, NOTE_WRITE);
> return (error);
The ugly gotos and uglier braces near them could be replaced by simple
returns now. Some related points:
(1) I think it is just a bug that null changes for failed operations are
knoted. It might be useful for knote to report attempted operations
and why they failed, but reporting failed operations doesn't seem
very useful unless they can be distinguished from successful ones.
Anyway, knote isn't told about failures before here. Doing the
knoting at a higher level upon successful completion would work
better in many cases including the above. This would be similar
to setting file times only upon successful completion, except it is
easier because knotes apply to vnodes but file times are fs-specific.
In both cases, it might be useful to set flags at lower levels in
some cases and back out the settings together with backing out the
main effects of the operation if the operation becomes unsuccessful
later.
(2) The gotos seem to exist to join common code that does a vput(tdvp) as
well as the things removed in the above. The vput() was removed some
time ago. There also used to be VOP_ABORTOP()s, but they weren't in
the common code for some reason.
(3) Removing the VOP_ABORTOP() after vn_lock() failure left ugly braces.
Similarly in ext2fs, except its support for kevent is missing in most
places including here.
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?20020910182523.D367-100000>