Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 12 Mar 2021 11:32:30 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 16dea8341024 - main - null_vput_pair(): release use reference on dvp earlier
Message-ID:  <202103121132.12CBWUXL077553@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=16dea8341024b8ee8be619c27d4e63bd81bd9b6c

commit 16dea8341024b8ee8be619c27d4e63bd81bd9b6c
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-03-07 21:08:38 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-03-12 11:31:08 +0000

    null_vput_pair(): release use reference on dvp earlier
    
    We might own the last use reference, and then vrele() at the end would
    need to take the dvp vnode lock to inactivate, which causes deadlock
    with vp. We cannot vrele() dvp from start since this might unlock ldvp.
    
    Handle it by holding the vnode and dropping use ref after lowerfs
    VOP_VPUT_PAIR() ended.  This effectivaly requires unlock of the vp vnode
    after VOP_VPUT_PAIR(), so the call is changed to set unlock_vp to true
    unconditionally.  This opens more opportunities for vp to be reclaimed,
    if lvp is still alive we reinstantiate vp with null_nodeget().
    
    Reported and tested by: pho
    Reviewed by:    mckusick
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 weeks
    Differential revision:  https://reviews.freebsd.org/D29178
---
 sys/fs/nullfs/null_vnops.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/sys/fs/nullfs/null_vnops.c b/sys/fs/nullfs/null_vnops.c
index 45065e0be7b5..bc0f5cdb7801 100644
--- a/sys/fs/nullfs/null_vnops.c
+++ b/sys/fs/nullfs/null_vnops.c
@@ -985,33 +985,50 @@ null_vput_pair(struct vop_vput_pair_args *ap)
 	vpp = ap->a_vpp;
 	vp = NULL;
 	lvp = NULL;
-	if (vpp != NULL) {
+	mp = NULL;
+	if (vpp != NULL)
 		vp = *vpp;
-		if (vp != NULL) {
+	if (vp != NULL) {
+		lvp = NULLVPTOLOWERVP(vp);
+		vref(lvp);
+		if (!ap->a_unlock_vp) {
 			vhold(vp);
+			vhold(lvp);
 			mp = vp->v_mount;
-			lvp = NULLVPTOLOWERVP(vp);
-			if (ap->a_unlock_vp)
-				vref(lvp);
+			vfs_ref(mp);
 		}
 	}
 
-	res = VOP_VPUT_PAIR(ldvp, &lvp, ap->a_unlock_vp);
+	res = VOP_VPUT_PAIR(ldvp, lvp != NULL ? &lvp : NULL, true);
+	if (vp != NULL && ap->a_unlock_vp)
+		vrele(vp);
+	vrele(dvp);
+
+	if (vp == NULL || ap->a_unlock_vp)
+		return (res);
 
-	/* lvp might have been unlocked and vp reclaimed */
-	if (vp != NULL) {
-		if (!ap->a_unlock_vp && vp->v_vnlock != lvp->v_vnlock) {
+	/* lvp has been unlocked and vp might be reclaimed */
+	VOP_LOCK(vp, LK_EXCLUSIVE | LK_RETRY);
+	if (vp->v_data == NULL && vfs_busy(mp, MBF_NOWAIT) == 0) {
+		vput(vp);
+		vget(lvp, LK_EXCLUSIVE | LK_RETRY);
+		if (VN_IS_DOOMED(lvp)) {
+			vput(lvp);
+			vget(vp, LK_EXCLUSIVE | LK_RETRY);
+		} else {
 			error = null_nodeget(mp, lvp, &vp1);
 			if (error == 0) {
-				vput(vp);
 				*vpp = vp1;
+			} else {
+				vget(vp, LK_EXCLUSIVE | LK_RETRY);
 			}
 		}
-		if (ap->a_unlock_vp)
-			vrele(vp);
-		vdrop(vp);
+		vfs_unbusy(mp);
 	}
-	vrele(dvp);
+	vdrop(lvp);
+	vdrop(vp);
+	vfs_rel(mp);
+
 	return (res);
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202103121132.12CBWUXL077553>