]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
sanitize handling of long-term internal mounts
authorAl Viro <viro@zeniv.linux.org.uk>
Sat, 3 May 2025 01:32:01 +0000 (21:32 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Sun, 29 Jun 2025 22:13:41 +0000 (18:13 -0400)
Original rationale for those had been the reduced cost of mntput()
for the stuff that is mounted somewhere.  Mount refcount increments and
decrements are frequent; what's worse, they tend to concentrate on the
same instances and cacheline pingpong is quite noticable.

As the result, mount refcounts are per-cpu; that allows a very cheap
increment.  Plain decrement would be just as easy, but decrement-and-test
is anything but (we need to add the components up, with exclusion against
possible increment-from-zero, etc.).

Fortunately, there is a very common case where we can tell that decrement
won't be the final one - if the thing we are dropping is currently
mounted somewhere.  We have an RCU delay between the removal from mount
tree and dropping the reference that used to pin it there, so we can
just take rcu_read_lock() and check if the victim is mounted somewhere.
If it is, we can go ahead and decrement without and further checks -
the reference we are dropping is not the last one.  If it isn't, we
get all the fun with locking, carefully adding up components, etc.,
but the majority of refcount decrements end up taking the fast path.

There is a major exception, though - pipes and sockets.  Those live
on the internal filesystems that are not going to be mounted anywhere.
They are not going to be _un_mounted, of course, so having to take the
slow path every time a pipe or socket gets closed is really obnoxious.
Solution had been to mark them as long-lived ones - essentially faking
"they are mounted somewhere" indicator.

With minor modification that works even for ones that do eventually get
dropped - all it takes is making sure we have an RCU delay between
clearing the "mounted somewhere" indicator and dropping the reference.

There are some additional twists (if you want to drop a dozen of such
internal mounts, you'd be better off with clearing the indicator on
all of them, doing an RCU delay once, then dropping the references),
but in the basic form it had been
* use kern_mount() if you want your internal mount to be
a long-term one.
* use kern_unmount() to undo that.

Unfortunately, the things did rot a bit during the mount API reshuffling.
In several cases we have lost the "fake the indicator" part; kern_unmount()
on the unmount side remained (it doesn't warn if you use it on a mount
without the indicator), but all benefits regaring mntput() cost had been
lost.

To get rid of that bitrot, let's add a new helper that would work
with fs_context-based API: fc_mount_longterm().  It's a counterpart
of fc_mount() that does, on success, mark its result as long-term.
It must be paired with kern_unmount() or equivalents.

Converted:
1) mqueue (it used to use kern_mount_data() and the umount side
is still as it used to be)
2) hugetlbfs (used to use kern_mount_data(), internal mount is
never unmounted in this one)
3) i915 gemfs (used to be kern_mount() + manual remount to set
options, still uses kern_unmount() on umount side)
4) v3d gemfs (copied from i915)

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
drivers/gpu/drm/i915/gem/i915_gemfs.c
drivers/gpu/drm/v3d/v3d_gemfs.c
fs/hugetlbfs/inode.c
fs/namespace.c
include/linux/mount.h
ipc/mqueue.c

index 65d84a93c52532b6f207f1b6123dd7b5b347e3a3..a09e2eb47175617f8609efc2d53773d217e85cf1 100644 (file)
@@ -5,16 +5,23 @@
 
 #include <linux/fs.h>
 #include <linux/mount.h>
+#include <linux/fs_context.h>
 
 #include "i915_drv.h"
 #include "i915_gemfs.h"
 #include "i915_utils.h"
 
