Re: d_splice_alias() problem.

[Neil Brown]

On Monday May 3, gnb@xxxxxxxxxxxxxxxxx wrote:
> 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):

I seem to recall that this was intentional.  When I first wrote the
code I wanted to leave the splicing code to just splice things, and
when the code that cared about disconnected-ness (in knfsd) discovered
that something really was connected, it would clear the bit
(find_exported_dentry does this). 
However if we want to ensure that there is at-most one disconnected
dentry for an inode, we do need to set the bit more aggressively.
I'll try and review the code with that in mind.  Thanks.

> 
> 
> *   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) {

One problem with this is that (in include/asm-i386/atomic.h at least):
  static __inline__ void atomic_inc(atomic_t *v)

atomic_inc returns "void".

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/