Trond Myklebust wrote:
On Wed, 2005-11-30 at 16:30 -0500, William H. Taber wrote:
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.
Problem with that, as I understand it and Ian Kent knows better than I,
is that the autofs lookup code creates the dentry and fills it in
partially and marks it as waiting for mounting and wakes up the
automount demon. The demon completes the mount and finishes filling in
the dentry. So we cannot have some other lookup coming in and removing
the dentry on us. At least that is what I understand from Ian's answer
when I proposed the same sort of thing to him.
What do you mean by "removing the dentry on us"? It is perfectly
possible to have lookup() return the original dentry every time, which
is precisely what I suggested above.
What I meant was that the autofs code created this dentry and then
called d_add to put it in the hash chain, woke up the autofs demon to
perform the mount then waited for the mount to complete. The autofs
code is, I think, intending for the demon to find this entry in a
revalidate and complete the mount. But Ian knows better than I. Anyway
I did not think that it would be good for a racing call to do_lookup to
unhash a dentry that the automounter was expecting to find.
I was probably unclear when I referred to lookup. I meant it in the
generic sense of do_lookup or lookup_one_len and not the i_op->lookup
function. I had already suggested to Ian that they not d_add the dentry
in autofs4_lookup until the mount demon came in to complete the mount
and have autofs4_lookup be responsible for queing up subsequent lookups
until the mount completed and moving the code for that out of
autofs4_revalidate. He allowed how it would be possible but a lot of work.
Even if they end up
doing something like that in a future version of the automounter, I
would still like a simple patch that can be applied to existing systems
as an interim fix.
"Interim" fixes to the entire VFS API such as the ones that have
proposed here tend to be a poor idea...
Which is why I was discussing the ideas here. From the start I have
been asking for input from people with more understanding than I have of
the subleties of VFS locking. I have been trying to find a solution
short of waiting for autofs5 since we are seeing the problem now. But
obviously we don't want a fix that causes more problems. My original
thought was that the solution to the problem was to make the locking
requirements for d_revalidate consistent. I now have a greater
understanding of why things are as they are. In another post I have
outlined what I think a workable solution is that is confined to the
autofs code. Your input it has been quite helpful in helping me
understand all of this. Thanks.
Will
_______________________________________________
autofs mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/autofs
