* [PATCH 1/4] slab: separate struct freelist_tid from kmem_cache_cpu
2025-11-07 13:51 [PATCH 0/4] slab: cmpxchg cleanups enabled by -fms-extensions Vlastimil Babka
@ 2025-11-07 13:51 ` Vlastimil Babka
2025-11-13 7:22 ` Harry Yoo
2025-11-07 13:51 ` [PATCH 2/4] slab: turn freelist_aba_t to a struct and fully define counters there Vlastimil Babka
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2025-11-07 13:51 UTC (permalink / raw)
To: linux-mm
Cc: Andrew Morton, Christoph Lameter, David Rientjes, Roman Gushchin,
Harry Yoo, linux-kernel, Vlastimil Babka
In kmem_cache_cpu we currently have a union of the freelist+tid pair
with freelist_aba_t, relying implicitly on the type compatibility with the
freelist+counters pair used in freelist_aba_t.
To allow further changes to freelist_aba_t, we can instead define a
separate struct freelist_tid (instead of a typedef, per the coding
style) for kmem_cache_cpu, as that affects only a single helper
__update_cpu_freelist_fast().
We can add the resulting struct freelist_tid to kmem_cache_cpu as
unnamed field thanks to -fms-extensions, so that freelist and tid fields
can still be accessed directly.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/slub.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 074abe8e79f8..5f6408c9e0fd 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -411,18 +411,22 @@ enum stat_item {
};
#ifndef CONFIG_SLUB_TINY
-/*
- * When changing the layout, make sure freelist and tid are still compatible
- * with this_cpu_cmpxchg_double() alignment requirements.
- */
-struct kmem_cache_cpu {
+struct freelist_tid {
union {
struct {
- void **freelist; /* Pointer to next available object */
+ void *freelist; /* Pointer to next available object */
unsigned long tid; /* Globally unique transaction id */
};
- freelist_aba_t freelist_tid;
+ freelist_full_t freelist_tid;
};
+};
+
+/*
+ * When changing the layout, make sure freelist and tid are still compatible
+ * with this_cpu_cmpxchg_double() alignment requirements.
+ */
+struct kmem_cache_cpu {
+ struct freelist_tid;
struct slab *slab; /* The slab from which we are allocating */
#ifdef CONFIG_SLUB_CPU_PARTIAL
struct slab *partial; /* Partially allocated slabs */
@@ -4367,11 +4371,11 @@ __update_cpu_freelist_fast(struct kmem_cache *s,
void *freelist_old, void *freelist_new,
unsigned long tid)
{
- freelist_aba_t old = { .freelist = freelist_old, .counter = tid };
- freelist_aba_t new = { .freelist = freelist_new, .counter = next_tid(tid) };
+ struct freelist_tid old = { .freelist = freelist_old, .tid = tid };
+ struct freelist_tid new = { .freelist = freelist_new, .tid = next_tid(tid) };
- return this_cpu_try_cmpxchg_freelist(s->cpu_slab->freelist_tid.full,
- &old.full, new.full);
+ return this_cpu_try_cmpxchg_freelist(s->cpu_slab->freelist_tid,
+ &old.freelist_tid, new.freelist_tid);
}
/*
--
2.51.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] slab: turn freelist_aba_t to a struct and fully define counters there
2025-11-07 13:51 [PATCH 0/4] slab: cmpxchg cleanups enabled by -fms-extensions Vlastimil Babka
2025-11-07 13:51 ` [PATCH 1/4] slab: separate struct freelist_tid from kmem_cache_cpu Vlastimil Babka
@ 2025-11-07 13:51 ` Vlastimil Babka
2025-11-13 7:34 ` Harry Yoo
2025-11-07 13:51 ` [PATCH 3/4] slab: use struct freelist_counters for local variables instead of struct slab Vlastimil Babka
2025-11-07 13:51 ` [PATCH 4/4] slab: use struct freelist_counters as parameters in relevant functions Vlastimil Babka
3 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2025-11-07 13:51 UTC (permalink / raw)
To: linux-mm
Cc: Andrew Morton, Christoph Lameter, David Rientjes, Roman Gushchin,
Harry Yoo, linux-kernel, Vlastimil Babka
In struct slab we currently have freelist and counters pair, where
counters itself is a union of unsigned long with a sub-struct of
several smaller fields. Then for the usage with double cmpxchg we have
freelist_aba_t that duplicates the definition of the freelist+counters
with implicitly the same layout as the full definition in struct slab.
Thanks to -fms-extension we can now move the full counters definition to
freelist_aba_t (while changing it to struct freelist_counters as a
typedef is unnecessary and discouraged) and replace the relevant part in
struct slab to an unnamed reference to it.
The immediate benefit is the removal of duplication and no longer
relying on the same layout implicitly. It also allows further cleanups
thanks to having the full definition of counters in struct
freelist_counters.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/slab.h | 52 ++++++++++++++++++++++++----------------------------
mm/slub.c | 8 +++++---
2 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/mm/slab.h b/mm/slab.h
index 078daecc7cf5..42627b87d50c 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -40,13 +40,29 @@ typedef u64 freelist_full_t;
* Freelist pointer and counter to cmpxchg together, avoids the typical ABA
* problems with cmpxchg of just a pointer.
*/
-typedef union {
- struct {
- void *freelist;
- unsigned long counter;
+struct freelist_counters {
+ union {
+ struct {
+ void *freelist;
+ union {
+ unsigned long counters;
+ struct {
+ unsigned inuse:16;
+ unsigned objects:15;
+ /*
+ * If slab debugging is enabled then the
+ * frozen bit can be reused to indicate
+ * that the slab was corrupted
+ */
+ unsigned frozen:1;
+ };
+ };
+ };
+#ifdef system_has_freelist_aba
+ freelist_full_t freelist_counters;
+#endif
};
- freelist_full_t full;
-} freelist_aba_t;
+};
/* Reuses the bits in struct page */
struct slab {
@@ -69,27 +85,7 @@ struct slab {
#endif
};
/* Double-word boundary */
- union {
- struct {
- void *freelist; /* first free object */
- union {
- unsigned long counters;
- struct {
- unsigned inuse:16;
- unsigned objects:15;
- /*
- * If slab debugging is enabled then the
- * frozen bit can be reused to indicate
- * that the slab was corrupted
- */
- unsigned frozen:1;
- };
- };
- };
-#ifdef system_has_freelist_aba
- freelist_aba_t freelist_counter;
-#endif
- };
+ struct freelist_counters;
};
struct rcu_head rcu_head;
};
@@ -114,7 +110,7 @@ SLAB_MATCH(_unused_slab_obj_exts, obj_exts);
#undef SLAB_MATCH
static_assert(sizeof(struct slab) <= sizeof(struct page));
#if defined(system_has_freelist_aba)
-static_assert(IS_ALIGNED(offsetof(struct slab, freelist), sizeof(freelist_aba_t)));
+static_assert(IS_ALIGNED(offsetof(struct slab, freelist), sizeof(struct freelist_counters)));
#endif
/**
diff --git a/mm/slub.c b/mm/slub.c
index 5f6408c9e0fd..8330e4f8b3b2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -764,10 +764,12 @@ __update_freelist_fast(struct slab *slab,
void *freelist_new, unsigned long counters_new)
{
#ifdef system_has_freelist_aba
- freelist_aba_t old = { .freelist = freelist_old, .counter = counters_old };
- freelist_aba_t new = { .freelist = freelist_new, .counter = counters_new };
+ struct freelist_counters old = { .freelist = freelist_old, .counters = counters_old };
+ struct freelist_counters new = { .freelist = freelist_new, .counters = counters_new };
- return try_cmpxchg_freelist(&slab->freelist_counter.full, &old.full, new.full);
+ return try_cmpxchg_freelist(&slab->freelist_counters,
+ &old.freelist_counters,
+ new.freelist_counters);
#else
return false;
#endif
--
2.51.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] slab: use struct freelist_counters as parameters in relevant functions
2025-11-07 13:51 [PATCH 0/4] slab: cmpxchg cleanups enabled by -fms-extensions Vlastimil Babka
` (2 preceding siblings ...)
2025-11-07 13:51 ` [PATCH 3/4] slab: use struct freelist_counters for local variables instead of struct slab Vlastimil Babka
@ 2025-11-07 13:51 ` Vlastimil Babka
2025-11-13 8:32 ` Harry Yoo
3 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2025-11-07 13:51 UTC (permalink / raw)
To: linux-mm
Cc: Andrew Morton, Christoph Lameter, David Rientjes, Roman Gushchin,
Harry Yoo, linux-kernel, Vlastimil Babka
In functions such as [__]slab_update_freelist() and
__slab_update_freelist_fast/slow() we pass old and new freelist and
counters as 4 separate parameters. The underlying
__update_freelist_fast() then constructs struct freelist_counters
variables for passing the full freelist+counter combinations to cmpxchg
double.
In most cases we actually start with struct freelist_counters variables,
but then pass the individual fields, only to construct new struct
freelist_counters variables. While it's all inlined and thus should be
efficient, we can simplify this code.
Thus replace the 4 parameters for individual fields with two
freelist_aba_t pointers wherever applicable. __update_freelist_fast()
can then pass them directly to try_cmpxchg_freelist().
The code is also more obvious as the pattern becomes unified such that
we set up "old" and "new" struct freelist_counters variables upfront as
we fully need them to be, and simply call [__]slab_update_freelist() on
them. Previously some of the "new" values would be hidden as one of the
many parameters and thus make it harder to figure out what the code
does.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/slub.c | 126 ++++++++++++++++++++++++++------------------------------------
1 file changed, 52 insertions(+), 74 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index a55e0af26ec7..ddd71f4937fa 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -759,34 +759,29 @@ static __always_inline void slab_unlock(struct slab *slab)
}
static inline bool
-__update_freelist_fast(struct slab *slab,
- void *freelist_old, unsigned long counters_old,
- void *freelist_new, unsigned long counters_new)
+__update_freelist_fast(struct slab *slab, struct freelist_counters *old,
+ struct freelist_counters *new)
{
#ifdef system_has_freelist_aba
- struct freelist_counters old = { .freelist = freelist_old, .counters = counters_old };
- struct freelist_counters new = { .freelist = freelist_new, .counters = counters_new };
-
return try_cmpxchg_freelist(&slab->freelist_counters,
- &old.freelist_counters,
- new.freelist_counters);
+ &old->freelist_counters,
+ new->freelist_counters);
#else
return false;
#endif
}
static inline bool
-__update_freelist_slow(struct slab *slab,
- void *freelist_old, unsigned long counters_old,
- void *freelist_new, unsigned long counters_new)
+__update_freelist_slow(struct slab *slab, struct freelist_counters *old,
+ struct freelist_counters *new)
{
bool ret = false;
slab_lock(slab);
- if (slab->freelist == freelist_old &&
- slab->counters == counters_old) {
- slab->freelist = freelist_new;
- slab->counters = counters_new;
+ if (slab->freelist == old->freelist &&
+ slab->counters == old->counters) {
+ slab->freelist = new->freelist;
+ slab->counters = new->counters;
ret = true;
}
slab_unlock(slab);
@@ -802,22 +797,18 @@ __update_freelist_slow(struct slab *slab,
* interrupt the operation.
*/
static inline bool __slab_update_freelist(struct kmem_cache *s, struct slab *slab,
- void *freelist_old, unsigned long counters_old,
- void *freelist_new, unsigned long counters_new,
- const char *n)
+ struct freelist_counters *old, struct freelist_counters *new, const char *n)
{
bool ret;
if (USE_LOCKLESS_FAST_PATH())
lockdep_assert_irqs_disabled();
- if (s->flags & __CMPXCHG_DOUBLE) {
- ret = __update_freelist_fast(slab, freelist_old, counters_old,
- freelist_new, counters_new);
- } else {
- ret = __update_freelist_slow(slab, freelist_old, counters_old,
- freelist_new, counters_new);
- }
+ if (s->flags & __CMPXCHG_DOUBLE)
+ ret = __update_freelist_fast(slab, old, new);
+ else
+ ret = __update_freelist_slow(slab, old, new);
+
if (likely(ret))
return true;
@@ -832,21 +823,17 @@ static inline bool __slab_update_freelist(struct kmem_cache *s, struct slab *sla
}
static inline bool slab_update_freelist(struct kmem_cache *s, struct slab *slab,
- void *freelist_old, unsigned long counters_old,
- void *freelist_new, unsigned long counters_new,
- const char *n)
+ struct freelist_counters *old, struct freelist_counters *new, const char *n)
{
bool ret;
if (s->flags & __CMPXCHG_DOUBLE) {
- ret = __update_freelist_fast(slab, freelist_old, counters_old,
- freelist_new, counters_new);
+ ret = __update_freelist_fast(slab, old, new);
} else {
unsigned long flags;
local_irq_save(flags);
- ret = __update_freelist_slow(slab, freelist_old, counters_old,
- freelist_new, counters_new);
+ ret = __update_freelist_slow(slab, old, new);
local_irq_restore(flags);
}
if (likely(ret))
@@ -3774,10 +3761,7 @@ static void deactivate_slab(struct kmem_cache *s, struct slab *slab,
} else {
new.freelist = old.freelist;
}
- } while (!slab_update_freelist(s, slab,
- old.freelist, old.counters,
- new.freelist, new.counters,
- "unfreezing slab"));
+ } while (!slab_update_freelist(s, slab, &old, &new, "unfreezing slab"));
/*
* Stage three: Manipulate the slab list based on the updated state.
@@ -4389,27 +4373,24 @@ __update_cpu_freelist_fast(struct kmem_cache *s,
*/
static inline void *get_freelist(struct kmem_cache *s, struct slab *slab)
{
- struct freelist_counters new;
- unsigned long counters;
- void *freelist;
+ struct freelist_counters old, new;
lockdep_assert_held(this_cpu_ptr(&s->cpu_slab->lock));
do {
- freelist = slab->freelist;
- counters = slab->counters;
+ old.freelist = slab->freelist;
+ old.counters = slab->counters;
- new.counters = counters;
+ new.freelist = NULL;
+ new.counters = old.counters;
- new.inuse = slab->objects;
- new.frozen = freelist != NULL;
+ new.inuse = old.objects;
+ new.frozen = old.freelist != NULL;
- } while (!__slab_update_freelist(s, slab,
- freelist, counters,
- NULL, new.counters,
- "get_freelist"));
- return freelist;
+ } while (!__slab_update_freelist(s, slab, &old, &new, "get_freelist"));
+
+ return old.freelist;
}
/*
@@ -4417,26 +4398,22 @@ static inline void *get_freelist(struct kmem_cache *s, struct slab *slab)
*/
static inline void *freeze_slab(struct kmem_cache *s, struct slab *slab)
{
- struct freelist_counters new;
- unsigned long counters;
- void *freelist;
+ struct freelist_counters old, new;
do {
- freelist = slab->freelist;
- counters = slab->counters;
+ old.freelist = slab->freelist;
+ old.counters = slab->counters;
- new.counters = counters;
+ new.freelist = NULL;
+ new.counters = old.counters;
VM_BUG_ON(new.frozen);
- new.inuse = slab->objects;
+ new.inuse = old.objects;
new.frozen = 1;
- } while (!slab_update_freelist(s, slab,
- freelist, counters,
- NULL, new.counters,
- "freeze_slab"));
+ } while (!slab_update_freelist(s, slab, &old, &new, "freeze_slab"));
- return freelist;
+ return old.freelist;
}
/*
@@ -5864,10 +5841,8 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab,
unsigned long addr)
{
- void *old_head;
bool was_frozen, was_full;
- struct freelist_counters new;
- unsigned long counters;
+ struct freelist_counters old, new;
struct kmem_cache_node *n = NULL;
unsigned long flags;
bool on_node_partial;
@@ -5891,13 +5866,19 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab,
spin_unlock_irqrestore(&n->list_lock, flags);
n = NULL;
}
- old_head = slab->freelist;
- counters = slab->counters;
- set_freepointer(s, tail, old_head);
- new.counters = counters;
- was_frozen = !!new.frozen;
- was_full = (old_head == NULL);
+
+ old.freelist = slab->freelist;
+ old.counters = slab->counters;
+
+ was_full = (old.freelist == NULL);
+ was_frozen = old.frozen;
+
+ set_freepointer(s, tail, old.freelist);
+
+ new.freelist = head;
+ new.counters = old.counters;
new.inuse -= cnt;
+
/*
* Might need to be taken off (due to becoming empty) or added
* to (due to not being full anymore) the partial list.
@@ -5926,10 +5907,7 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab,
}
}
- } while (!slab_update_freelist(s, slab,
- old_head, counters,
- head, new.counters,
- "__slab_free"));
+ } while (!slab_update_freelist(s, slab, &old, &new, "__slab_free"));
if (likely(!n)) {
--
2.51.1
^ permalink raw reply related [flat|nested] 10+ messages in thread