Re: [autofs] [RFC PATCH]autofs4: hang and proposed fix

[Ian Kent <raven@themaw.net>]

On Fri, 18 Nov 2005, William H. Taber wrote:

> Ram Pai wrote:
> > On Fri, 2005-11-18 at 09:12, William H. Taber wrote:
> 
> > 
> > I think the problem is with cached_lookup(). It is the only place which
> > calls ->revalidate() holding the parent's inode-semaphore AFAICT.
> > 
> > note: cached_lookup() is only called from __lookup_hash() and
> > __lookup_hash() is always called holding the semaphore.
> > 
> > VFS experts agree?
> > RP
> > 
> Ram,
> Lookup_one_len calls lookup_hash and it is the callers of lookup_one_len that
> are problematical.  Just as an example, lookup_one_len is called from
> nfs_sillyrename which is called, among other places in the nfs_rename code.
> In that path the parent i_sem is obtained in do_rename   in the vfs code
> (namei.c). I would think that it would be extremely difficult to to change
> that usage.  The alternative is to move the obtaining of the parent i_sem from
> real_lookup to do_lookup.  We would also have to put the locking around the
> d_revalidate call at return_reval in __link_path_walk.
>
 
Perhaps we are making this altogether to complicated.

I'm sure that there are good reasons for the locking being the way 
it is and any attempt to change it is likely to be a disaster. So what 
about solving this by defining a usage policy based on the intent of 
the functions concerned.

For example.

The lookup_one_len a special use funtion to return the dentry 
corresponding to a path element and by definition it does not follow 
mounts or symlinks. To function correctly autofs needs to follow mounts 
and some time soon I will be posting patches that will use the the 
follow_link method as well.

So the policy could be that if autofs revalidate is called with the 
directory inode semaphore held it must validate the autofs dentry itself 
and not cause a mount request be triggered. The responsibility then 
moves to the filesystem to check if the dentry is an autofs dentry and to 
decide if it needs to then make an unlocked revalidate call. It is easy 
enough to check if the semaphore is held the autofs module. The 
filesystem check is easy enough to do once the filesystem magic number is 
moved to one of the common autofs header files.

Thoughts?

Ian