Commit 006ff749 authored by Al Viro's avatar Al Viro
Browse files

saner calling conventions for ->d_automount() 



Currently the calling conventions for ->d_automount() instances have
an odd wart - returned new mount to be attached is expected to have
refcount 2.

That kludge is intended to make sure that mark_mounts_for_expiry() called
before we get around to attaching that new mount to the tree won't decide
to take it out.  finish_automount() drops the extra reference after it's
done with attaching mount to the tree - or drops the reference twice in
case of error.  ->d_automount() instances have rather counterintuitive
boilerplate in them.

There's a much simpler approach: have mark_mounts_for_expiry() skip the
mounts that are yet to be mounted.  And to hell with grabbing/dropping
those extra references.  Makes for simpler correctness analysis, at that...

Reviewed-by: default avatarChristian Brauner <brauner@kernel.org>
Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Reviewed-by: default avatarPaulo Alcantara (Red Hat) <pc@manguebit.com>
Acked-by: default avatarDavid Howells <dhowells@redhat.com>
Tested-by: default avatarDavid Howells <dhowells@redhat.com>
Acked-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 92a09c47

Documentation/filesystems/porting.rst

+7 −0
Original line number Diff line number Diff line
@@ -1203,3 +1203,10 @@ should use d_drop();d_splice_alias() and return the result of the latter. 
If a positive dentry cannot be returned for some reason, in-kernel

clients such as cachefiles, nfsd, smb/server may not perform ideally but

will fail-safe.



---



**mandatory**



Calling conventions for ->d_automount() have changed; we should *not* grab

an extra reference to new mount - it should be returned with refcount 1.

Documentation/filesystems/vfs.rst

+1 −3
Original line number Diff line number Diff line
@@ -1411,9 +1411,7 @@ defined: 


	If a vfsmount is returned, the caller will attempt to mount it

	on the mountpoint and will remove the vfsmount from its

	expiration list in the case of failure.  The vfsmount should be

	returned with 2 refs on it to prevent automatic expiration - the

	caller will clean up the additional ref.

	expiration list in the case of failure.



	This function is only used if DCACHE_NEED_AUTOMOUNT is set on

	the dentry.  This is set by __d_instantiate() if S_AUTOMOUNT is

fs/afs/mntpt.c

+0 −1
Original line number Diff line number Diff line
@@ -189,7 +189,6 @@ struct vfsmount *afs_d_automount(struct path *path) 
	if (IS_ERR(newmnt))

		return newmnt;



	mntget(newmnt); /* prevent immediate expiration */

	mnt_set_expiry(newmnt, &afs_vfsmounts);

	queue_delayed_work(afs_wq, &afs_mntpt_expiry_timer,

			   afs_mntpt_expiry_timeout * HZ);

fs/fuse/dir.c

+0 −3
Original line number Diff line number Diff line
@@ -319,9 +319,6 @@ static struct vfsmount *fuse_dentry_automount(struct path *path) 


	/* Create the submount */

	mnt = fc_mount(fsc);

	if (!IS_ERR(mnt))

		mntget(mnt);



	put_fs_context(fsc);

	return mnt;

}

fs/namespace.c

+3 −6
Original line number Diff line number Diff line
@@ -3902,10 +3902,6 @@ int finish_automount(struct vfsmount *m, const struct path *path) 
		return PTR_ERR(m);



	mnt = real_mount(m);

	/* The new mount record should have at least 2 refs to prevent it being

	 * expired before we get a chance to add it

	 */

	BUG_ON(mnt_get_count(mnt) < 2);



	if (m->mnt_sb == path->mnt->mnt_sb &&

	    m->mnt_root == dentry) {
@@ -3938,7 +3934,6 @@ int finish_automount(struct vfsmount *m, const struct path *path) 
	unlock_mount(mp);

	if (unlikely(err))

		goto discard;

	mntput(m);

	return 0;



discard_locked:
@@ -3952,7 +3947,6 @@ int finish_automount(struct vfsmount *m, const struct path *path) 
		namespace_unlock();

	}

	mntput(m);

	mntput(m);

	return err;

}
@@ -3989,11 +3983,14 @@ void mark_mounts_for_expiry(struct list_head *mounts) 


	/* extract from the expiration list every vfsmount that matches the

	 * following criteria:

	 * - already mounted

	 * - only referenced by its parent vfsmount

	 * - still marked for expiry (marked on the last call here; marks are

	 *   cleared by mntput())

	 */

	list_for_each_entry_safe(mnt, next, mounts, mnt_expire) {

		if (!is_mounted(&mnt->mnt))

			continue;

		if (!xchg(&mnt->mnt_expiry_mark, 1) ||

			propagate_mount_busy(mnt, 1))

			continue;