git: 64b494a1050a - main - softdep_prelink(): only do sync if other thread changed the vnode metadata since previous prelink

Konstantin Belousov kib at FreeBSD.org
Wed Jun 23 20:47:36 UTC 2021


The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=64b494a1050ae2cf2412edc19b57dc80f49eeda1

commit 64b494a1050ae2cf2412edc19b57dc80f49eeda1
Author:     Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2021-05-01 21:53:21 +0000
Commit:     Konstantin Belousov <kib at FreeBSD.org>
CommitDate: 2021-06-23 20:46:54 +0000

    softdep_prelink(): only do sync if other thread changed the vnode metadata since previous prelink
    
    We call into softdep_prerename() and softdep_prelink() when there is
    low free space in the journal. Functions sync all vnodes participating
    in the VOP, in the hope that this would reduce journal utilization. But
    if the vnodes are already synced, doing sync would only spend writes,
    journal is filled not due to the records from modifications of our
    vnodes.
    
    Remember original seqc numbers for vnodes, and only initiate syncs when
    seqc changed.
    
    Reviewed by:    mckusick
    Discussed with: markj
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 weeks
    Differential revision:  https://reviews.freebsd.org/D30041
---
 sys/sys/namei.h           | 12 +++++++++++-
 sys/ufs/ffs/ffs_extern.h  |  3 ++-
 sys/ufs/ffs/ffs_softdep.c | 32 +++++++++++++++++++++++++++-----
 sys/ufs/ufs/ufs_vnops.c   | 12 ++++++------
 4 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/sys/sys/namei.h b/sys/sys/namei.h
index b4db0e758e2b..9e0a82ea1659 100644
--- a/sys/sys/namei.h
+++ b/sys/sys/namei.h
@@ -38,6 +38,7 @@
 #include <sys/caprights.h>
 #include <sys/filedesc.h>
 #include <sys/queue.h>
+#include <sys/_seqc.h>
 #include <sys/_uio.h>
 
 enum nameiop { LOOKUP, CREATE, DELETE, RENAME };
@@ -111,6 +112,12 @@ struct nameidata {
 	 */
 	struct componentname ni_cnd;
 	struct nameicap_tracker_head ni_cap_tracker;
+	/*
+	 * Private helper data for UFS, must be at the end.  See
+	 * NDINIT_PREFILL().
+	 */
+	seqc_t	ni_dvp_seqc;
+	seqc_t	ni_vp_seqc;
 };
 
 #ifdef _KERNEL
@@ -224,7 +231,8 @@ int	cache_fplookup(struct nameidata *ndp, enum cache_fpl_status *status,
  * Note the constant pattern may *hide* bugs.
  */
 #ifdef INVARIANTS
-#define NDINIT_PREFILL(arg)	memset(arg, 0xff, sizeof(*arg))
+#define NDINIT_PREFILL(arg)	memset(arg, 0xff, offsetof(struct nameidata,	\
+    ni_dvp_seqc))
 #define NDINIT_DBG(arg)		{ (arg)->ni_debugflags = NAMEI_DBG_INITED; }
 #define NDREINIT_DBG(arg)	{						\
 	if (((arg)->ni_debugflags & NAMEI_DBG_INITED) == 0)			\
@@ -266,6 +274,8 @@ do {										\
 } while (0)
 
 #define	NDPREINIT(ndp) do {							\
+	(ndp)->ni_dvp_seqc = SEQC_MOD;						\
+	(ndp)->ni_vp_seqc = SEQC_MOD;						\
 } while (0)
 
 #define NDF_NO_DVP_RELE		0x00000001
diff --git a/sys/ufs/ffs/ffs_extern.h b/sys/ufs/ffs/ffs_extern.h
index 7080edd21ab3..2ea828861b42 100644
--- a/sys/ufs/ffs/ffs_extern.h
+++ b/sys/ufs/ffs/ffs_extern.h
@@ -181,7 +181,8 @@ 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	softdep_prelink(struct vnode *, struct vnode *,
+	    struct componentname *);
 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 2a76ec41f03d..d274e0898dfb 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -621,9 +621,10 @@ softdep_prerename(fdvp, fvp, tdvp, tvp)
 }
 
 int
-softdep_prelink(dvp, vp)
+softdep_prelink(dvp, vp, cnp)
 	struct vnode *dvp;
 	struct vnode *vp;
