Ian Kent wrote:
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
So you are asking that lookup_one_len be modified so that it knows about
the internals of the autofs4 so that it can determine enough to know,
before it makes the revalidate call that the the call is going to pend
so that it can release the lock if it needs to? This does not seem like
a good idea to me. The whole point of having the d_revalidate functions
is so the VFS does not have to know the specifics of any individual
filesystem.
Since there does not appear to be a clear locking policy on
d_revalidate, then the autofs4 revalidate function cannot make
assumptions about that locking state. This means that
autofs4_revalidate cannot pend. I have looked some more at the
real_lookup code, and it is prepared for the case in which the lookup
function returns a dentry other than the one passed in. So here is a
proposal that might work (but I haven't looked at the autofs4 code to
verify this.)
1) A lookup request is made for a non-existant automounted file.
Real_lookup calls autofs4_lookup.
2) Autofs4_lookup saves the information about this request somewhere it
can find it again and wakes up the automount demon that it has work to
do. It does not put a dentry in the dentry cache and it then releases
the parent i_sem and waits for the mount to complete.
3) Any subsequent lookup for this directory that is not from the
automount demon will look for a mount request in progress, and if found,
it will also release the parent lock and add itself to the wait queue.
4)The automount demon will run and get the information that it needs to
complete the mount request and then issue the mount. The lookup
request from mount will call real_lookup. Since the demon is in OZ mode
it does not pend, it fills in the dentry and when the dentry is fully
ready for consumption, it calls d_add and wakes up the waiters.
5) When the waiters wake up, they get the new dentry and real_lookup
will discard the one that had been allocated.
This keeps all of the waiting inside the autofs4 lookup function where
the lock state is defined. I realize that this may be a lot of work,
but I haven't seen a possible solution that doesn't involve that.
Will
_______________________________________________
autofs mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/autofs
