From owner-freebsd-fs@FreeBSD.ORG Thu Mar 12 13:58:39 2009 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 601761065676 for ; Thu, 12 Mar 2009 13:58:39 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 2E9B58FC18 for ; Thu, 12 Mar 2009 13:58:39 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (pool-98-109-39-197.nwrknj.fios.verizon.net [98.109.39.197]) by cyrus.watson.org (Postfix) with ESMTPSA id 9F18B46B03; Thu, 12 Mar 2009 09:58:38 -0400 (EDT) Received: from localhost (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.14.3/8.14.3) with ESMTP id n2CDwVOF046493; Thu, 12 Mar 2009 09:58:32 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-fs@freebsd.org Date: Thu, 12 Mar 2009 08:37:07 -0400 User-Agent: KMail/1.9.7 References: <200902192210.n1JMAddn009074@svn.freebsd.org> <20090309151400.GA807@a91-153-125-115.elisa-laajakaista.fi> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200903120837.08170.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Thu, 12 Mar 2009 09:58:32 -0400 (EDT) X-Virus-Scanned: ClamAV 0.94.2/9100/Thu Mar 12 05:07:56 2009 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Subject: Re: [patch] invalidate NFS attribute cache if setattr fails with ESTALE X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Mar 2009 13:58:39 -0000 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