linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] fs: hide names_cache behind runtime const machinery
@ 2025-12-01  8:32 Mateusz Guzik
  2025-12-01  8:51 ` Al Viro
  0 siblings, 1 reply; 9+ messages in thread
From: Mateusz Guzik @ 2025-12-01  8:32 UTC (permalink / raw)
  To: viro; +Cc: brauner, jack, linux-kernel, linux-fsdevel, Mateusz Guzik

s/names_cachep/names_cache/ for consistency with dentry cache.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---

v2:
- rebased on top of work.filename-refcnt

ACHTUNG: there is a change queued for 6.19 merge window which treats
dentry cache the same way:
commit 21b561dab1406e63740ebe240c7b69f19e1bcf58
Author: Mateusz Guzik <mjguzik@gmail.com>
Date:   Wed Nov 5 16:36:22 2025 +0100

    fs: hide dentry_cache behind runtime const machinery

which would result in a merge conflict in vmlinux.lds.h. thus I
cherry-picked before generating the diff to avoid the issue for later.

 fs/namei.c                        | 16 ++++++++++------
 include/asm-generic/vmlinux.lds.h |  3 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d6eac90084e1..eff4cbffe241 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -41,6 +41,8 @@
 #include <linux/init_task.h>
 #include <linux/uaccess.h>
 
+#include <asm/runtime-const.h>
+
 #include "internal.h"
 #include "mount.h"
 
@@ -124,23 +126,25 @@
  */
 
 /* SLAB cache for struct filename instances */
-static struct kmem_cache *names_cachep __ro_after_init;
+static struct kmem_cache *__names_cache __ro_after_init;
+#define names_cache	runtime_const_ptr(__names_cache)
 
 void __init filename_init(void)
 {
-	names_cachep = kmem_cache_create_usercopy("names_cache", sizeof(struct filename), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, offsetof(struct filename, iname),
-						EMBEDDED_NAME_MAX, NULL);
+	__names_cache = kmem_cache_create_usercopy("names_cache", sizeof(struct filename), 0,
+			 SLAB_HWCACHE_ALIGN|SLAB_PANIC, offsetof(struct filename, iname),
+			 EMBEDDED_NAME_MAX, NULL);
+	runtime_const_init(ptr, __names_cache);
 }
 
 static inline struct filename *alloc_filename(void)
 {
-	return kmem_cache_alloc(names_cachep, GFP_KERNEL);
+	return kmem_cache_alloc(names_cache, GFP_KERNEL);
 }
 
 static inline void free_filename(struct filename *p)
 {
-	kmem_cache_free(names_cachep, p);
+	kmem_cache_free(names_cache, p);
 }
 
 static inline void initname(struct filename *name)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 20939d2445e7..3abd76ac723a 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -956,7 +956,8 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 #define RUNTIME_CONST_VARIABLES						\
 		RUNTIME_CONST(shift, d_hash_shift)			\
 		RUNTIME_CONST(ptr, dentry_hashtable)			\
-		RUNTIME_CONST(ptr, __dentry_cache)
+		RUNTIME_CONST(ptr, __dentry_cache)			\
+		RUNTIME_CONST(ptr, __names_cache)
 
 /* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
 #define KUNIT_TABLE()							\
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-01  8:32 [PATCH v2] fs: hide names_cache behind runtime const machinery Mateusz Guzik
@ 2025-12-01  8:51 ` Al Viro
  2025-12-02  2:31   ` Al Viro
  0 siblings, 1 reply; 9+ messages in thread
From: Al Viro @ 2025-12-01  8:51 UTC (permalink / raw)
  To: Mateusz Guzik; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Mon, Dec 01, 2025 at 09:32:26AM +0100, Mateusz Guzik wrote:
> s/names_cachep/names_cache/ for consistency with dentry cache.
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
> 
> v2:
> - rebased on top of work.filename-refcnt
> 
> ACHTUNG: there is a change queued for 6.19 merge window which treats
> dentry cache the same way:
> commit 21b561dab1406e63740ebe240c7b69f19e1bcf58
> Author: Mateusz Guzik <mjguzik@gmail.com>
> Date:   Wed Nov 5 16:36:22 2025 +0100
> 
>     fs: hide dentry_cache behind runtime const machinery
> 
> which would result in a merge conflict in vmlinux.lds.h. thus I
> cherry-picked before generating the diff to avoid the issue for later.

*shrug*
For now I'm working on top of v6.18; rebase to -rc1 will happen at the
end of window...

Anyway, not a problem; applied with obvious massage.  Will push tomorrow
once I sort the linearization out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-01  8:51 ` Al Viro
@ 2025-12-02  2:31   ` Al Viro
  2025-12-02  5:10     ` Mateusz Guzik
  0 siblings, 1 reply; 9+ messages in thread
