From: Al Viro Date: Thu, 24 Apr 2025 05:45:05 +0000 (-0400) Subject: saner calling conventions for ->d_automount() X-Git-Tag: v6.16-rc1~104^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=006ff7498fe89bd9dfb891101f02557d5cfcf427;p=thirdparty%2Fkernel%2Flinux.git 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 Reviewed-by: Jeff Layton Reviewed-by: Paulo Alcantara (Red Hat) Acked-by: David Howells Tested-by: David Howells Acked-by: Steven Rostedt (Google) Signed-off-by: Al Viro --- diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 767b2927c762d..749637231773d 100644 --- 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 ae79c30b6c0cc..cc0a58e967705 100644 --- 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 45cee6534122e..9434a5399f2b0 100644 --- 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 83ac192e7fdd1..05d8584fd3b91 100644 --- 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 98a5cd756e9ae..018e95fe5459e 100644 --- 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 973aed9cc5fe9..7f1ec9c67ff21 100644 --- 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 e3f9213131c46..778daf11f1dbd 100644 --- 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 5b8db27fb6ef3..e22aacb0028ae 100644 --- 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; }