Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 22 Oct 2019 16:17:39 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r353891 - in head/sys/fs: nfs nfsclient
Message-ID:  <201910221617.x9MGHd32059393@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Tue Oct 22 16:17:38 2019
New Revision: 353891
URL: https://svnweb.freebsd.org/changeset/base/353891

Log:
  Fix interface between nfsclient and vnode pager.
  
  Make the nfsclient always call vnode_pager_setsize() with the vnode
  exclusively locked.  This ensures that page fault always can find the
  backing page if the object size check succeeded.  Set VV_VMSIZEVNLOCK
  flag on NFS nodes.
  
  The main offender breaking the interface in nfsclient is
  nfs_loadattrcache(), which is used whenever server responded with
  updated attributes, which can happen on non-changing operations as
  well.  Also, iod threads only have buffers locked (and even that is
  LK_KERNPROC), but they still may call nfs_loadattrcache() on RPC
  response.
  
  Instead of immediately calling vnode_pager_setsize() if server
  response indicated changed file size, but the vnode is not exclusively
  locked, set a new node flag NVNSETSZSKIP.  When the vnode exclusively
  locked, or when we can temporary upgrade the lock to exclusive, call
  vnode_pager_setsize(), by providing the nfsclient VOP_LOCK() implementation.
  
  Tested by:	pho
  Discussed with:	rmacklem
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks
  Differential revision:	https://reviews.freebsd.org/D21883

Modified:
  head/sys/fs/nfs/nfsport.h
  head/sys/fs/nfsclient/nfs_clnode.c
  head/sys/fs/nfsclient/nfs_clport.c
  head/sys/fs/nfsclient/nfs_clsubs.c
  head/sys/fs/nfsclient/nfs_clvnops.c
  head/sys/fs/nfsclient/nfsnode.h

Modified: head/sys/fs/nfs/nfsport.h
==============================================================================
--- head/sys/fs/nfs/nfsport.h	Tue Oct 22 16:09:25 2019	(r353890)
+++ head/sys/fs/nfs/nfsport.h	Tue Oct 22 16:17:38 2019	(r353891)
@@ -878,6 +878,7 @@ MALLOC_DECLARE(M_NEWNFSDSESSION);
 int nfscl_loadattrcache(struct vnode **, struct nfsvattr *, void *, void *,
     int, int);
 int newnfs_realign(struct mbuf **, int);
