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

[Vlastimil Babka <vbabka@suse.cz>]

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.