From: Al Viro @ 2025-12-02  2:31 UTC (permalink / raw)
  To: Mateusz Guzik; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Mon, Dec 01, 2025 at 08:51:17AM +0000, Al Viro wrote:
> On Mon, Dec 01, 2025 at 09:32:26AM +0100, Mateusz Guzik wrote:
> > s/names_cachep/names_cache/ for consistency with dentry cache.
> > 
> > Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> > ---
> > 
> > v2:
> > - rebased on top of work.filename-refcnt
> > 
> > ACHTUNG: there is a change queued for 6.19 merge window which treats
> > dentry cache the same way:
> > commit 21b561dab1406e63740ebe240c7b69f19e1bcf58
> > Author: Mateusz Guzik <mjguzik@gmail.com>
> > Date:   Wed Nov 5 16:36:22 2025 +0100
> > 
> >     fs: hide dentry_cache behind runtime const machinery
> > 
> > which would result in a merge conflict in vmlinux.lds.h. thus I
> > cherry-picked before generating the diff to avoid the issue for later.
> 
> *shrug*
> For now I'm working on top of v6.18; rebase to -rc1 will happen at the
> end of window...
> 
> Anyway, not a problem; applied with obvious massage.  Will push tomorrow
> once I sort the linearization out.

	FWIW, I wonder if we would be better off with the following trick:
add
	struct kmem_cache *preallocated;
to struct kmem_cache_args.  Semantics: if the value is non-NULL, it must
point to an unitialized object of type struct kmem_cache; in that case
__kmem_cache_create_args() will use that object (and return its address
on success) instead of allocating one from kmem_cache.  kmem_cache_destroy()
should not be called for it.

It's very easy to do, AFAICS:
	1) non-NULL => have __kmem_cache_create_args() skip the __kmem_cache_alias()
path.
	2) non-NULL => have create_cache() zero what it points to and use that pointer
instead of calling kmem_cache_zalloc()
	3) non-NULL => skip kmem_cache_free() at create_cache() out_free_cache:

"Don't do kmem_cache_destroy() to those" might or might not be worth relaxing -
I hadn't looked into the lifetime issues for kmem_cache instances, no idea
how painful would that be; for core kernel caches it's not an issue, obviously.
For modules it is, but then runtime_constant machinery is not an option there
either.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-02  2:31   ` Al Viro
@ 2025-12-02  5:10     ` Mateusz Guzik
  2025-12-02  5:52       ` Al Viro
  0 siblings, 1 reply; 9+ messages in thread
From: Mateusz Guzik @ 2025-12-02  5:10 UTC (permalink / raw)
  To: Al Viro; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Tue, Dec 2, 2025 at 3:31 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>         FWIW, I wonder if we would be better off with the following trick:
> add
>         struct kmem_cache *preallocated;
> to struct kmem_cache_args.  Semantics: if the value is non-NULL, it must
> point to an unitialized object of type struct kmem_cache; in that case
> __kmem_cache_create_args() will use that object (and return its address
> on success) instead of allocating one from kmem_cache.  kmem_cache_destroy()
> should not be called for it.
>
> It's very easy to do, AFAICS:
>         1) non-NULL => have __kmem_cache_create_args() skip the __kmem_cache_alias()
> path.
>         2) non-NULL => have create_cache() zero what it points to and use that pointer
> instead of calling kmem_cache_zalloc()
>         3) non-NULL => skip kmem_cache_free() at create_cache() out_free_cache:
>
> "Don't do kmem_cache_destroy() to those" might or might not be worth relaxing -
> I hadn't looked into the lifetime issues for kmem_cache instances, no idea
> how painful would that be; for core kernel caches it's not an issue, obviously.
> For modules it is, but then runtime_constant machinery is not an option there
> either.

So IIUC whatever APIs aside, the crux of this idea is to have
kmem_cache objs defined instead of having pointers to them, as in:
-struct kmem_cache *names_cachep __ro_after_init;
+struct kmem_cache names_cachep __ro_after_init;

I thought about doing it that way prior to runtime const machinery,
but given that said machinery exists I don't know if that's
justifiable.

To elaborate, while it apparently was created as a hack and does not
work for modules, it does not have to be that way and I would argue it
should be patched up to a fully-fleshed out solution.

Everything marked __ro_after_init is eligible for being patched up to
avoid being accessed, including numerous kmem caches.

Apart from those an example frequently read var is
percpu_counter_batch, which for vfs comes into play very time a new
file obj is allocated. The thing is also used by some of the
filesystems.

So if one was to pretend for a minute runtime-const *does* work for
modules and there are no header mess issues and usage is popping up
everywhere, is there a reason to handle kmem differently?

Both with your idea and the runtime thing extra changes would be
needed. in your case the thing at hand is no longer a pointer and all
consumers of a given cache need to get adjusted. If instead one went
the runtime route, some macros could be added for syntactic sugar to
provide the relevant accessor + init, which should be very easy to do
by wrapping existing code.

So I would vote against your idea, but it's the call of the mm folk.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-02  5:10     ` Mateusz Guzik
@ 2025-12-02  5:52       ` Al Viro
  2025-12-02  6:18         ` Mateusz Guzik
  2025-12-02  6:20         ` Al Viro
  0 siblings, 2 replies; 9+ messages in thread
From: Al Viro @ 2025-12-02  5:52 UTC (permalink / raw)
  To: Mateusz Guzik; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Tue, Dec 02, 2025 at 06:10:36AM +0100, Mateusz Guzik wrote:

> So IIUC whatever APIs aside, the crux of this idea is to have
> kmem_cache objs defined instead of having pointers to them, as in:
> -struct kmem_cache *names_cachep __ro_after_init;
> +struct kmem_cache names_cachep __ro_after_init;

Huh?  __ro_after_init will break instantly - the contents changes with
each allocation, after all.  What I want is
static struct kmem_cache_store names_cache;

As for the many places to modify...

fs/file.c:390:  newf = kmem_cache_alloc(files_cachep, GFP_KERNEL);
fs/file.c:422:                  kmem_cache_free(files_cachep, newf);
fs/file.c:514:          kmem_cache_free(files_cachep, files);
include/linux/fdtable.h:116:extern struct kmem_cache *files_cachep;
kernel/fork.c:429:struct kmem_cache *files_cachep;
kernel/fork.c:2987:     files_cachep = kmem_cache_create("files_cache",
samples/kmemleak/kmemleak-test.c:52:    pr_info("kmem_cache_alloc(files_cachep) = 0x%px\n",
samples/kmemleak/kmemleak-test.c:53:            kmem_cache_alloc(files_cachep, GFP_KERNEL));
samples/kmemleak/kmemleak-test.c:54:    pr_info("kmem_cache_alloc(files_cachep) = 0x%px\n",
samples/kmemleak/kmemleak-test.c:55:            kmem_cache_alloc(files_cachep, GFP_KERNEL));

I would argue for making it static in fs/file.c, where we have the grand
total of 3 places using the sucker, between two functions.

dentry_cache:
fs/dcache.c:345:        kmem_cache_free(dentry_cache, dentry); 
fs/dcache.c:352:        kmem_cache_free(dentry_cache, dentry);
fs/dcache.c:1690:       dentry = kmem_cache_alloc_lru(dentry_cache, &sb->s_dentry_lru,
fs/dcache.c:1711:                       kmem_cache_free(dentry_cache, dentry); 
fs/dcache.c:1748:                       kmem_cache_free(dentry_cache, dentry);

5 lines, between 3 functions (__d_free(), __d_free_external(), __d_allock()).

mnt_cache:
fs/namespace.c:293:     struct mount *mnt = kmem_cache_zalloc(mnt_cache, GFP_KERNEL);
fs/namespace.c:342:     kmem_cache_free(mnt_cache, mnt);
fs/namespace.c:737:     kmem_cache_free(mnt_cache, mnt);

3 lines, alloc_vfsmnt() and free_vfsmnt()

sock_inode_cachep:
net/socket.c:322:       ei = alloc_inode_sb(sb, sock_inode_cachep, GFP_KERNEL);
net/socket.c:343:       kmem_cache_free(sock_inode_cachep, ei);

2 lines, sock_alloc_inode() and sock_free_inode() (sockets are coallocated with
inodes).

struct filename: two lines after that series.

task_struct_cachep:
kernel/fork.c:184:      return kmem_cache_alloc_node(task_struct_cachep, GFP_KERNEL, node);
kernel/fork.c:189:      kmem_cache_free(task_struct_cachep, tsk);

and so it goes; that's the sane pattern - you want few places where objects of given
type are allocated and freed, so that tracing the callchains would be feasible.
names_cachep used to be shitty in that respect, what with its abuse by weird __getname()
callers.  It's not the common situation, thankfully.

The delicate part is headers, indeed - we don't want to expose struct kmem_cache guts
anywhere outside of mm/*, and not the entire mm/* either.  But that's not hard to
deal with - see include/generate/bounds.h, include/generate/rq-offsets.h, etc.
Exact same technics can be used to get sizeof(struct kmem_cache) calculated and
put into generated header.  Then we get something like struct kmem_cache_store with
the right size and alignment, and _that_ would be what the variables would be.
With static inline struct kmem_cache *to_kmem_cache(struct kmem_cache_store *)
returning a cast and e.g.

static inline void free_filename(struct __filename *p)
{
        kmem_cache_free(to_kmem_cache(&names_cache), p);
}

as an example of use.

Anyway, for now I've applied your patch pretty much as-is; conversion of the
sort described above can be done afterwards just fine.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-02  5:52       ` Al Viro
@ 2025-12-02  6:18         ` Mateusz Guzik
  2025-12-02  6:32           ` Al Viro
  2025-12-02  6:20         ` Al Viro
  1 sibling, 1 reply; 9+ messages in thread
From: Mateusz Guzik @ 2025-12-02  6:18 UTC (permalink / raw)
  To: Al Viro; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Tue, Dec 2, 2025 at 6:52 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Dec 02, 2025 at 06:10:36AM +0100, Mateusz Guzik wrote:
>
> > So IIUC whatever APIs aside, the crux of this idea is to have
> > kmem_cache objs defined instead of having pointers to them, as in:
> > -struct kmem_cache *names_cachep __ro_after_init;
> > +struct kmem_cache names_cachep __ro_after_init;
>
> Huh?  __ro_after_init will break instantly - the contents changes with
> each allocation, after all.  What I want is
> static struct kmem_cache_store names_cache;
>

c'mon man, I copy pasted the existing line and removed the asterisk to
de-pointer it to make for illustrative purposes. You went straight to
description how to make your idea happen, so I wanted to make sure we
are on the same page on what it is.

> As for the many places to modify...
>
> fs/file.c:390:  newf = kmem_cache_alloc(files_cachep, GFP_KERNEL);
> fs/file.c:422:                  kmem_cache_free(files_cachep, newf);
> fs/file.c:514:          kmem_cache_free(files_cachep, files);
> include/linux/fdtable.h:116:extern struct kmem_cache *files_cachep;
> kernel/fork.c:429:struct kmem_cache *files_cachep;
> kernel/fork.c:2987:     files_cachep = kmem_cache_create("files_cache",
> samples/kmemleak/kmemleak-test.c:52:    pr_info("kmem_cache_alloc(files_cachep) = 0x%px\n",
> samples/kmemleak/kmemleak-test.c:53:            kmem_cache_alloc(files_cachep, GFP_KERNEL));
> samples/kmemleak/kmemleak-test.c:54:    pr_info("kmem_cache_alloc(files_cachep) = 0x%px\n",
> samples/kmemleak/kmemleak-test.c:55:            kmem_cache_alloc(files_cachep, GFP_KERNEL));
>
> I would argue for making it static in fs/file.c, where we have the grand
> total of 3 places using the sucker, between two functions.
>

The claim was not that your idea results in insurmountable churn. The
claim was *both* your idea and runtime const require churn on per kmem
cache basis. Then the question is if one is going to churn it
regardless, why this way over runtime const. I do think the runtime
thing is a little bit less churn and less work on the mm side to get
it going, but then the runtime thing *itself* needs productizing
(which I'm not signing up to do).

Per the previous e-mail I don't have a strong opinion myself and it is
the mm folk who need either idea sold to anyway.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-02  5:52       ` Al Viro
  2025-12-02  6:18         ` Mateusz Guzik
@ 2025-12-02  6:20         ` Al Viro
  1 sibling, 0 replies; 9+ messages in thread