+static int add_param(struct fs_context *fc, const char *key, const char *val)
+{
+       return vfs_parse_fs_string(fc, key, val, strlen(val));
+}
+
 void i915_gemfs_init(struct drm_i915_private *i915)
 {
-       char huge_opt[] = "huge=within_size"; /* r/w */
        struct file_system_type *type;
+       struct fs_context *fc;
        struct vfsmount *gemfs;
+       int ret;
 
        /*
         * By creating our own shmemfs mountpoint, we can pass in
@@ -38,8 +45,16 @@ void i915_gemfs_init(struct drm_i915_private *i915)
        if (!type)
                goto err;
 
-       gemfs = vfs_kern_mount(type, SB_KERNMOUNT, type->name, huge_opt);
-       if (IS_ERR(gemfs))
+       fc = fs_context_for_mount(type, SB_KERNMOUNT);
+       if (IS_ERR(fc))
+               goto err;
+       ret = add_param(fc, "source", "tmpfs");
+       if (!ret)
+               ret = add_param(fc, "huge", "within_size");
+       if (!ret)
+               gemfs = fc_mount_longterm(fc);
+       put_fs_context(fc);
+       if (ret)
                goto err;
 
        i915->mm.gemfs = gemfs;
index 4c5e18590a5cf1d835fb2836b3753fecf8133a71..8ec6ed82b3d94105b6ebe720f548c8cd4fcd083a 100644 (file)
@@ -3,14 +3,21 @@
 
 #include <linux/fs.h>
 #include <linux/mount.h>
+#include <linux/fs_context.h>
 
 #include "v3d_drv.h"
 
+static int add_param(struct fs_context *fc, const char *key, const char *val)
+{
+       return vfs_parse_fs_string(fc, key, val, strlen(val));
+}
+
 void v3d_gemfs_init(struct v3d_dev *v3d)
 {
-       char huge_opt[] = "huge=within_size";
        struct file_system_type *type;
+       struct fs_context *fc;
        struct vfsmount *gemfs;
+       int ret;
 
        /*
         * By creating our own shmemfs mountpoint, we can pass in
@@ -28,8 +35,16 @@ void v3d_gemfs_init(struct v3d_dev *v3d)
        if (!type)
                goto err;
 
-       gemfs = vfs_kern_mount(type, SB_KERNMOUNT, type->name, huge_opt);
-       if (IS_ERR(gemfs))
+       fc = fs_context_for_mount(type, SB_KERNMOUNT);
+       if (IS_ERR(fc))
+               goto err;
+       ret = add_param(fc, "source", "tmpfs");
+       if (!ret)
+               ret = add_param(fc, "huge", "within_size");
+       if (!ret)
+               gemfs = fc_mount_longterm(fc);
+       put_fs_context(fc);
+       if (ret)
                goto err;
 
        v3d->gemfs = gemfs;
index e4de5425838d92c861d5e1f4811ab6ec2582ccc3..4e039777516793e5144c073ad6ace532c1de309e 100644 (file)
@@ -1587,7 +1587,7 @@ static struct vfsmount *__init mount_one_hugetlbfs(struct hstate *h)
        } else {
                struct hugetlbfs_fs_context *ctx = fc->fs_private;
                ctx->hstate = h;
-               mnt = fc_mount(fc);
+               mnt = fc_mount_longterm(fc);
                put_fs_context(fc);
        }
        if (IS_ERR(mnt))
index 57b0974a5d1e82c279b6cff5b7b9e9954304d488..6a0697eeda74263bc7c967f33584f73e86663910 100644 (file)
@@ -1260,6 +1260,15 @@ struct vfsmount *fc_mount(struct fs_context *fc)
 }
 EXPORT_SYMBOL(fc_mount);
 
+struct vfsmount *fc_mount_longterm(struct fs_context *fc)
+{
+       struct vfsmount *mnt = fc_mount(fc);
+       if (!IS_ERR(mnt))
+               real_mount(mnt)->mnt_ns = MNT_NS_INTERNAL;
+       return mnt;
+}
+EXPORT_SYMBOL(fc_mount_longterm);
+
 struct vfsmount *vfs_kern_mount(struct file_system_type *type,
                                int flags, const char *name,
                                void *data)
index 1a508beba44601b23871989e3244b5155814f362..c145820fcbbf30b9b94d8864e8fa1ea539f59f94 100644 (file)
@@ -98,6 +98,7 @@ int mnt_get_write_access(struct vfsmount *mnt);
 void mnt_put_write_access(struct vfsmount *mnt);
 
 extern struct vfsmount *fc_mount(struct fs_context *fc);
+extern struct vfsmount *fc_mount_longterm(struct fs_context *fc);
 extern struct vfsmount *vfs_create_mount(struct fs_context *fc);
 extern struct vfsmount *vfs_kern_mount(struct file_system_type *type,
                                      int flags, const char *name,
index 82ed2d3c98463ebfedb2a476b251b51abacf97a3..de7432efbf4a5b3490efda482e13cb16ce2fc7f3 100644 (file)
@@ -482,7 +482,7 @@ static struct vfsmount *mq_create_mount(struct ipc_namespace *ns)
        put_user_ns(fc->user_ns);
        fc->user_ns = get_user_ns(ctx->ipc_ns->user_ns);
 
-       mnt = fc_mount(fc);
+       mnt = fc_mount_longterm(fc);
        put_fs_context(fc);
        return mnt;
 }