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.
