Re: d_splice_alias() problem.

[Greg Banks <gnb@melbourne.sgi.com>]

Neil Brown wrote:
> 
> This problem can be resolved by making sure that an inode never has
> both a connected and a disconnected dentry.
> 
> This is already the case for directories (as they must only have one
> dentry), but it is not the case for non-directories.
> 
> The following patch tries to address this.  It is a "technology
> preview" in that the only testing I have done is that it compiles OK.
> 
> Please consider reviewing it to see if it makes sense.

It does, and it fixes one of the dcache bugs that was tripping my debug
code.  Here are a couple more.

*   Logic bug in d_splice_alias() forgets to clear the DCACHE_DISCONNECTED
    flag when a lookup connects a disconnected dentry.  Fix is (relative
    to Neil's patch):

--- linux.orig/fs/dcache.c	Mon May  3 21:46:30 2004
+++ linux/fs/dcache.c	Mon May  3 21:49:07 2004
@@ -894,6 +895,7 @@
 		new = __d_find_alias(inode, 1);
 		if (new) {
 			BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
+			new->d_flags &= ~DCACHE_DISCONNECTED;
 			spin_unlock(&dcache_lock);
 			security_d_instantiate(new, inode);
 			d_rehash(dentry);


*   Dentry_stat.nr_unused can be be spuriously decremented when dput()
    races with __dget_unlocked().  Eventual result is nr_unused<0
    and kswapd loops.  This is the problem I mentioned earlier.  Note
    that this is not an NFS-specific problem.  Fix is:

--- linux.orig/fs/dcache.c	Mon May  3 21:46:30 2004
+++ linux/fs/dcache.c	Mon May  3 21:49:07 2004
@@ -255,8 +255,8 @@
 
 static inline struct dentry * __dget_locked(struct dentry *dentry)
 {
-	atomic_inc(&dentry->d_count);
-	if (atomic_read(&dentry->d_count) == 1) {
+	if (atomic_inc(&dentry->d_count) == 1) {
+	    	BUG_ON(list_empty(&dentry->d_lru));
 		dentry_stat.nr_unused--;
 		list_del_init(&dentry->d_lru);
 	}
@@ -663,6 +663,7 @@
 		if (gfp_mask & __GFP_FS)
 			prune_dcache(nr);
 	}
+	BUG_ON(dentry_stat.nr_unused < 0);
 	return dentry_stat.nr_unused;
 }
 

Greg.
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.