+	struct componentname *cnp;
 {
 
 	panic("softdep_prelink called");
@@ -3384,11 +3385,13 @@ softdep_prerename(fdvp, fvp, tdvp, tvp)
  * syscall must be restarted at top level from the lookup.
  */
 int
-softdep_prelink(dvp, vp)
+softdep_prelink(dvp, vp, cnp)
 	struct vnode *dvp;
 	struct vnode *vp;
+	struct componentname *cnp;
 {
 	struct ufsmount *ump;
+	struct nameidata *ndp;
 
 	ASSERT_VOP_ELOCKED(dvp, "prelink dvp");
 	if (vp != NULL)
@@ -3404,13 +3407,28 @@ softdep_prelink(dvp, vp)
 	if (journal_space(ump, 0) || (vp != NULL && IS_SNAPSHOT(VTOI(vp))))
 		return (0);
 
+	/*
+	 * Check if the journal space consumption can in theory be
+	 * accounted on dvp and vp.  If the vnodes metadata was not
+	 * changed comparing with the previous round-trip into
+	 * softdep_prelink(), as indicated by the seqc generation
+	 * recorded in the nameidata, then there is no point in
+	 * starting the sync.
+	 */
+	ndp = __containerof(cnp, struct nameidata, ni_cnd);
+	if (!seqc_in_modify(ndp->ni_dvp_seqc) &&
+	    vn_seqc_consistent(dvp, ndp->ni_dvp_seqc) &&
+	    (vp == NULL || (!seqc_in_modify(ndp->ni_vp_seqc) &&
+	    vn_seqc_consistent(vp, ndp->ni_vp_seqc))))
+		return (0);
+
 	stat_journal_low++;
 	if (vp != NULL) {
 		VOP_UNLOCK(dvp);
 		ffs_syncvnode(vp, MNT_NOWAIT, 0);
 		vn_lock_pair(dvp, false, vp, true);
 		if (dvp->v_data == NULL)
-			return (ERELOOKUP);
+			goto out;
 	}
 	if (vp != NULL)
 		VOP_UNLOCK(vp);
@@ -3421,7 +3439,7 @@ softdep_prelink(dvp, vp)
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 		if (vp->v_data == NULL) {
 			vn_lock_pair(dvp, false, vp, true);
-			return (ERELOOKUP);
+			goto out;
 		}
 		ACQUIRE_LOCK(ump);
 		process_removes(vp);
@@ -3431,7 +3449,7 @@ softdep_prelink(dvp, vp)
 		vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY);
 		if (dvp->v_data == NULL) {
 			vn_lock_pair(dvp, true, vp, false);
-			return (ERELOOKUP);
+			goto out;
 		}
 	}
 
@@ -3450,6 +3468,10 @@ softdep_prelink(dvp, vp)
 	FREE_LOCK(ump);
 
 	vn_lock_pair(dvp, false, vp, false);
+out:
+	ndp->ni_dvp_seqc = vn_seqc_read_any(dvp);
+	if (vp != NULL)
+		ndp->ni_vp_seqc = vn_seqc_read_any(vp);
 	return (ERELOOKUP);
 }
 
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index b0fb1b74b900..2dfc2e24f772 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1010,7 +1010,7 @@ ufs_remove(ap)
 	    (VTOI(dvp)->i_flags & APPEND))
 		return (EPERM);
 	if (DOINGSUJ(dvp)) {
-		error = softdep_prelink(dvp, vp);
+		error = softdep_prelink(dvp, vp, ap->a_cnp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -1075,7 +1075,7 @@ ufs_link(ap)
 #endif
 
 	if (DOINGSUJ(tdvp)) {
-		error = softdep_prelink(tdvp, vp);
+		error = softdep_prelink(tdvp, vp, cnp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -1147,7 +1147,7 @@ ufs_whiteout(ap)
 
 	if (DOINGSUJ(dvp) && (ap->a_flags == CREATE ||
 	    ap->a_flags == DELETE)) {
-		error = softdep_prelink(dvp, NULL);
+		error = softdep_prelink(dvp, NULL, cnp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -1962,7 +1962,7 @@ ufs_mkdir(ap)
 	}
 
 	if (DOINGSUJ(dvp)) {
-		error = softdep_prelink(dvp, NULL);
+		error = softdep_prelink(dvp, NULL, cnp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -2226,7 +2226,7 @@ ufs_rmdir(ap)
 		goto out;
 	}
 	if (DOINGSUJ(dvp)) {
-		error = softdep_prelink(dvp, vp);
+		error = softdep_prelink(dvp, vp, cnp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);
@@ -2751,7 +2751,7 @@ ufs_makeinode(mode, dvp, vpp, cnp, callfunc)
 		return (EINVAL);
 	}
 	if (DOINGSUJ(dvp)) {
-		error = softdep_prelink(dvp, NULL);
+		error = softdep_prelink(dvp, NULL, cnp);
 		if (error != 0) {
 			MPASS(error == ERELOOKUP);
 			return (error);


More information about the dev-commits-src-all mailing list