Re: d_splice_alias() problem.

Greg Banks
2004-05-03T12:03:03Z

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.