[src] cvs commit: src/sys/fs/tmpfs tmpfs.h tmpfs_subr.c tmpfs_vfsops.c tmpfs_vnops.c

[Kostik Belousov]

On Fri, Aug 10, 2007 at 05:24:54AM +0000, Xin LI wrote:
> delphij     2007-08-10 05:24:49 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/fs/tmpfs         tmpfs.h tmpfs_subr.c tmpfs_vfsops.c 
>                          tmpfs_vnops.c 
>   Log:
>   MFp4:
>   
>    - Respect cnflag and don't lock vnode always as LK_EXCLUSIVE [1]
>    - Properly lock around tn_vnode to avoid NULL deference
>    - Be more careful handling vnodes (*)
>   
>   (*) This is a WIP
>   [1] by pjd via howardsu
>   
>   Thanks kib@ for his valuable VFS related comments.
>   
>   Tested with:    fsx, fstest, tmpfs regression test set
>   Found by:       pho's stress2 suite
>   Approved by:    re (tmpfs blanket)
>   
>   Revision  Changes    Path
>   1.10      +2 -2      src/sys/fs/tmpfs/tmpfs.h
>   1.11      +28 -19    src/sys/fs/tmpfs/tmpfs_subr.c
>   1.9       +5 -2      src/sys/fs/tmpfs/tmpfs_vfsops.c
>   1.9       +34 -13    src/sys/fs/tmpfs/tmpfs_vnops.c

> Index: src/sys/fs/tmpfs/tmpfs_subr.c
> diff -u src/sys/fs/tmpfs/tmpfs_subr.c:1.10 src/sys/fs/tmpfs/tmpfs_subr.c:1.11
> --- src/sys/fs/tmpfs/tmpfs_subr.c:1.10	Fri Aug  3 06:24:31 2007
> +++ src/sys/fs/tmpfs/tmpfs_subr.c	Fri Aug 10 05:24:49 2007
> @@ -302,15 +308,20 @@
>   * Returns zero on success or an appropriate error code on failure.
>   */
>  int
> -tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, struct vnode **vpp,
> -    struct thread *td)
> +tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, int lkflag,
> +    struct vnode **vpp, struct thread *td)
>  {
>  	int error;
>  	struct vnode *vp;
>  
>  loop:
> +	TMPFS_NODE_LOCK(node);
>  	if ((vp = node->tn_vnode) != NULL) {
> -		error = vget(vp, LK_EXCLUSIVE | LK_RETRY, td);
> +		VI_LOCK(vp);
> +		TMPFS_NODE_UNLOCK(node);
> +		vholdl(vp);
> +		error = vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, td);
> +		vdrop(vp);
>  		if (error)
>  			return error;
LK_RETRY prohibits the vget() and vn_lock() to return error.

> Index: src/sys/fs/tmpfs/tmpfs_vnops.c
> diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.8 src/sys/fs/tmpfs/tmpfs_vnops.c:1.9
> --- src/sys/fs/tmpfs/tmpfs_vnops.c:1.8	Thu Jul 19 03:34:50 2007
> +++ src/sys/fs/tmpfs/tmpfs_vnops.c	Fri Aug 10 05:24:49 2007
> @@ -95,12 +95,20 @@
>  	    !(cnp->cn_flags & ISDOTDOT)));
>  
>  	if (cnp->cn_flags & ISDOTDOT) {
> -		VOP_UNLOCK(dvp, 0, td);
> +		int ltype = 0;
>  
> +		ltype = VOP_ISLOCKED(dvp, td);
> +		vholdl(dvp);
vholdl() assumes that vnode interlock is locked, not vnode itself. You shall
use vhold() when not holding vnode interlock.

Later, you use dvp->v_mount, that could change to NULL once you dropped the
dvp lock. Moreover, filesystem may be unmounted after you dropped vnode
lock. UFS has the same problem.

> +		VOP_UNLOCK(dvp, 0, td);
>  		/* Allocate a new vnode on the matching entry. */
> -		error = tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent, vpp, td);
> +		error = tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent,
> +		    cnp->cn_lkflags, vpp, td);
>  
> -		vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
> +		vn_lock(dvp, ltype | LK_RETRY, td);
> +		if (ltype & LK_INTERLOCK)
> +			vdropl(dvp);
> +		else
> +			vdrop(dvp);
ltype cannot contain LK_INTERLOCK flag. Moreover, vn_lock() always returns
with interlock being dropped. You shall simply vdrop(dvp) there.

>  
>  		dnode->tn_dir.tn_parent->tn_lookup_dirent = NULL;
>  	} else if (cnp->cn_namelen == 1 && cnp->cn_nameptr[0] == '.') {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20070810/a4e9117d/attachment.pgp