| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20251107-slab-fms-cleanup-v1-4-650b1491ac9e@suse.cz>
Date: Fri, 07 Nov 2025 14:51:26 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: linux-mm@...ck.org
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Christoph Lameter <cl@...two.org>, David Rientjes <rientjes@...gle.com>,
Roman Gushchin <roman.gushchin@...ux.dev>, Harry Yoo <harry.yoo@...cle.com>,
linux-kernel@...r.kernel.org, Vlastimil Babka <vbabka@...e.cz>
Subject: [PATCH 4/4] slab: use struct freelist_counters as parameters in
relevant functions
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@...e.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
Powered by blists - more mailing lists