+bool ncl_pager_setsize(struct vnode *vp, u_quad_t *nsizep);
 
 /*
  * If the port runs on an SMP box that can enforce Atomic ops with low

Modified: head/sys/fs/nfsclient/nfs_clnode.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clnode.c	Tue Oct 22 16:09:25 2019	(r353890)
+++ head/sys/fs/nfsclient/nfs_clnode.c	Tue Oct 22 16:17:38 2019	(r353891)
@@ -162,6 +162,8 @@ ncl_nget(struct mount *mntp, u_int8_t *fhp, int fhsize
 			vp->v_type = VDIR;
 		vp->v_vflag |= VV_ROOT;
 	}
+
+	vp->v_vflag |= VV_VMSIZEVNLOCK;
 	
 	np->n_fhp = malloc(sizeof (struct nfsfh) + fhsize,
 	    M_NFSFH, M_WAITOK);

Modified: head/sys/fs/nfsclient/nfs_clport.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clport.c	Tue Oct 22 16:09:25 2019	(r353890)
+++ head/sys/fs/nfsclient/nfs_clport.c	Tue Oct 22 16:17:38 2019	(r353891)
@@ -246,6 +246,8 @@ nfscl_nget(struct mount *mntp, struct vnode *dvp, stru
 			vp->v_type = VDIR;
 		vp->v_vflag |= VV_ROOT;
 	}
+
+	vp->v_vflag |= VV_VMSIZEVNLOCK;
 	
 	np->n_fhp = nfhp;
 	/*
@@ -414,10 +416,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvatt
 	struct nfsnode *np;
 	struct nfsmount *nmp;
 	struct timespec mtime_save;
-	vm_object_t object;
-	u_quad_t nsize;
 	int error, force_fid_err;
-	bool setnsize;
 
 	error = 0;
 
@@ -565,27 +564,53 @@ out:
 	if (np->n_attrstamp != 0)
 		KDTRACE_NFS_ATTRCACHE_LOAD_DONE(vp, vap, error);
 #endif
+	(void)ncl_pager_setsize(vp, NULL);
+	return (error);
+}
+
+/*
+ * Call vnode_pager_setsize() if the size of the node changed, as
+ * recorded in nfsnode vs. v_object, or delay the call if notifying
+ * the pager is not possible at the moment.
+ *
+ * If nsizep is non-NULL, the call is delayed and the new node size is
+ * provided.  Caller should itself call vnode_pager_setsize() if
+ * function returned true.  If nsizep is NULL, function tries to call
+ * vnode_pager_setsize() itself if needed and possible, and the nfs
+ * node is unlocked unconditionally, the return value is not useful.
+ */
+bool
+ncl_pager_setsize(struct vnode *vp, u_quad_t *nsizep)
+{
+	struct nfsnode *np;
+	vm_object_t object;
+	struct vattr *vap;
+	u_quad_t nsize;
+	bool setnsize;
+
+	np = VTONFS(vp);
+	NFSASSERTNODE(np);
+
+	vap = &np->n_vattr.na_vattr;
 	nsize = vap->va_size;
 	object = vp->v_object;
 	setnsize = false;
-	if (object != NULL) {
-		if (OFF_TO_IDX(nsize + PAGE_MASK) < object->size) {
-			/*
-			 * When shrinking the size, the call to
-			 * vnode_pager_setsize() cannot be done with
-			 * the mutex held, because we might need to
-			 * wait for a busy page.  Delay it until after
-			 * the node is unlocked.
-			 */
+
+	if (object != NULL && nsize != object->un_pager.vnp.vnp_size) {
+		if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE)
 			setnsize = true;
-		} else {
+		else
+			np->n_flag |= NVNSETSZSKIP;
+	}
+	if (nsizep == NULL) {
+		NFSUNLOCKNODE(np);
+		if (setnsize)
 			vnode_pager_setsize(vp, nsize);
-		}
+		setnsize = false;
+	} else {
+		*nsizep = nsize;
 	}
-	NFSUNLOCKNODE(np);
-	if (setnsize)
-		vnode_pager_setsize(vp, nsize);
-	return (error);
+	return (setnsize);
 }
 
 /*

Modified: head/sys/fs/nfsclient/nfs_clsubs.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clsubs.c	Tue Oct 22 16:09:25 2019	(r353890)
+++ head/sys/fs/nfsclient/nfs_clsubs.c	Tue Oct 22 16:17:38 2019	(r353891)
@@ -185,6 +185,8 @@ ncl_getattrcache(struct vnode *vp, struct vattr *vaper
 	struct vattr *vap;
 	struct nfsmount *nmp;
 	int timeo, mustflush;
+	u_quad_t nsize;
+	bool setnsize;
 	
 	np = VTONFS(vp);
 	vap = &np->n_vattr.na_vattr;
@@ -230,6 +232,7 @@ ncl_getattrcache(struct vnode *vp, struct vattr *vaper
 		return( ENOENT);
 	}
 	nfsstatsv1.attrcache_hits++;
+	setnsize = false;
 	if (vap->va_size != np->n_size) {
 		if (vap->va_type == VREG) {
 			if (np->n_flag & NMODIFIED) {
@@ -240,7 +243,7 @@ ncl_getattrcache(struct vnode *vp, struct vattr *vaper
 			} else {
 				np->n_size = vap->va_size;
 			}
-			vnode_pager_setsize(vp, np->n_size);
+			setnsize = ncl_pager_setsize(vp, &nsize);
 		} else {
 			np->n_size = vap->va_size;
 		}
@@ -253,6 +256,8 @@ ncl_getattrcache(struct vnode *vp, struct vattr *vaper
 			vaper->va_mtime = np->n_mtim;
 	}
 	NFSUNLOCKNODE(np);
+	if (setnsize)
+		vnode_pager_setsize(vp, nsize);
 	KDTRACE_NFS_ATTRCACHE_GET_HIT(vp, vap);
 	return (0);
 }

Modified: head/sys/fs/nfsclient/nfs_clvnops.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clvnops.c	Tue Oct 22 16:09:25 2019	(r353890)
+++ head/sys/fs/nfsclient/nfs_clvnops.c	Tue Oct 22 16:17:38 2019	(r353891)
@@ -142,6 +142,7 @@ static vop_advlock_t	nfs_advlock;
 static vop_advlockasync_t nfs_advlockasync;
 static vop_getacl_t nfs_getacl;
 static vop_setacl_t nfs_setacl;
+static vop_lock1_t	nfs_lock;
 
 /*
  * Global vfs data structures for nfs
@@ -160,6 +161,7 @@ static struct vop_vector newnfs_vnodeops_nosig = {
 	.vop_putpages =		ncl_putpages,
 	.vop_inactive =		ncl_inactive,
 	.vop_link =		nfs_link,
+	.vop_lock1 =		nfs_lock,
 	.vop_lookup =		nfs_lookup,
 	.vop_mkdir =		nfs_mkdir,
 	.vop_mknod =		nfs_mknod,
@@ -294,6 +296,73 @@ SYSCTL_INT(_vfs_nfs, OID_AUTO, nfs_directio_allow_mmap
        rep->r_mtx
  * rep->r_mtx : Protects the fields in an nfsreq.
  */
