Re: More fun with unmounting ESTALE directories.

[Jeff Layton <jlayton@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3574 bytes --]

On Tue, 19 Feb 2013 10:10:31 +1100
NeilBrown <neilb@suse.de> wrote:

> On Mon, 18 Feb 2013 18:46:09 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> > On Mon, Feb 18, 2013 at 01:25:09PM +1100, NeilBrown wrote:
> > 
> > > I would be really nice if sys_unmount used a LOOKUP_MOUNTPOINT flag that
> > > works a bit like LOOKUP_PARENT and LOOKUP_NOFOLLOW in that it skips the very
> > > last step and returns the mounted-on directory, not the mountpoint that is
> > > mounted there - or at least makes sure not revalidate happens on that final
> > > mounted directory.
> > 
> > I don't think LOOKUP_MOUNTPOINT is a good idea.  For one thing, we have
> > fairly few places that might want it, all of them in core VFS.  Might as
> > well provide a separate function for them, a-la path_lookupat() vs.
> > path_openat().
> > 
> > For another, we need to decide what to do with a really nasty corner case:
> > 	a/b is a mountpoint, with c/d bound on it.
> > 	c/d is a symlink to c/e
> > 	c/e is a mountpoint
> > What should umount("a/b", 0) do?  There are two possibilities - removing
> > vfsmount on top of a/b or one on top of c/e...
> > 
> > We have the latter semantics; _that_ is what this GETATTR is about.  It's
> > a fairly obscure corner case - the question is not even whether to follow
> > symlinks, it's whether to follow _mounts_ on the last component.  Hell
> > knows; I'm seriously tempted to change it "don't follow mounts" and see if
> > anyone complains.  The only case when behaviour would change would be
> > a symlink mounted somewhere (note that this is _not_ something that can easily
> > happen; e.g. mount --bind does follow symlinks) and umount(2) given the
> > path resolving to the mountpoint of that symlink.
> 
> Thinking about this some more, I now realise that my LOOKUP_MOUNTPOINT idea
> was too simplistic and missed the real point.
> 
> The real point is that for unmount, we want to resolve the the path without
> any reference to any filesystem at all - the lookup should be handled
> entirely by the dcache.
> Any mountpoint is pinned in the dcache, and consequently any ancestor of any
> mount point also is.  So the dcache will lead us to the dentry that we want.
> 
> And the dentry is *all* we want.  It doesn't really matter what the inode is
> like, or whether the filesystem thinks that the inode or name still exist.
> All we need to do is find a dentry that must be  in the cache, and detach the
> mount that is there.
> 
> Whether that is achieved by a LOOKUP_ flag or a separate lookup function
> doesn't matter much to me.
> 
> I suspect we need to allow for passing a symlink to unmount, and the symlink
> might not be in cache, so we cannot insist uniformly on only using the dcache.
> However if a name is in the cache, and the cached data suggests that it is a
> directory, then we should trust that and avoid referring to the filesystem.
> 
> umount is really quite unique in this.  All other times we lookup a path we
> want to use the thing we found.  With umount, we want to stop using it.
> 

From an IRC conversation with Al yesterday, which may point you in the
right direction:

12:49 < viro> jlayton: umount() simply shouldn't do full lookup for path
12:50 < viro> it should get the parent
12:50 < viro> _then_ do pure dcache lookup for the last step

...of course that's still tricky if the last component is a symlink
since you'd need to chase it by hand, but that seems like a reasonable
way to start.

-- 
Jeff Layton <jlayton@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]