Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 9 Mar 2009 12:34:33 -0400 (EDT)
From:      Rick Macklem <rmacklem@uoguelph.ca>
To:        Jaakko Heinonen <jh@saunalahti.fi>
Cc:        freebsd-fs@FreeBSD.org
Subject:   Re: [patch] invalidate NFS attribute cache if setattr fails with ESTALE
Message-ID:  <Pine.GSO.4.63.0903091228010.275@muncher.cs.uoguelph.ca>
In-Reply-To: <20090309151400.GA807@a91-153-125-115.elisa-laajakaista.fi>
References:  <200902192210.n1JMAddn009074@svn.freebsd.org> <20090309151400.GA807@a91-153-125-115.elisa-laajakaista.fi>

next in thread | previous in thread | raw e-mail | index | archive | help


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?

For example, a patch something like...
*** nfs_krpc.c.sav	Sun Mar  8 23:45:35 2009
--- nfs_krpc.c	Sun Mar  8 23:47:12 2009
***************
*** 520,527 ****
   		 * If the File Handle was stale, invalidate the lookup
   		 * cache, just in case.
   		 */
! 		if (error == ESTALE)
   			cache_purge(vp);
   		/*
   		 * Skip wcc data on NFS errors for now. NetApp filers
   		 * return corrupt postop attrs in the wcc data for NFS
--- 520,529 ----
   		 * If the File Handle was stale, invalidate the lookup
   		 * cache, just in case.
   		 */
! 		if (error == ESTALE) {
   			cache_purge(vp);
+ 			VTONFS(vp)->n_attrstamp = 0;
+ 		}
   		/*
   		 * Skip wcc data on NFS errors for now. NetApp filers
   		 * return corrupt postop attrs in the wcc data for NFS
*** nfs_socket.c.sav	Mon Mar  9 00:11:51 2009
--- nfs_socket.c	Mon Mar  9 00:12:32 2009
***************
*** 1363,1370 ****
   			 * If the File Handle was stale, invalidate the
   			 * lookup cache, just in case.
   			 */
! 			if (error == ESTALE)
   				cache_purge(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 
--- 1363,1372 ----
   			 * If the File Handle was stale, invalidate the
   			 * lookup cache, just in case.
   			 */
! 			if (error == ESTALE) {
   				cache_purge(vp);
+ 				VTONFS(vp)->n_attrstamp = 0;
+ 			}
   			/*
   			 * 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

rick




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