Re: [PATCH 1/7] autofs4: Save autofs trigger's vfsmount in super block info

[Al Viro <viro@ZenIV.linux.org.uk>]

On Mon, Jan 18, 2010 at 05:14:58PM +0800, Ian Kent wrote:

> The possibility of more than one vfsmount being present is, as you say
> possible, but it is not legal for autofs (and last time I checked I
> concluded it wasn't possible for me to veto bind mount requests). Other
> than bind mounts I'm struggling to think of a case where I would have
> more than one autofs fs mount with the same s_dev.

That's interesting...  Other place where we go through the stack of mounts
is where we look for one with given bits in type; do we have a possibility
of multiple candidates there and which one do we really want?  Same function,
case when we have ioctlfd == -1, !autofs_type_any(type).  Current code
(as well as original) goes for one closest to root; it would certainly be
simpler to go for one on top of stack...

> Yes, the dentry should always be positive here but let me think about it
> a little more in case I'm missing something. And yes, using the vfsmount
> super block pointer would be better. I'll fix these too.

FWIW, I'd rather have that stuff in vfs tree; that's not the only patch
eliminating mnt_mountpoint use.  AFAICT, none of the uses outside of
core VFS code are legitimate (and BTW, NFSv4 one is contrary to RFC - it
should do follow_down() in loop before reporting mount_fileid instead of
just picking the immediate ancestor for one thing and I'm not at all sure
that check around that thing is correct; it ignores export path.dentry
and looks at path.mnt.root instead, which seems to be wrong).  Other places
would be just as happy with mnt/mnt->mnt_root instead of mnt->mnt_parent/
mnt->mnt_mountpoint or, worse, mnt/mnt->mnt_mountpoint.  Pohmelfs is even
nastier, with its d_path() abuses, but that's a separate story.

IMO we ought to get rid of mnt_mountpoint/mnt_parent uses outside of core
VFS and I'd rather do that in one patch queue.

Back to another question: which syscalls should and which syscalls should not 
trigger automount on the last component?  Note that it's something we'd better
be consistent about between autofs4 and cifs/afs/nfs...