On 8/8/12, Harald Schmalzbauer <h.schmalzba...@omnilan.de> wrote:
>  schrieb Pavel Polyakov am 06.03.2012 11:20 (localtime):
>>>> mount -t unionfs -o noatime /usr /mnt
>>>>
>>>> insmntque: mp-safe fs and non-locked vp: 0xfffffe01d96704f0 is not
>>>> exclusive locked but should be
>>>> KDB: enter: lock violation
>>>
>>> Pavel,
>>> can you give a spin to this patch?:
>>> http://www.freebsd.org/~attilio/unionfs_missing_insmntque_lock.patch
>>>
>>> I think that the unlocking is due at that point as the vnode lock can
>>> be switch later on.
>>>
>>> Let me know what you think about it and what the test does.
>>
>> Thanks!
>> This patch fixes the problem with lock violation. Sorry I've tested it so
>> late.
>
> Hello,
>
> this patch still applies cleanly to RELENG_9_1. Was there another fix
> for the issue or has it just not been PR-sent and thus forgotten?

Can you and Pavel try the attached patch? Unfortunately I had no time
to test it, I just made in 5 free mins from a non-FreeBSD workstation,
then you should be able to tell me if it works or not, even compiling
it on a RELENG_9_1.
Please try with INVARIANTS option on.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
Index: sys/fs/unionfs/union_subr.c
===================================================================
--- sys/fs/unionfs/union_subr.c	(revision 239152)
+++ sys/fs/unionfs/union_subr.c	(working copy)
@@ -237,7 +237,8 @@ unionfs_nodeget(struct mount *mp, struct vnode *up
 		if (vp != NULLVP) {
 			vref(vp);
 			*vpp = vp;
-			goto unionfs_nodeget_out;
+			lockmgr(vp->v_vnlock, LK_EXCLUSIVE, NULL);
+			return (0);
 		}
 	}
 
@@ -255,17 +256,19 @@ unionfs_nodeget(struct mount *mp, struct vnode *up
 	 */
 	unp = malloc(sizeof(struct unionfs_node),
 	    M_UNIONFSNODE, M_WAITOK | M_ZERO);
+	if (path != NULL) {
+		unp->un_path = (char *)
+		    malloc(cnp->cn_namelen +1, M_UNIONFSPATH, M_WAITOK|M_ZERO);
+		bcopy(cnp->cn_nameptr, unp->un_path, cnp->cn_namelen);
+		unp->un_path[cnp->cn_namelen] = '\0';
+	}
 
 	error = getnewvnode("unionfs", mp, &unionfs_vnodeops, &vp);
 	if (error != 0) {
+		free(unp->un_path, M_UNIONFSNODE);
 		free(unp, M_UNIONFSNODE);
 		return (error);
 	}
-	error = insmntque(vp, mp);	/* XXX: Too early for mpsafe fs */
-	if (error != 0) {
-		free(unp, M_UNIONFSNODE);
-		return (error);
-	}
 	if (dvp != NULLVP)
 		vref(dvp);
 	if (uppervp != NULLVP)
@@ -286,15 +289,22 @@ unionfs_nodeget(struct mount *mp, struct vnode *up
 	else
 		vp->v_vnlock = lowervp->v_vnlock;
 
-	if (path != NULL) {
-		unp->un_path = (char *)
-		    malloc(cnp->cn_namelen +1, M_UNIONFSPATH, M_WAITOK|M_ZERO);
-		bcopy(cnp->cn_nameptr, unp->un_path, cnp->cn_namelen);
-		unp->un_path[cnp->cn_namelen] = '\0';
-	}
 	vp->v_type = vt;
 	vp->v_data = unp;
 
+	lockmgr(vp->v_vnlock, LK_EXCLUSIVE, NULL);
+	error = insmntque(vp, mp);
+	if (error != 0) {
+		if (dvp != NULLVP)
+			vrele(dvp);
+		if (uppervp != NULLVP)
+			vrele(uppervp);
+		if (lowervp != NULLVP)
+			vrele(lowervp);
+		free(unp->un_path, M_UNIONFSNODE);
+		free(unp, M_UNIONFSNODE);
+		return (error);
+	}
 	if ((uppervp != NULLVP && ump->um_uppervp == uppervp) &&
 	    (lowervp != NULLVP && ump->um_lowervp == lowervp))
 		vp->v_vflag |= VV_ROOT;
@@ -317,11 +327,6 @@ unionfs_nodeget(struct mount *mp, struct vnode *up
 		vref(vp);
 	} else
 		*vpp = vp;
-
-unionfs_nodeget_out:
-	if (lkflags & LK_TYPE_MASK)
-		vn_lock(vp, lkflags | LK_RETRY);
-
 	return (0);
 }
 
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Reply via email to