On Tue, 19 Feb 2013 10:10:31 +1100
NeilBrown <neilb@xxxxxxx> wrote:
> On Mon, 18 Feb 2013 18:46:09 +0000 Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
