[patch] invalidate NFS attribute cache if setattr fails with ESTALE

[John Baldwin]

On Monday 09 March 2009 12:34:33 pm Rick Macklem wrote:
> 
> On Mon, 9 Mar 2009, Jaakko Heinonen wrote:
> 
> >
> > Hi,
> >
> > Here is a patch which changes nfs_setattrrpc() to invalidate the NFS
> > attribute cache in case the RPC failed with ESTALE.
> >
> > The issue was originally reported by Timo Sirainen in PR kern/123755:
> >
> >> NFS client: open() a file
> >> NFS server: unlink() the file
> >> NFS client: fchown() the file -> ESTALE (as expected)
> >> NFS client: fstat() the file -> success (not expected)
> >
> > %%%
> > Index: sys/nfsclient/nfs_vnops.c
> > ===================================================================
> > --- sys/nfsclient/nfs_vnops.c	(revision 188842)
> > +++ sys/nfsclient/nfs_vnops.c	(working copy)
> > @@ -838,6 +838,10 @@ nfs_setattrrpc(struct vnode *vp, struct
> > 		nfsm_loadattr(vp, NULL);
> > 	m_freem(mrep);
> > nfsmout:
> > +	/* Invalidate the attribute cache if the NFS file handle is stale. */
> > +	if (error == ESTALE)
> > +		np->n_attrstamp = 0;
> > +
> > 	return (error);
> > }
> >
> > %%%
> >
> > Could someone take a look if this could be committed?
> >
> I'm not a committer, but I think that it might be appropriate to 
> invalidate the attribute cache for any ESTALE reply from a server.
> (Since ESTALE implies that the file no linger exists on the server,
> I can't think why the attribute cache should remain valid for anything.)
> 
> There is already code in nfs_request() that purges the name cache for
> ESTALE and I think that invalidating the cache there too, might make
> sense?

I think we should purge the access cache too?  I just put this in my p4 branch 
yesterday (it also has a merge of your changes to expand the per-node access 
cache, so the access cache clearing is a bit larger than it would be 
otherwise):

--- //depot/projects/smpng/sys/nfs4client/nfs4_socket.c	2008/11/03 21:11:59
+++ //depot/user/jhb/lock/nfs4client/nfs4_socket.c	2009/03/11 22:15:03
@@ -259,7 +259,7 @@
 	 ** lookup cache, just in case.
 	 **/
 	if (error == ESTALE)
-		cache_purge(vp);
+		nfs_purgecache(vp);
 
 	return (error);
 }
--- //depot/projects/smpng/sys/nfsclient/nfs.h	2008/11/18 23:25:45
+++ //depot/user/jhb/lock/nfsclient/nfs.h	2009/03/11 22:15:03
@@ -319,6 +322,7 @@
 #endif /* ! NFS4_USE_RPCCLNT */
 #endif
 
+void	nfs_purgecache(struct vnode *);
 int	nfs_vinvalbuf(struct vnode *, int, struct thread *, int);
 int	nfs_readrpc(struct vnode *, struct uio *, struct ucred *);
 int	nfs_writerpc(struct vnode *, struct uio *, struct ucred *, int *,
--- //depot/projects/smpng/sys/nfsclient/nfs_krpc.c	2008/11/03 21:11:59
+++ //depot/user/jhb/lock/nfsclient/nfs_krpc.c	2009/03/11 22:15:03
@@ -521,7 +521,7 @@
 		 * cache, just in case.
 		 */
 		if (error == ESTALE)
-			cache_purge(vp);
+			nfs_purgecache(vp);
 		/*
 		 * Skip wcc data on NFS errors for now. NetApp filers
 		 * return corrupt postop attrs in the wcc data for NFS
--- //depot/projects/smpng/sys/nfsclient/nfs_socket.c	2008/11/03 21:11:59
+++ //depot/user/jhb/lock/nfsclient/nfs_socket.c	2009/03/11 22:15:03
@@ -1364,7 +1364,7 @@
 			 * lookup cache, just in case.
 			 */
 			if (error == ESTALE)
-				cache_purge(vp);
+				nfs_purgecache(vp);
 			/*
 			 * Skip wcc data on NFS errors for now. NetApp filers return corrupt
 			 * postop attrs in the wcc data for NFS err EROFS. Not sure if they 
--- //depot/projects/smpng/sys/nfsclient/nfs_subs.c	2008/11/03 21:11:59
+++ //depot/user/jhb/lock/nfsclient/nfs_subs.c	2009/03/11 22:15:03
@@ -826,6 +826,27 @@
 	return (0);
 }
 
+/*
+ * Purge all cached information about an NFS vnode including name
+ * cache entries, the attribute cache, and the access cache.  This is
+ * called when an NFS request for a node fails with a stale
+ * filehandle.
+ */
+void
+nfs_purgecache(struct vnode *vp)
+{
+	struct nfsnode *np;
+	int i;
+
+	np = VTONFS(vp);
+	cache_purge(vp);
+	mtx_lock(&np->n_mtx);
+	np->n_attrstamp = 0;
+	for (i = 0; i < NFS_ACCESSCACHESIZE; i++)
+		np->n_accesscache[i].stamp = 0;
+	mtx_unlock(&np->n_mtx);
+}
+
 static nfsuint64 nfs_nullcookie = { { 0, 0 } };
 /*
  * This function finds the directory cookie that corresponds to the

-- 
John Baldwin