kern/107439: 6.2-PRE repeatable panic: userret: Returning with 1 locks held

[Mikolaj Golub]

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

From: Mikolaj Golub <to.my.trociny at gmail.com>
To: bug-followup at FreeBSD.org, Eugene Grosbein <eugen at grosbein.pp.ru>
Cc:  
Subject: Re: kern/107439: 6.2-PRE repeatable panic: userret: Returning with 1 locks held
Date: Fri, 02 Apr 2010 09:07:37 +0300

 --=-=-=
 
 I have reproduced the problem on 8.0-STABLE following the Eugene's
 instructions.
 
 On the system compiled without WITNESS I have:
 
 mv: rename /mnt/ntfs/test to /mnt/ufs/test: Operation not supported
 
 and any access to /mnt/ufs/test is being locked after this.
 
 On the system with WITNESS when running mv I have a witness_warn panic:
 
 System call rename returning with the following locks held:
 exclusive lockmgr ufs (ufs) r = 0 (0xc76ab058) locked @ /usr/src/sys/kern/vfs_subr.c:2091
 panic: witness_warn
 cpuid = 2
 KDB: enter: panic
 exclusive lockmgr ufs (ufs) r = 0 (0xc76ab058) locked @ /usr/src/sys/kern/vfs_subr.c:2091
 exclusive lockmgr ufs (ufs) r = 0 (0xc76ab058) locked @ /usr/src/sys/kern/vfs_subr.c:2091
 
 0xc76ab000: tag ufs, type VDIR
     usecount 1, writecount 0, refcount 3 mountedhere 0
     flags (VV_ROOT)
     v_object 0xc77637f8 ref 0 pages 1
     lock type ufs: EXCL by thread 0xc76b14a0 (pid 2100)
 #0 0xc088feb2 at __lockmgr_args+0x592
 #1 0xc0af20b1 at ffs_lock+0xa1
 #2 0xc0c047b3 at VOP_LOCK1_APV+0xf3
 #3 0xc09488d8 at _vn_lock+0x78
 #4 0xc093b98b at vget+0xbb
 #5 0xc092e7cd at vfs_hash_get+0xed
 #6 0xc0aec9d9 at ffs_vgetf+0x49
 #7 0xc0aecf4e at ffs_vget+0x2e
 #8 0xc0afc6b8 at ufs_root+0x28
 #9 0xc092fd61 at lookup+0x9a1
 #10 0xc09308bf at namei+0x5bf
 #11 0xc0944433 at kern_renameat+0x1b3
 #12 0xc0944736 at kern_rename+0x36
 #13 0xc0944769 at rename+0x29
 #14 0xc0beab50 at syscall+0x270
 #15 0xc0bcce20 at Xint0x80_syscall+0x20
         ino 2, on dev md0
 
 So we have ufs lock leakage here. The VOP_RENAME(9) routine is expected to
 vput(9) both the destination directory and file prior to returning. But for
 ntfs vop_rename() is not implemented and as a result vop_bypass() is called
 instead in VOP_RENAME_APV() and the vnods remain locked.
 
 So we need to add the vnods unlocking somewhere. I have not thought of a
 better place then in vop_rename_post() (see the patch below). The better place
 would be VOP_RENAME_APV(), after calling vop_bypass(), but VOP_RENAME_APV() is
 generated automatically by tools/vnode_if.awk and it looks like this can't be
 done there.
 
 I have tested the patch and it works for me.
 
 Also note the problem with leaked lock also exists when trying to rename
 files inside ntfs folder -- operation failed and after this you can't unmount
 ntfs. The patch cures this case too.
 
 -- 
 Mikolaj Golub
 
 
 --=-=-=
 Content-Type: text/x-patch
 Content-Disposition: inline; filename=vfs_subr.c.vop_rename.patch
 
 --- sys/kern/vfs_subr.c.orig	2010-04-02 07:42:16.000000000 +0300
 +++ sys/kern/vfs_subr.c	2010-04-02 07:58:01.000000000 +0300
 @@ -3946,6 +3946,20 @@ vop_rename_post(void *ap, int rc)
  		if (a->a_tvp)
  			VFS_KNOTE_UNLOCKED(a->a_tvp, NOTE_DELETE);
  	}
 +	/* 
 +	 * The VOP routine is expected to vput(9) both the destination
 +	 * directory and file prior to returning. If rc is EOPNOTSUPP then
 +	 * vop_rename() is not implemented for this fs and vop_bypass() has
 +	 * been called instead, so we need to unlock the vnods here.
 +	 */
 +	if (rc == EOPNOTSUPP) {
 +		if (a->a_tdvp == a->a_tvp)
 +			vrele(a->a_tdvp);
 +		else
 +			vput(a->a_tdvp);
 +		vrele(a->a_fdvp);
 +		vrele(a->a_fvp);
 +	}
  	if (a->a_tdvp != a->a_fdvp)
  		vdrop(a->a_fdvp);
  	if (a->a_tvp != a->a_fvp)
 
 --=-=-=--