+
+static int
+nfs_lock(struct vop_lock1_args *ap)
+{
+	struct vnode *vp;
+	struct nfsnode *np;
+	u_quad_t nsize;
+	int error, lktype;
+	bool onfault;
+
+	vp = ap->a_vp;
+	lktype = ap->a_flags & LK_TYPE_MASK;
+	error = VOP_LOCK1_APV(&default_vnodeops, ap);
+	if (error != 0 || vp->v_op != &newnfs_vnodeops)
+		return (error);
+	np = VTONFS(vp);
+	NFSLOCKNODE(np);
+	if ((np->n_flag & NVNSETSZSKIP) == 0 || (lktype != LK_SHARED &&
+	    lktype != LK_EXCLUSIVE && lktype != LK_UPGRADE &&
+	    lktype != LK_TRYUPGRADE)) {
+		NFSUNLOCKNODE(np);
+		return (0);
+	}
+	onfault = (ap->a_flags & LK_EATTR_MASK) == LK_NOWAIT &&
+	    (ap->a_flags & LK_INIT_MASK) == LK_CANRECURSE &&
+	    (lktype == LK_SHARED || lktype == LK_EXCLUSIVE);
+	if (onfault && vp->v_vnlock->lk_recurse == 0) {
+		/*
+		 * Force retry in vm_fault(), to make the lock request
+		 * sleepable, which allows us to piggy-back the
+		 * sleepable call to vnode_pager_setsize().
+		 */
+		NFSUNLOCKNODE(np);
+		VOP_UNLOCK(vp, 0);
+		return (EBUSY);
+	}
+	if ((ap->a_flags & LK_NOWAIT) != 0 ||
+	    (lktype == LK_SHARED && vp->v_vnlock->lk_recurse > 0)) {
+		NFSUNLOCKNODE(np);
+		return (0);
+	}
+	if (lktype == LK_SHARED) {
+		NFSUNLOCKNODE(np);
+		VOP_UNLOCK(vp, 0);
+		ap->a_flags &= ~(LK_TYPE_MASK | LK_INTERLOCK);
+		ap->a_flags |= LK_EXCLUSIVE;
+		error = VOP_LOCK1_APV(&default_vnodeops, ap);
+		if (error != 0 || vp->v_op != &newnfs_vnodeops)
+			return (error);
+		NFSLOCKNODE(np);
+		if ((np->n_flag & NVNSETSZSKIP) == 0) {
+			NFSUNLOCKNODE(np);
+			goto downgrade;
+		}
+	}
+	np->n_flag &= ~NVNSETSZSKIP;
+	nsize = np->n_size;
+	NFSUNLOCKNODE(np);
+	vnode_pager_setsize(vp, nsize);
+downgrade:
+	if (lktype == LK_SHARED) {
+		ap->a_flags &= ~(LK_TYPE_MASK | LK_INTERLOCK);
+		ap->a_flags |= LK_DOWNGRADE;
+		(void)VOP_LOCK1_APV(&default_vnodeops, ap);
+	}
+	return (0);
+}
 
 static int
 nfs34_access_otw(struct vnode *vp, int wmode, struct thread *td,

Modified: head/sys/fs/nfsclient/nfsnode.h
==============================================================================
--- head/sys/fs/nfsclient/nfsnode.h	Tue Oct 22 16:09:25 2019	(r353890)
+++ head/sys/fs/nfsclient/nfsnode.h	Tue Oct 22 16:17:38 2019	(r353891)
@@ -163,6 +163,7 @@ struct nfsnode {
 #define	NWRITEOPENED	0x00040000  /* Has been opened for writing */
 #define	NHASBEENLOCKED	0x00080000  /* Has been file locked. */
 #define	NDSCOMMIT	0x00100000  /* Commit is done via the DS. */
+#define	NVNSETSZSKIP	0x00200000  /* Skipped vnode_pager_setsize() */
 
 /*
  * Convert between nfsnode pointers and vnode pointers



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