[PATCH] nfs: don't drop dentry in d_revalidate

[PATCH] nfs: don't drop dentry in d_revalidate

[Miklos Szeredi]

NFS d_revalidate() is doing things that are supposed to be done by
d_invalidate().

Dropping the dentry is especially bad, since it will make
d_invalidate() bypass all checks.

Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
---

Index: linux/fs/nfs/dir.c
===================================================================
--- linux.orig/fs/nfs/dir.c	2005-10-04 13:59:57.000000000 +0200
+++ linux/fs/nfs/dir.c	2005-10-07 12:53:45.000000000 +0200
@@ -762,12 +762,7 @@ out_zap_parent:
 	if (inode && S_ISDIR(inode->i_mode)) {
 		/* Purge readdir caches. */
 		nfs_zap_caches(inode);
-		/* If we have submounts, don't unhash ! */
-		if (have_submounts(dentry))
-			goto out_valid;
-		shrink_dcache_parent(dentry);
 	}
-	d_drop(dentry);
 	unlock_kernel();
 	dput(parent);
 	return 0;

Re: [PATCH] nfs: don't drop dentry in d_revalidate

[Trond Myklebust]

fr den 07.10.2005 Klokka 13:21 (+0200) skreiv Miklos Szeredi:
> NFS d_revalidate() is doing things that are supposed to be done by
> d_invalidate().
> 
> Dropping the dentry is especially bad, since it will make
> d_invalidate() bypass all checks.

NAK!

Bypassing the stupid d_invalidate checks is precisely the point here.

Unlike local filesystems, we have to deal with the case where someone
deletes a file on the server and then creates a new one with the same
name. The d_invalidate checks will keep the wrong dentry hashed for as
long as some borken process has the file open.

Trond

> Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
> ---
> 
> Index: linux/fs/nfs/dir.c
> ===================================================================
> --- linux.orig/fs/nfs/dir.c	2005-10-04 13:59:57.000000000 +0200
> +++ linux/fs/nfs/dir.c	2005-10-07 12:53:45.000000000 +0200
> @@ -762,12 +762,7 @@ out_zap_parent:
>  	if (inode && S_ISDIR(inode->i_mode)) {
>  		/* Purge readdir caches. */
>  		nfs_zap_caches(inode);
> -		/* If we have submounts, don't unhash ! */
> -		if (have_submounts(dentry))
> -			goto out_valid;
> -		shrink_dcache_parent(dentry);
>  	}
> -	d_drop(dentry);
>  	unlock_kernel();
>  	dput(parent);
>  	return 0;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] nfs: don't drop dentry in d_revalidate

[Miklos Szeredi]

> > NFS d_revalidate() is doing things that are supposed to be done by
> > d_invalidate().
> > 
> > Dropping the dentry is especially bad, since it will make
> > d_invalidate() bypass all checks.
> 
> NAK!
> 
> Bypassing the stupid d_invalidate checks is precisely the point here.
> 
> Unlike local filesystems, we have to deal with the case where someone
> deletes a file on the server and then creates a new one with the same
> name. The d_invalidate checks will keep the wrong dentry hashed for as
> long as some borken process has the file open.
> 

d_invalidate() only makes sense for network filesystems like NFS.  If
it's broken, fix it.  But bypassing it is definitely the wrong thing
to do.

Miklos

Re: [PATCH] nfs: don't drop dentry in d_revalidate

[Al Viro]

On Fri, Oct 07, 2005 at 01:21:01PM +0200, Miklos Szeredi wrote:
> NFS d_revalidate() is doing things that are supposed to be done by
> d_invalidate().
> 
> Dropping the dentry is especially bad, since it will make
> d_invalidate() bypass all checks.

NAK.  _IF_ you are going to start playing with that area, start with
handling that stuff in caller before going after methods.

Note that the only relatively sane semantics is to have equivalent on
umount -l done to everything that becomes unreachable.