Re: [PATCH 0/3] Fixes for vfs-scale and vfs-automount

[Al Viro]

On Thu, Feb 24, 2011 at 02:34:20PM +0800, Ian Kent wrote:

> It does now but it doesn't do a whole lot, just checks for a negative
> dentry, returning false, and drops out of RCU mode when the dentry isn't
> selected or being checked for expiry yet.

Er?  Where does ->d_revalidate() exist in fs/autofs4?  Mainline doesn't
have anything of that kind...
 
> I've seen walks fail with a dentry that has gone away (ie. I'm walking
> in a tree that is being expired, they seem to be trees with no actual
> mount at their base), which should be because I'm not catching the block
> request at a higher level dentry. Unfortunately, including a check for
> the base dentry in __follow_mount_rcu(), regardless of it being a
> mountpoint, doesn't always catch the need to block either. That the tree
> is being selected for expire is probably because the expire can't detect
> a walk within the mount tree when the walker is between the
> __d_lookup_rcu() and __follow_mount_rcu() calls. Jumping out of rcu-walk
> mode in an added revalidate reduces the possibility of this happening
> (assuming walks are sneaking in between dropping and acquiring the
> vfsmount_lock) but doesn't eliminate it. 
> 
> At this point I loop back to the trying to work out why a reference gets
> picked up and the cycle repeats with various small changes to the
> overall approach.
> 
> Actually, it's not just the root of a mount that is getting an extra
> reference either. When I notice a root dentry has picked up a reference
> (from a printk) and SIGKILL everything and start umounting manually I
> get to a point in the tree where I need to umount autofs mounts twice,
> which usually means nothing more than a get/put mismatch somewhere, but
> in that case I normally don't see a root dentry pick up a reference.
> That's confusing me too.

OK, now I'm really confused.  Where does your tree live?

I really don't like the look of autofs4_tree_busy(), BTW - you don't need
RCU to get races here; just a plain chdir("../../.."); done by a process
that sits deep enough in subtree you are checking can very well race with
your check, so that your iterator gets through the new cwd before chdir()
and through the old one after it...

Maybe I'm missing something subtle here.  A braindump on how you expect
all "we choose dentry for expiry" stuff to work and what is really required
from it would be nice...
 
> > BTW, Nick has moved to kernel.dk; whether he's still reachable there is
> > a question, though - he seems to have disappeared since mid-January.
> 
> Yeah, I've forwarded a couple of messages where I forgot to change the
> email. A few emails back Nick said he was following the thread anyway.

Could you _please_ forward to him the question I tried to ask, without
any success so far?  Namely, what was the reason for reverting do_filp_open()
to use of do_path_lookup() in non-create case?  I have a couple of guesses,
but...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/