Al Viro <v...@zeniv.linux.org.uk> wrote:
> > d_op->d_automount() may return one of:
> >
> > (a) The vfsmount mounted upon that dentry, in which case pathwalk will
> > move to the root dentry of that vfsmount. ->d_automount() must
> > have in some manner mounted this before returning.
> >
> > (b) NULL if something was already mounted there, in which case
> > pathwalk will loop around and recheck the mountings.
>
> That makes very little sense as-is. Look:
> * autofs4 never does (a)
> * everybody else could replace (a) with (b) just fine - we do (a)
> only when we'd just mounted new vfsmount on top of path. So (b) would lead
> to follow_managed() looping over, finding DCACHE_MOUNTED and cheerfully
> transiting into the root of that vfsmount.
Indeed. It's merely an optimisation and not strictly necessary. I suppose
that, given the amount of time that's probably spent in performing the mount
part of the automount, repeating the check is negligible cost.
> Now, I'd like to have (a) and (b) distinct, but not in that fashion. Namely,
> let's take do_add_mount() et.al. into follow_automount().
I presume that people aren't expected to do things like do_move_mount() here,
but might they want to do a bind mount? Or do we just say if they want to do
something more exotic than do_add_mount(), they have to follow path (b)?
> Leave autofs4 as in your series; it'll be completely unaffected. But switch
> all (b) in nfs/cifs/afs over to modified (a). That is,
> * have vfsmount created as it's done in your series
> * grab extra reference and put it on chosen list. That'd be done
> by helper in fs/namespace.c under namespace_sem. Extra ref would make sure
> that nobody walking the list would decide that it's expirable.
It would be simpler, perhaps, to allow d_automount() to return the list also:
struct vfsmount *(*d_automount)(struct path *mountpoint,
struct list_head **expiry_list_to_use);
This pointer can then be passed directly to do_add_mount() and we don't have
to worry about having an extra reference or cleaning up the list on error.
> * schedule whatever expiry activity we currently do.
> * return vfsmount
> In follow_automount() we'd see that we have non-NULL and non-ERR_PTR. Then
> we'd attempt do_add_mount(), without bothering to pass it expiry list. And
> do the same checks for return value, etc. we currently do in the method
> instances; just remember that we have an extra vfsmount reference that will
> need to be dropped and that we'll need to take the sucker off the expiry list
> in case we decide we don't need it (again, namespace.c helper).
>
> As the result, we stop abusing do_add_mount() in there. Moreover, with
> pending mnt_devname nfs rework we will be able to get rid of passing
> vfsmount to ->d_automount(), AFAICT, which would be nice...
:-)
David
_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs
