Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 12 Feb 2021 01:07:21 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: ede40b067515 - main - ffs softdep: remove will_direnter argument of softdep_prelink()
Message-ID:  <202102120107.11C17L12070761@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=ede40b0675155b5cc862652f2fee11c738a46bcd

commit ede40b0675155b5cc862652f2fee11c738a46bcd
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-01-23 22:40:19 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-02-12 01:02:21 +0000

    ffs softdep: remove will_direnter argument of softdep_prelink()
    
    Originally this was done in 8a1509e442bc9a075 to forcibly cover cases
    where a hole in the directory could be created by extending into
    indirect block, since dependency of writing out indirect block is not
    tracked.  This results in excessive amount of fsyncing the directories,
    where all creation of new entry forced fsync before it.  This is not needed,
    it is enough to fsync when IN_NEEDSYNC is set, and VOP_VPUT_PAIR() provides
    the required hook to only perform required syncing.
    
    The series of changes culminating in this commit puts the performance of
    metadata-intensive loads back to that before 8a1509e442bc9a075.
    
    Analyzed by:    mckusick
    Reviewed by:    chs, mckusick
    Tested by:      pho
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
---
 sys/ufs/ffs/ffs_extern.h  |  2 +-
 sys/ufs/ffs/ffs_softdep.c | 46 ++++++++--------------------------------------
 sys/ufs/ufs/ufs_vnops.c   | 12 ++++++------
 3 files changed, 15 insertions(+), 45 deletions(-)

diff --git a/sys/ufs/ffs/ffs_extern.h b/sys/ufs/ffs/ffs_extern.h
index 9694489266b6..544012089046 100644
--- a/sys/ufs/ffs/ffs_extern.h
+++ b/sys/ufs/ffs/ffs_extern.h
@@ -178,7 +178,7 @@ int	softdep_request_cleanup(struct fs *, struct vnode *,
 	    struct ucred *, int);
 int	softdep_prerename(struct vnode *, struct vnode *, struct vnode *,
 	    struct vnode *);
-int	softdep_prelink(struct vnode *, struct vnode *, int);
+int	softdep_prelink(struct vnode *, struct vnode *);
 void	softdep_setup_freeblocks(struct inode *, off_t, int);
 void	softdep_setup_inomapdep(struct buf *, struct inode *, ino_t, int);
 void	softdep_setup_blkmapdep(struct buf *, struct mount *, ufs2_daddr_t,
diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index e90593b20e40..3cc76f9142c3 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -621,10 +621,9 @@ softdep_prerename(fdvp, fvp, tdvp, tvp)
 }
 
 int
-softdep_prelink(dvp, vp, will_direnter)
+softdep_prelink(dvp, vp)
 	struct vnode *dvp;
 	struct vnode *vp;
-	int will_direnter;
 {
 
 	panic("softdep_prelink called");
@@ -3358,13 +3357,11 @@ softdep_prerename(fdvp, fvp, tdvp, tvp)
  * syscall must be restarted at top level from the lookup.
  */
 int
-softdep_prelink(dvp, vp, will_direnter)
+softdep_prelink(dvp, vp)
 	struct vnode *dvp;
 	struct vnode *vp;
-	int will_direnter;
 {
 	struct ufsmount *ump;
-	int error, error1;
 
 	ASSERT_VOP_ELOCKED(dvp, "prelink dvp");
 	if (vp != NULL)
@@ -3372,40 +3369,13 @@ softdep_prelink(dvp, vp, will_direnter)
 	ump = VFSTOUFS(dvp->v_mount);
 
 	/*
-	 * Nothing to do if we have sufficient journal space.
-	 * If we currently hold the snapshot lock, we must avoid
-	 * handling other resources that could cause deadlock.
-	 *
-	 * will_direnter == 1: In case allocated a directory block in
-	 * an indirect block, we must prevent holes in the directory
-	 * created if directory entries are written out of order.  To
-	 * accomplish this we fsync when we extend a directory into
-	 * indirects.  During rename it's not safe to drop the tvp
-	 * lock so sync must be delayed until it is.
-	 *
-	 * This synchronous step could be removed if fsck and the
-	 * kernel were taught to fill in sparse directories rather
-	 * than panic.
+	 * Nothing to do if we have sufficient journal space.  We skip
+	 * flushing when vp is a snapshot to avoid deadlock where
+	 * another thread is trying to update the inodeblock for dvp
+	 * and is waiting on snaplk that vp holds.
 	 */
-	if (journal_space(ump, 0) || (vp != NULL && IS_SNAPSHOT(VTOI(vp)))) {
-		error = 0;
-		if (will_direnter && (vp == NULL || !IS_SNAPSHOT(VTOI(vp)))) {
-			if (vp != NULL)
-				VOP_UNLOCK(vp);
-			error = ffs_syncvnode(dvp, MNT_WAIT, 0);
-			if (vp != NULL) {
-				error1 = vn_lock(vp, LK_EXCLUSIVE | LK_NOWAIT);
-				if (error1 != 0) {
-					vn_lock_pair(dvp, true, vp, false);
-					if (error == 0)
-						error = ERELOOKUP;
-				} else if (vp->v_data == NULL) {
-					error = ERELOOKUP;
-				}
-			}
-		}
-		return (error);
-	}
+	if (journal_space(ump, 0) || (vp != NULL && IS_SNAPSHOT(VTOI(vp))))
+		return (0);
 
 	stat_journal_low++;
 	if (vp != NULL) {
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index 22199a390dd4..b035a8b1c34d 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1007,7 +1007,7 @@ ufs_remove(ap)
 	    (VTOI(dvp)->i_flags & APPEND))
 		return (EPERM);
 	if (DOINGSOFTDEP(dvp)) {
-		error = softdep_prelink(dvp, vp, true);
+		error = softdep_prelink(dvp, vp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -1072,7 +1072,7 @@ ufs_link(ap)
 #endif
 
 	if (DOINGSOFTDEP(tdvp)) {
-		error = softdep_prelink(tdvp, vp, true);
+		error = softdep_prelink(tdvp, vp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -1144,7 +1144,7 @@ ufs_whiteout(ap)
 
 	if (DOINGSOFTDEP(dvp) && (ap->a_flags == CREATE ||
 	    ap->a_flags == DELETE)) {
-		error = softdep_prelink(dvp, NULL, true);
+		error = softdep_prelink(dvp, NULL);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -1946,7 +1946,7 @@ ufs_mkdir(ap)
 	}
 
 	if (DOINGSOFTDEP(dvp)) {
-		error = softdep_prelink(dvp, NULL, true);
+		error = softdep_prelink(dvp, NULL);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -2210,7 +2210,7 @@ ufs_rmdir(ap)
 		goto out;
 	}
 	if (DOINGSOFTDEP(dvp)) {
-		error = softdep_prelink(dvp, vp, false);
+		error = softdep_prelink(dvp, vp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -2737,7 +2737,7 @@ ufs_makeinode(mode, dvp, vpp, cnp, callfunc)
 		return (EINVAL);
 	}
 	if (DOINGSOFTDEP(dvp)) {
-		error = softdep_prelink(dvp, NULL, true);
+		error = softdep_prelink(dvp, NULL);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);



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