Re: 2.5.70-bk16: nfs crash

[Dipankar Sarma]

On Thu, Jun 12, 2003 at 11:13:31PM -0700, Trond Myklebust wrote:
> >>>>> " " == Dipankar Sarma <[e-mail]> writes:
> d_lookup() is the only place where someone can pick up an existing
> dentry for which they do not already hold a reference.

AFAICS, the way to do a perfectly safe "sole ownership" check is
to grab the per-dentry lock (instead of dcache_lock) and
do the check.

>     >> d_invalidate(), d_prune_aliases(), prune_dcache(),
>     >> shrink_dcache_sb() are but a few functions that rely on the
>     >> above code snippet working to keep d_lookup() from intruding.
> 
>      > Those routines hold the per-dentry lock as required and that
>      > protects them from intruding lockfree d_lookup().
> 
> d_invalidate() does not. d_prune_aliases() does not. d_unhash() does
> not.
> 
> Down in the per-filesystem code, I know of several locations in the
> NFS code that do not. There's one in procfs. I'm sure you can find
> more examples in the other filesystems too...

Yes and we need an audit. Besides I see places where d_count check
is being done without holding *any* lock. That is ok only if the
dentry is guranteed to be unhashed and we are doing a "sole ownership"
check. Otherwise it may not work.

> 
> Your argument only holds water if you demand that all callers of
> d_drop() should also take the per-dentry lock. AFAICS this is not
> being enforced.

There are some rules that we need to work out irrespective of
RCU and document clearly -

1. d_unhashed() checks that are being done with and without dcache_lock -

	if (!d_unhashed(new_dentry)) {
		d_drop(new_dentry);
		rehash = new_dentry;
	}
This seems to require that either we hold the dcache_lock or the operations
that we do on the dentry allow unhashed dentries.

2. Checking for "sole ownership" of dentries. Depending on whether the
   dentry is hashed and what operation we are going to do, we will
   need to hold the per-dentry lock.

I am glad that you are reviewing this. Is there any other dcache
operations in filesystems that you would like to add to the list
above ?

Thanks
Dipankar