Date: Thu, 12 Mar 2009 08:37:07 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-fs@freebsd.org Subject: Re: [patch] invalidate NFS attribute cache if setattr fails with ESTALE Message-ID: <200903120837.08170.jhb@freebsd.org> In-Reply-To: <Pine.GSO.4.63.0903091228010.275@muncher.cs.uoguelph.ca> References: <200902192210.n1JMAddn009074@svn.freebsd.org> <20090309151400.GA807@a91-153-125-115.elisa-laajakaista.fi> <Pine.GSO.4.63.0903091228010.275@muncher.cs.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903120837.08170.jhb>