Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <dccaf41a-96d1-5c37-ccf2-8971d68f8a23@suse.cz>
Date:   Tue, 24 Oct 2023 10:19:12 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [RFC PATCH v2 0/6] slub: Delay freezing of CPU partial slabs
To:     "Christoph Lameter (Ampere)" <cl@gentwo.org>
Cc:     chengming.zhou@linux.dev, penberg@kernel.org, rientjes@google.com,
        iamjoonsoo.kim@lge.com, akpm@linux-foundation.org,
        roman.gushchin@linux.dev, 42.hyeyoo@gmail.com, willy@infradead.org,
        pcc@google.com, tytso@mit.edu, maz@kernel.org,
        ruansy.fnst@fujitsu.com, vishal.moola@gmail.com,
        lrh2000@pku.edu.cn, hughd@google.com, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, Chengming Zhou <zhouchengming@bytedance.com>
References: <20231021144317.3400916-1-chengming.zhou@linux.dev>
 <4134b039-fa99-70cd-3486-3d0c7632e4a3@suse.cz>
 <fcb9b7f0-fb21-cce8-d452-766a5cc73d4a@gentwo.org>
 <ba3d6ac7-6900-3e8d-46b5-8302ca61f8ef@suse.cz>
 <c6f12967-e7bc-bf36-9c6b-0111dea1f0de@gentwo.org>
Content-Language: en-US
From:   Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <c6f12967-e7bc-bf36-9c6b-0111dea1f0de@gentwo.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.