From: Al Viro @ 2025-12-02  6:20 UTC (permalink / raw)
  To: Mateusz Guzik; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Tue, Dec 02, 2025 at 05:52:58AM +0000, Al Viro wrote:

> The delicate part is headers, indeed - we don't want to expose struct kmem_cache guts
> anywhere outside of mm/*, and not the entire mm/* either.  But that's not hard to
> deal with - see include/generate/bounds.h, include/generate/rq-offsets.h, etc.
> Exact same technics can be used to get sizeof(struct kmem_cache) calculated and
> put into generated header.  Then we get something like struct kmem_cache_store with
> the right size and alignment, and _that_ would be what the variables would be.
> With static inline struct kmem_cache *to_kmem_cache(struct kmem_cache_store *)
> returning a cast and e.g.
> 
> static inline void free_filename(struct __filename *p)
> {
>         kmem_cache_free(to_kmem_cache(&names_cache), p);
> }
> 
> as an example of use.
> 
> Anyway, for now I've applied your patch pretty much as-is; conversion of the
> sort described above can be done afterwards just fine.
> 

FWIW, the Kbuild side of that would be like this - not a lot of magic there:

diff --git a/Kbuild b/Kbuild
index 13324b4bbe23..eb985a6614eb 100644
--- a/Kbuild
+++ b/Kbuild
@@ -45,13 +45,24 @@ kernel/sched/rq-offsets.s: $(offsets-file)
 $(rq-offsets-file): kernel/sched/rq-offsets.s FORCE
 	$(call filechk,offsets,__RQ_OFFSETS_H__)
 
+# generate kmem_cache_size.h
+
+kmem_cache_size-file := include/generated/kmem_cache_size.h
+
+targets += mm/kmem_cache_size.s
+
+mm/kmem_cache_size.s: $(rq-offsets-file)
+
+$(kmem_cache_size-file): mm/kmem_cache_size.s FORCE
+	$(call filechk,offsets,__KMEM_CACHE_SIZE_H__)
+
 # Check for missing system calls
 
 quiet_cmd_syscalls = CALL    $<
       cmd_syscalls = $(CONFIG_SHELL) $< $(CC) $(c_flags) $(missing_syscalls_flags)
 
 PHONY += missing-syscalls
-missing-syscalls: scripts/checksyscalls.sh $(rq-offsets-file)
+missing-syscalls: scripts/checksyscalls.sh $(kmem_cache_size-file)
 	$(call cmd,syscalls)
 
 # Check the manual modification of atomic headers
diff --git a/include/linux/kmem_cache_store.h b/include/linux/kmem_cache_store.h
new file mode 100644
index 000000000000..4bd21480d3cf
--- /dev/null
+++ b/include/linux/kmem_cache_store.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_KMEM_CACHE_STORE_H
+#define __LINUX_KMEM_CACHE_STORE_H
+
+#include <generated/kmem_cache_size.h>
+
+/* same size and alignment as struct kmem_cache */
+struct kmem_cache_store {
+	unsigned char opaque[KMEM_CACHE_SIZE];
+} __attribute__((__aligned__(KMEM_CACHE_ALIGN)));
+
+struct kmem_cache;
+
+static inline struct kmem_cache *to_kmem_cache(struct kmem_cache_store *p)
+{
+	return (struct kmem_cache *)p;
+}
+#endif
diff --git a/mm/kmem_cache_size.c b/mm/kmem_cache_size.c
new file mode 100644
index 000000000000..52395b225aa1
--- /dev/null
+++ b/mm/kmem_cache_size.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generate definitions needed by the preprocessor.
+ * This code generates raw asm output which is post-processed
+ * to extract and format the required data.
+ */
+
+#define __GENERATING_KMEM_CACHE_SIZE_H
+/* Include headers that define the enum constants of interest */
+#include <linux/kbuild.h>
+#include "slab.h"
+
+int main(void)
+{
+	/* The enum constants to put into include/generated/bounds.h */
+	DEFINE(KMEM_CACHE_SIZE, sizeof(struct kmem_cache));
+	DEFINE(KMEM_CACHE_ALIGN, __alignof(struct kmem_cache));
+	/* End of constants */
+
+	return 0;
+}

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-02  6:18         ` Mateusz Guzik
@ 2025-12-02  6:32           ` Al Viro
  2025-12-02  7:21             ` Al Viro
  0 siblings, 1 reply; 9+ messages in thread
From: Al Viro @ 2025-12-02  6:32 UTC (permalink / raw)
  To: Mateusz Guzik; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Tue, Dec 02, 2025 at 07:18:16AM +0100, Mateusz Guzik wrote:

> The claim was not that your idea results in insurmountable churn. The
> claim was *both* your idea and runtime const require churn on per kmem
> cache basis. Then the question is if one is going to churn it
> regardless, why this way over runtime const. I do think the runtime
> thing is a little bit less churn and less work on the mm side to get
> it going, but then the runtime thing *itself* needs productizing
> (which I'm not signing up to do).

Umm...  runtime thing is lovely for shifts, but for pointers it's
going to be a headache on a bunch of architectures; for something
like dentry_hashtable it's either that or the cost of dereference,
but for kmem_cache I'd try it - if architecture has a good way for
"load a 64bit constant into a register staying within I$", I'd
expect the code generated for &global_variable to be not worse than
that, after all.

Churn is pretty much negligible in case of core kernel caches either
way.

As for the amount of churn in mm/*...  Turns out to be fairly minor;
kmem_cache_args allows to propagate it without any calling convention
changes.

I'll post when I get it to reasonable shape - so far it looks easy...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] fs: hide names_cache behind runtime const machinery
  2025-12-02  6:32           ` Al Viro
