linux-kernel - Re: [RFC PATCH v2 0/6] slub: Delay freezing of CPU partial slabs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dccaf41a-96d1-5c37-ccf2-8971d68f8a23@suse.cz>
Date:   Tue, 24 Oct 2023 10:19:12 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     "Christoph Lameter (Ampere)" <cl@...two.org>
Cc:     chengming.zhou@...ux.dev, penberg@...nel.org, rientjes@...gle.com,
        iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        roman.gushchin@...ux.dev, 42.hyeyoo@...il.com, willy@...radead.org,
        pcc@...gle.com, tytso@....edu, maz@...nel.org,
        ruansy.fnst@...itsu.com, vishal.moola@...il.com,
        lrh2000@....edu.cn, hughd@...gle.com, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, Chengming Zhou <zhouchengming@...edance.com>
Subject: Re: [RFC PATCH v2 0/6] slub: Delay freezing of CPU partial slabs

On 10/23/23 23:05, Christoph Lameter (Ampere) wrote:
> On Mon, 23 Oct 2023, Vlastimil Babka wrote:
> 
>>> For much of the frozen handling we must be holding the node list lock
>>> anyways in order to add/remove from the list. So we already have a lock
>>> that could be used to protect flag operations.
>>
>> I can see the following differences between the traditional frozen bit and
>> the new flag:
>>
>> frozen bit advantage:
>> - __slab_free() on an already-frozen slab can ignore list operations and
>> list_lock completely
>>
>> frozen bit disadvantage:
>> - acquire_slab() trying to do cmpxchg_double() under list_lock (see commit
>> 9b1ea29bc0d7)
> 
> 
> Ok so a slab is frozen if either of those conditions are met. That gets a 
> bit complicated to test for. Can we just get away with the 
> slab_node_partial flag?

Might be worth trying, but I'd try only as a next separate step. I think
freezing the slab that becomes cpu slab (not partial cpu) still has benefits
and no extra cost as that's when we're doing the cmpxchg_double anyway. And
the complicated tests are confined to __slab_free() and it's not *that* bad
IMHO, one condition checks for was_frozen, another for slab_test_node_partial().

> The advantage with the frozen state is that it can be changed with a 
> cmpxchg together with some other values (list pointer, counter) that need 
> updating at free and allocation.

Exactly, but for taking a slab off the node partial list we don't need to
deal with those, so that's where it makes sense to delay the frozen bit
handling.

> But frozen updates are rarer so maybe its worth to completely drop the 
> frozen bit. If both need to be updates then we would have two atomic ops. 
> One is the cmpxchg and the other the operation on the page flag.

The flag update doesn't even have to be atomic as it's only done under
list_lock.