Re: kern/50375: layerfs (nullfs) locking problem leading to livelock

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

The following reply was made to PR kern/50375; it has been noted by GNATS.

From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: Jeff Rizzo <riz%NetBSD.org@localhost>
Subject: Re: kern/50375: layerfs (nullfs) locking problem leading to livelock
Date: Fri, 30 Oct 2015 17:29:10 +0100

 --Apple-Mail=_8B134FE5-D1A3-411B-9E18-F9AA4C1811B0
 Content-Transfer-Encoding: 7bit
 Content-Type: text/plain;
 	charset=us-ascii
 
 Please try the attached patch.  It will take the vnode lock before
 the vnode is marked VI_CHANGING and fed to vclean().
 
 vclean() should no longer block.
 
 --
 J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig (Germany)
 
 --Apple-Mail=_8B134FE5-D1A3-411B-9E18-F9AA4C1811B0
 Content-Disposition: attachment;
 	filename=vfs_vnode.diff
 Content-Type: application/octet-stream;
 	name="vfs_vnode.diff"
 Content-Transfer-Encoding: 7bit
 
 Index: vfs_vnode.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_vnode.c,v
 retrieving revision 1.45
 diff -p -u -2 -r1.45 vfs_vnode.c
 --- vfs_vnode.c	12 Jul 2015 08:11:28 -0000	1.45
 +++ vfs_vnode.c	30 Oct 2015 16:22:24 -0000
 @@ -326,13 +326,15 @@ try_nextlist:
  		KASSERT(vp->v_freelisthd == listhd);
  
 -		if (!mutex_tryenter(vp->v_interlock))
 +		if (vn_lock(vp, LK_EXCLUSIVE | LK_NOWAIT) != 0)
  			continue;
 -		if ((vp->v_iflag & VI_XLOCK) != 0) {
 -			mutex_exit(vp->v_interlock);
 +		if (!mutex_tryenter(vp->v_interlock)) {
 +			VOP_UNLOCK(vp);
  			continue;
  		}
 +		KASSERT((vp->v_iflag & VI_XLOCK) == 0);
  		mp = vp->v_mount;
  		if (fstrans_start_nowait(mp, FSTRANS_SHARED) != 0) {
  			mutex_exit(vp->v_interlock);
 +			VOP_UNLOCK(vp);
  			continue;
  		}
 @@ -644,4 +646,8 @@ vrelel(vnode_t *vp, int flags)
  		 */
  		VOP_INACTIVE(vp, &recycle);
 +		if (recycle) {
 +			/* vclean() below will drop the lock. */
 +			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 +		}
  		mutex_enter(vp->v_interlock);
  		if (!recycle) {
 @@ -868,4 +874,5 @@ holdrelel(vnode_t *vp)
   * Disassociate the underlying file system from a vnode.
   *
 + * Must be called with vnode locked and will return unlocked.
   * Must be called with the interlock held, and will return with it held.
   */
 @@ -877,4 +884,6 @@ vclean(vnode_t *vp)
  	int error;
  
 +	KASSERT((vp->v_vflag & VV_LOCKSWORK) == 0 ||
 +	    VOP_ISLOCKED(vp) == LK_EXCLUSIVE);
  	KASSERT(mutex_owned(vp->v_interlock));
  	KASSERT((vp->v_iflag & VI_MARKER) == 0);
 @@ -883,17 +892,9 @@ vclean(vnode_t *vp)
  	/* If already clean, nothing to do. */
  	if ((vp->v_iflag & VI_CLEAN) != 0) {
 +		VOP_UNLOCK(vp);
  		return;
  	}
  
  	active = (vp->v_usecount > 1);
 -	mutex_exit(vp->v_interlock);
 -
 -	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 -
 -	/*
 -	 * Prevent the vnode from being recycled or brought into use
 -	 * while we clean it out.
 -	 */
 -	mutex_enter(vp->v_interlock);
  	KASSERT((vp->v_iflag & (VI_XLOCK | VI_CLEAN)) == 0);
  	vp->v_iflag |= VI_XLOCK;
 @@ -973,4 +974,7 @@ vrecycle(vnode_t *vp)
  {
  
 +	if (vn_lock(vp, LK_EXCLUSIVE) != 0)
 +		return false;
 +
  	mutex_enter(vp->v_interlock);
  
 @@ -979,4 +983,5 @@ vrecycle(vnode_t *vp)
  	if (vp->v_usecount != 1) {
  		mutex_exit(vp->v_interlock);
 +		VOP_UNLOCK(vp);
  		return false;
  	}
 @@ -985,9 +990,8 @@ vrecycle(vnode_t *vp)
  	if (vp->v_usecount != 1) {
  		mutex_exit(vp->v_interlock);
 +		VOP_UNLOCK(vp);
  		return false;
 -	} else if ((vp->v_iflag & VI_CLEAN) != 0) {
 -		mutex_exit(vp->v_interlock);
 -		return true;
  	}
 +	KASSERT((vp->v_iflag & VI_CLEAN) == 0);
  	vp->v_iflag |= VI_CHANGING;
  	vclean(vp);
 @@ -1037,4 +1041,9 @@ vgone(vnode_t *vp)
  {
  
 +	if (vn_lock(vp, LK_EXCLUSIVE) != 0) {
 +		KASSERT((vp->v_iflag & VI_CLEAN) != 0);
 +		vrele(vp);
 +	}
 +
  	mutex_enter(vp->v_interlock);
  	if ((vp->v_iflag & VI_CHANGING) != 0)
 
 --Apple-Mail=_8B134FE5-D1A3-411B-9E18-F9AA4C1811B0--