@ 2025-12-02  7:21             ` Al Viro
  0 siblings, 0 replies; 9+ messages in thread
From: Al Viro @ 2025-12-02  7:21 UTC (permalink / raw)
  To: Mateusz Guzik; +Cc: brauner, jack, linux-kernel, linux-fsdevel

On Tue, Dec 02, 2025 at 06:32:28AM +0000, Al Viro wrote:
> On Tue, Dec 02, 2025 at 07:18:16AM +0100, Mateusz Guzik wrote:
> 
> > The claim was not that your idea results in insurmountable churn. The
> > claim was *both* your idea and runtime const require churn on per kmem
> > cache basis. Then the question is if one is going to churn it
> > regardless, why this way over runtime const. I do think the runtime
> > thing is a little bit less churn and less work on the mm side to get
> > it going, but then the runtime thing *itself* needs productizing
> > (which I'm not signing up to do).
> 
> Umm...  runtime thing is lovely for shifts, but for pointers it's
> going to be a headache on a bunch of architectures; for something
> like dentry_hashtable it's either that or the cost of dereference,
> but for kmem_cache I'd try it - if architecture has a good way for
> "load a 64bit constant into a register staying within I$", I'd
> expect the code generated for &global_variable to be not worse than
> that, after all.
> 
> Churn is pretty much negligible in case of core kernel caches either
> way.
> 
> As for the amount of churn in mm/*...  Turns out to be fairly minor;
> kmem_cache_args allows to propagate it without any calling convention
> changes.
> 
> I'll post when I get it to reasonable shape - so far it looks easy...

OK, I'm going to grab some sleep; current (completely untested) delta
below, with conversion of mnt_cache as an example of use.

Uses of to_kmem_cache can be reduced with some use of _Generic
for kmem_cache_...alloc() and kmem_cache_free().  Even as it is,
the churn in fs/namespace.c is pretty minor...

Anyway, this is an intermediate variant:

diff --git a/Kbuild b/Kbuild
index 13324b4bbe23..eb985a6614eb 100644
--- a/Kbuild
+++ b/Kbuild
@@ -45,13 +45,24 @@ kernel/sched/rq-offsets.s: $(offsets-file)
 $(rq-offsets-file): kernel/sched/rq-offsets.s FORCE
 	$(call filechk,offsets,__RQ_OFFSETS_H__)
 
+# generate kmem_cache_size.h
+
+kmem_cache_size-file := include/generated/kmem_cache_size.h
+
+targets += mm/kmem_cache_size.s
+
+mm/kmem_cache_size.s: $(rq-offsets-file)
+
+$(kmem_cache_size-file): mm/kmem_cache_size.s FORCE
+	$(call filechk,offsets,__KMEM_CACHE_SIZE_H__)
+
 # Check for missing system calls
 
 quiet_cmd_syscalls = CALL    $<
       cmd_syscalls = $(CONFIG_SHELL) $< $(CC) $(c_flags) $(missing_syscalls_flags)
 
 PHONY += missing-syscalls
-missing-syscalls: scripts/checksyscalls.sh $(rq-offsets-file)
+missing-syscalls: scripts/checksyscalls.sh $(kmem_cache_size-file)
 	$(call cmd,syscalls)
 
 # Check the manual modification of atomic headers
diff --git a/fs/namespace.c b/fs/namespace.c
index d766e08e0736..08c7870de413 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -34,6 +34,7 @@
 #include <linux/mnt_idmapping.h>
 #include <linux/pidfs.h>
 #include <linux/nstree.h>
+#include <linux/kmem_cache_static.h>
 
 #include "pnode.h"
 #include "internal.h"
@@ -85,7 +86,7 @@ static u64 mnt_id_ctr = MNT_UNIQUE_ID_OFFSET;
 
 static struct hlist_head *mount_hashtable __ro_after_init;
 static struct hlist_head *mountpoint_hashtable __ro_after_init;
-static struct kmem_cache *mnt_cache __ro_after_init;
+static struct kmem_cache_store mnt_cache;
 static DECLARE_RWSEM(namespace_sem);
 static HLIST_HEAD(unmounted);	/* protected by namespace_sem */
 static LIST_HEAD(ex_mountpoints); /* protected by namespace_sem */
@@ -290,7 +291,8 @@ int mnt_get_count(struct mount *mnt)
 
 static struct mount *alloc_vfsmnt(const char *name)
 {
-	struct mount *mnt = kmem_cache_zalloc(mnt_cache, GFP_KERNEL);
+	struct mount *mnt = kmem_cache_zalloc(to_kmem_cache(&mnt_cache),
+					      GFP_KERNEL);
 	if (mnt) {
 		int err;
 
@@ -339,7 +341,7 @@ static struct mount *alloc_vfsmnt(const char *name)
 out_free_id:
 	mnt_free_id(mnt);
 out_free_cache:
-	kmem_cache_free(mnt_cache, mnt);
+	kmem_cache_free(to_kmem_cache(&mnt_cache), mnt);
 	return NULL;
 }
 
@@ -734,7 +736,7 @@ static void free_vfsmnt(struct mount *mnt)
 #ifdef CONFIG_SMP
 	free_percpu(mnt->mnt_pcp);
 #endif
-	kmem_cache_free(mnt_cache, mnt);
+	kmem_cache_free(to_kmem_cache(&mnt_cache), mnt);
 }
 
 static void delayed_free_vfsmnt(struct rcu_head *head)
@@ -6013,8 +6015,9 @@ void __init mnt_init(void)
 {
 	int err;
 
-	mnt_cache = kmem_cache_create("mnt_cache", sizeof(struct mount),
-			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
+	kmem_cache_setup("mnt_cache", sizeof(struct mount),
+			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT,
+			NULL, &mnt_cache);
 
 	mount_hashtable = alloc_large_system_hash("Mount-cache",
 				sizeof(struct hlist_head),
diff --git a/include/linux/kmem_cache_static.h b/include/linux/kmem_cache_static.h
new file mode 100644
index 000000000000..f007c3bf3e88
--- /dev/null
+++ b/include/linux/kmem_cache_static.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_KMEM_CACHE_STATIC_H
+#define __LINUX_KMEM_CACHE_STATIC_H
+
+#include <generated/kmem_cache_size.h>
+#include <linux/slab.h>
+
+/* same size and alignment as struct kmem_cache */
+struct kmem_cache_store {
+	unsigned char opaque[KMEM_CACHE_SIZE];
+} __attribute__((__aligned__(KMEM_CACHE_ALIGN)));
+
+struct kmem_cache;
+
+static inline struct kmem_cache *to_kmem_cache(struct kmem_cache_store *p)
+{
+	return (struct kmem_cache *)p;
+}
+
+static inline int
+kmem_cache_setup(const char *name, unsigned int size, unsigned int align,
+		 slab_flags_t flags, void (*ctor)(void *),
+		 struct kmem_cache_store *s)
+{
+	struct kmem_cache *res;
+
+	res = __kmem_cache_create_args(name, size,
+					&(struct kmem_cache_args){
+						.align	= align,
+						.ctor	= ctor,
+						.preallocated = s},
+					flags);
+	return PTR_ERR_OR_ZERO(res);
+}
+
+#endif
diff --git a/include/linux/slab.h b/include/linux/slab.h
index cf443f064a66..a016aa817139 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -266,6 +266,8 @@ struct mem_cgroup;
  */
 bool slab_is_available(void);
 
+struct kmem_cache_store;
+
 /**
  * struct kmem_cache_args - Less common arguments for kmem_cache_create()
  *
@@ -366,6 +368,7 @@ struct kmem_cache_args {
 	 * %0 means no sheaves will be created.
 	 */
 	unsigned int sheaf_capacity;
+	struct kmem_cache_store *preallocated;
 };
 
 struct kmem_cache *__kmem_cache_create_args(const char *name,
diff --git a/mm/kmem_cache_size.c b/mm/kmem_cache_size.c
new file mode 100644
index 000000000000..52395b225aa1
--- /dev/null
+++ b/mm/kmem_cache_size.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generate definitions needed by the preprocessor.
+ * This code generates raw asm output which is post-processed
+ * to extract and format the required data.
+ */
+
+#define __GENERATING_KMEM_CACHE_SIZE_H
+/* Include headers that define the enum constants of interest */
+#include <linux/kbuild.h>
+#include "slab.h"
+
+int main(void)
+{
+	/* The enum constants to put into include/generated/bounds.h */
+	DEFINE(KMEM_CACHE_SIZE, sizeof(struct kmem_cache));
+	DEFINE(KMEM_CACHE_ALIGN, __alignof(struct kmem_cache));
+	/* End of constants */
+
+	return 0;
+}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 932d13ada36c..e48775475097 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -29,6 +29,7 @@
 #include <linux/memcontrol.h>
 #include <linux/stackdepot.h>
 #include <trace/events/rcu.h>
+#include <linux/kmem_cache_static.h>
 
 #include "../kernel/rcu/rcu.h"
 #include "internal.h"
@@ -224,21 +225,21 @@ static struct kmem_cache *create_cache(const char *name,
 				       struct kmem_cache_args *args,
 				       slab_flags_t flags)
 {
-	struct kmem_cache *s;
+	struct kmem_cache *s = to_kmem_cache(args->preallocated);
 	int err;
 
 	/* If a custom freelist pointer is requested make sure it's sane. */
-	err = -EINVAL;
 	if (args->use_freeptr_offset &&
 	    (args->freeptr_offset >= object_size ||
 	     !(flags & SLAB_TYPESAFE_BY_RCU) ||
 	     !IS_ALIGNED(args->freeptr_offset, __alignof__(freeptr_t))))
-		goto out;
+		return ERR_PTR(-EINVAL);
 
-	err = -ENOMEM;
-	s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
-	if (!s)
-		goto out;
+	if (!s) {
+		s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
+		if (!s)
+			return ERR_PTR(-ENOMEM);
+	}
 	err = do_kmem_cache_create(s, name, object_size, args, flags);
 	if (err)
 		goto out_free_cache;
@@ -248,8 +249,8 @@ static struct kmem_cache *create_cache(const char *name,
 	return s;
 
 out_free_cache:
-	kmem_cache_free(kmem_cache, s);
-out:
+	if (!args->preallocated)
+		kmem_cache_free(kmem_cache, s);
 	return ERR_PTR(err);
 }
 
@@ -324,7 +325,7 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 		    object_size - args->usersize < args->useroffset))
 		args->usersize = args->useroffset = 0;
 
-	if (!args->usersize && !args->sheaf_capacity)
+	if (!args->usersize && !args->sheaf_capacity && !args->preallocated)
 		s = __kmem_cache_alias(name, object_size, args->align, flags,
 				       args->ctor);
 	if (s)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-12-02  7:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-01  8:32 [PATCH v2] fs: hide names_cache behind runtime const machinery Mateusz Guzik
2025-12-01  8:51 ` Al Viro
2025-12-02  2:31   ` Al Viro
2025-12-02  5:10     ` Mateusz Guzik
2025-12-02  5:52       ` Al Viro
2025-12-02  6:18         ` Mateusz Guzik
2025-12-02  6:32           ` Al Viro
2025-12-02  7:21             ` Al Viro
2025-12-02  6:20         ` Al Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).