| 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: <28e6827e-f689-45d9-b2b5-804a8aafad2e@suse.cz>
Date: Fri, 9 Jan 2026 09:16:37 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Chris Mason <clm@...a.com>, Roman Gushchin <roman.gushchin@...ux.dev>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Christoph Lameter <cl@...two.org>, David Rientjes <rientjes@...gle.com>,
Harry Yoo <harry.yoo@...cle.com>, 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,
Petr Tesarik <ptesarik@...e.com>, "Paul E . McKenney" <paulmck@...nel.org>
Subject: Re: [PATCH RFC 10/19] slab: remove cpu (partial) slabs usage from
allocation paths
On 10/24/25 16:29, Chris Mason wrote:
> On Thu, 23 Oct 2025 15:52:32 +0200 Vlastimil Babka <vbabka@...e.cz> 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 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.
>>
>
> Hi Vlastimil,
>
> We're trying out the AI patch review automation on the BPF commits and it had
> some questions about a few of these. Since the goal is to actually test the
> automation, I'm putting them in unedited, but I did try and make sure they
> were valid before sending.
>
> This one seems pretty unlikely but still worth fixing:
>
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index e2b052657d11..bd67336e7c1f 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>
> [ ... ]
>
>> @@ -3575,15 +3532,15 @@ static bool get_partial_node_bulk(struct kmem_cache *s,
>> }
>>
>> /*
>> - * Try to allocate a partial slab from a specific node.
>> + * Try to allocate object from a partial slab on a specific node.
>> */
>> -static struct slab *get_partial_node(struct kmem_cache *s,
>> - struct kmem_cache_node *n,
>> - struct partial_context *pc)
>> +static void *get_partial_node(struct kmem_cache *s,
>> + struct kmem_cache_node *n,
>> + struct partial_context *pc)
>> {
>> - struct slab *slab, *slab2, *partial = NULL;
>> + struct slab *slab, *slab2;
>> unsigned long flags;
>> - unsigned int partial_slabs = 0;
>> + void *object;
>>
>> /*
>> * Racy check. If we mistakenly see no partial slabs then we
>> @@ -3599,54 +3556,54 @@ 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) {
>> +
>> + unsigned long counters;
>> + struct slab new;
>> +
>> if (!pfmemalloc_match(slab, pc->flags))
>> continue;
>
> Can get_partial_node() return an uninitialized pointer? The variable
> 'object' is declared but never initialized. If all slabs in the partial
> list fail the pfmemalloc_match() check, the loop completes without
> setting 'object', then returns it at the end of the function.
>
> In the previous version, the equivalent 'partial' variable was explicitly
> initialized to NULL. When all slabs were skipped, NULL was returned.
By the way, there was another bug in this patch, causing a severe memory
leak, which the AI unfortunately didn't flag. Petr reported it during
performance testing and it took me more than a day to find it. Oh well :)
Wonder if things got better since then perhaps, and your or Roman's tools
would find it today? :)
>>
>> 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);
>> -
>> - if (!partial) {
>> - partial = slab;
>> - stat(s, ALLOC_FROM_PARTIAL);
>> -
>> - if ((slub_get_cpu_partial(s) == 0)) {
>> - break;
>> - }
>> - } else {
>> - put_cpu_partial(s, slab, 0);
>> - stat(s, CPU_PARTIAL_NODE);
>> -
>> - if (++partial_slabs > slub_get_cpu_partial(s) / 2) {
>> - break;
>> - }
>> - }
>> + /*
>> + * 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 {
>> + object = slab->freelist;
>> + counters = slab->counters;
>> + new.freelist = get_freepointer(s, object);
>> + new.counters = counters;
>> + new.inuse++;
>> + } while (!__slab_update_freelist(s, slab,
>> + object, counters,
>> + new.freelist, new.counters,
>> + "get_partial_node"));
>> +
>> + if (!new.freelist)
>> + remove_partial(n, slab);
>> }
>> spin_unlock_irqrestore(&n->list_lock, flags);
>> - return partial;
>> + return object;
>> }
>
> [ ... ]
>
>
>
Powered by blists - more mailing lists