[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aW8CoUkioJFywI4A@hyeyoo>
Date: Tue, 20 Jan 2026 13:20:49 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Petr Tesarik <ptesarik@...e.com>, Christoph Lameter <cl@...two.org>,
David Rientjes <rientjes@...gle.com>,
Roman Gushchin <roman.gushchin@...ux.dev>, Hao Li <hao.li@...ux.dev>,
Andrew Morton <akpm@...ux-foundation.org>,
Uladzislau Rezki <urezki@...il.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Suren Baghdasaryan <surenb@...gle.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Alexei Starovoitov <ast@...nel.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
bpf@...r.kernel.org, kasan-dev@...glegroups.com
Subject: Re: [PATCH v3 10/21] slab: remove cpu (partial) slabs usage from
allocation paths
On Fri, Jan 16, 2026 at 03:40:30PM +0100, Vlastimil Babka wrote:
> We now rely on sheaves as the percpu caching layer and can refill them
> directly from partial or newly allocated slabs. Start removing the cpu
> (partial) slabs code, first from allocation paths.
>
> This means that any allocation not satisfied from percpu sheaves will
> end up in ___slab_alloc(), where we remove the usage of cpu (partial)
> slabs, so it will only perform get_partial() or new_slab(). In the
> latter case we reuse alloc_from_new_slab() (when we don't use
> the debug/tiny alloc_single_from_new_slab() variant).
>
> In get_partial_node() we used to return a slab for freezing as the cpu
> slab and to refill the partial slab. Now we only want to return a single
> object and leave the slab on the list (unless it became full). We can't
> simply reuse alloc_single_from_partial() as that assumes freeing uses
> free_to_partial_list(). Instead we need to use __slab_update_freelist()
> to work properly against a racing __slab_free().
>
> The rest of the changes is removing functions that no longer have any
> callers.
>
> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
> ---
> mm/slub.c | 612 ++++++++------------------------------------------------------
> 1 file changed, 79 insertions(+), 533 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index dce80463f92c..698c0d940f06 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3607,54 +3564,55 @@ static struct slab *get_partial_node(struct kmem_cache *s,
> else if (!spin_trylock_irqsave(&n->list_lock, flags))
> return NULL;
> list_for_each_entry_safe(slab, slab2, &n->partial, slab_list) {
> +
> + struct freelist_counters old, new;
> +
> if (!pfmemalloc_match(slab, pc->flags))
> continue;
>
> if (IS_ENABLED(CONFIG_SLUB_TINY) || kmem_cache_debug(s)) {
> - void *object = alloc_single_from_partial(s, n, slab,
> + object = alloc_single_from_partial(s, n, slab,
> pc->orig_size);
> - if (object) {
> - partial = slab;
> - pc->object = object;
> + if (object)
> break;
> - }
> continue;
> }
>
> - remove_partial(n, slab);
> + /*
> + * get a single object from the slab. This might race against
> + * __slab_free(), which however has to take the list_lock if
> + * it's about to make the slab fully free.
> + */
> + do {
> + old.freelist = slab->freelist;
> + old.counters = slab->counters;
>
> - if (!partial) {
> - partial = slab;
> - stat(s, ALLOC_FROM_PARTIAL);
> + new.freelist = get_freepointer(s, old.freelist);
> + new.counters = old.counters;
> + new.inuse++;
>
> - if ((slub_get_cpu_partial(s) == 0)) {
> - break;
> - }
> - } else {
> - put_cpu_partial(s, slab, 0);
> - stat(s, CPU_PARTIAL_NODE);
> + } while (!__slab_update_freelist(s, slab, &old, &new, "get_partial_node"));
Hmm I was wondering if it would introduce an ABBA problem,
but it looks fine as allocations are serialized by n->list_lock.
> - if (++partial_slabs > slub_get_cpu_partial(s) / 2) {
> - break;
> - }
> - }
> + object = old.freelist;
> + if (!new.freelist)
> + remove_partial(n, slab);
> +
> + break;
> }
> spin_unlock_irqrestore(&n->list_lock, flags);
> - return partial;
> + return object;
> }
> @@ -4849,68 +4574,29 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
[...]
> + if (allow_spin)
> + goto new_objects;
>
> - stat(s, CPUSLAB_FLUSH);
> + /* This could cause an endless loop. Fail instead. */
> + return NULL;
>
> - goto retry_load_slab;
> - }
> - c->slab = slab;
> +success:
> + if (kmem_cache_debug_flags(s, SLAB_STORE_USER))
> + set_track(s, freelist, TRACK_ALLOC, addr, gfpflags);
Oh, it was gfpflags & ~(__GFP_DIRECT_RECLAIM) but clearing
__GFP_DIRECT_RECLAIM was removed because preemption isn't disabled
anymore.
Looks good to me,
Reviewed-by: Harry Yoo <harry.yoo@...cle.com>
>
> - goto load_freelist;
> + return freelist;
> }
> +
> /*
> * We disallow kprobes in ___slab_alloc() to prevent reentrance
> *
--
Cheers,
Harry / Hyeonggon
Powered by blists - more mailing lists