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: Christian Brauner <brauner@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Acked-by: David Howells <dhowells@redhat.com>
Tested-by: David Howells <dhowells@redhat.com>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index 767b2927c762d7bec6cc374ced59920abfb8ca16..749637231773d80cc3af21d9916f266db45cac26 100644 (file)
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -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.

diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index ae79c30b6c0cc0c0aac442cd767770a83a82dae7..cc0a58e9677050b75c1ef62ee2072a698eb00764 100644 (file)
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -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

diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 45cee6534122e8d97ccbd09b9f3b166adaa144ab..9434a5399f2b044f6c2e56e43d4195df6a4c4e76 100644 (file)
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -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);

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 83ac192e7fdd1929fa42b583c14ce75c5672e3ca..05d8584fd3b91b75df9f8c2442313b42544b03c3 100644 (file)
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -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;
 }

diff --git a/fs/namespace.c b/fs/namespace.c
index 98a5cd756e9ae44120f5f110fc233dc882cbc3f8..018e95fe5459e6a82179cf9534ebb590b82cc6a9 100644 (file)
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -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 @@ discard:
                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;

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 973aed9cc5fe97080e5e9b096e7758e982a95acf..7f1ec9c67ff21de7e2ce39431526b8dee75e7f49 100644 (file)
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -195,7 +195,6 @@ struct vfsmount *nfs_d_automount(struct path *path)
        if (IS_ERR(mnt))
                goto out_fc;
 
-       mntget(mnt); /* prevent immediate expiration */
        if (timeout <= 0)
                goto out_fc;
 

diff --git a/fs/smb/client/namespace.c b/fs/smb/client/namespace.c
index e3f9213131c4673a9381ba5c88f5a27f6ff660b2..778daf11f1dbdaf85db62b01a797bd4666391451 100644 (file)
--- a/fs/smb/client/namespace.c
+++ b/fs/smb/client/namespace.c
@@ -283,7 +283,6 @@ struct vfsmount *cifs_d_automount(struct path *path)
                return newmnt;
        }
 
-       mntget(newmnt); /* prevent immediate expiration */
        mnt_set_expiry(newmnt, &cifs_automount_list);
        schedule_delayed_work(&cifs_automount_task,
                              cifs_mountpoint_expiry_timeout);

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 5b8db27fb6ef37482c386c409caf0ec81e55028f..e22aacb0028aef24a6cba9de7573516a6a1de152 100644 (file)
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10088,8 +10088,6 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
        put_filesystem(type);
        if (IS_ERR(mnt))
                return NULL;
-       mntget(mnt);
-
        return mnt;
 }