On Wed, 2005-11-30 at 15:32 -0500, William H. Taber wrote:
> Not only is there this case, but the original premise is wrong as well.
> There is a second case in which a d_revalidate function is called with
> the parent i_sem and that is when it is called from inside of
> lookup_one_len. What makes this tricky is that lookup_one_len is called
> from nfs_sillyrename from inside of nfs_rename which is called,
> naturally enough by sys_rename. The rename code is very careful about
> the order in which it obtains the parent semaphores because it needs to
> get two of them. It must always obtain the locks in the same order so
> that does not get into a deadly embrace. If we start arbitrarily
> releasing a parent semaphore in cached_lookup and taking it again after
> the revalidate, we risk breaking the lock ordering and creating a deadly
> embrace.
>
> When I started writing this I thought that it would be safe for the
> autofs revalidate code to release the parent semaphore because they do
> not have a rename callback. But I looked again at the rename code and
> it calls lookup_hash on the final source and destination files after
> locking the parents so the potential for a deadly embrace still exists
> unless there is some other assurance that these final lookups will never
> pend waiting on the automounter in either their revalidate or lookup
> routines. (Actually the requirement is that they never give up the
> parent i_sem lock, but the lookup code has to give up the lock so that
> the autofs demon can run and perform the mount so it amounts to the same
> thing.)
>
> The same issue exists for devfs which also releases the parent i_sem
> lock so that it can wait inside its revalidation routine.
So exactly why does autofs4 want to hold the dir->i_sem in d_revalidate
in the first place? Can't we move any code that requires dir->i_sem to
be held into a ->lookup() method?
Trivially, if you have a d_revalidate that does something like
int autofs_revalidate(struct dentry *dentry, struct nameidata *nd)
{
d_drop(dentry);
return 0;
}
then the VFS will currently allocate a new dentry with the same name,
and call ->lookup() on it without dropping dir->i_sem. If you still need
to reference the old dentry, then put it on a private list somewhere.
That would also allow you to return the old dentry as the result of the
->lookup() operation if that is desirable.
Cheers,
Trond
_______________________________________________
autofs mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/autofs
