From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73EC0C4167B for ; Sun, 3 Dec 2023 10:15:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DB0D96B0310; Sun, 3 Dec 2023 05:15:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D60166B0311; Sun, 3 Dec 2023 05:15:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C4EFC6B0312; Sun, 3 Dec 2023 05:15:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B736C6B0310 for ; Sun, 3 Dec 2023 05:15:52 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 7F7B914012F for ; Sun, 3 Dec 2023 10:15:52 +0000 (UTC) X-FDA: 81525100944.19.6F223B9 Received: from out-183.mta1.migadu.com (out-183.mta1.migadu.com [95.215.58.183]) by imf22.hostedemail.com (Postfix) with ESMTP id 57C88C000F for ; Sun, 3 Dec 2023 10:15:50 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=S957iSFf; spf=pass (imf22.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.183 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701598550; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=G6CU/v4ke8mEGph34JAii61ix6sVUsEQddIUs3JPpFw=; b=zcgG+3zQ2eO1KMMV4lstQBK3QXsqJQhbBwBXAW6v9K4Bgec7OYeWP7lRMl+y7SrqYbbByp 7By1HhzgbWK7KTzIHX2C1VOItnAAy8Dhy35oj7aJ+JlbBJyQmhBoF5aTLWUTC7YajuYjVp CjjmNpmE5vdAQDYJ8NXzYibEvvwUiyE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701598550; a=rsa-sha256; cv=none; b=nBAHsRs6BN5On4ldsMND13OaJo9IfLguB5N6v+L81W1MmntbPnIWMJGn6KLAytGrkznpuq uINTGo//l41Odqg2cKo0j/jgWfPC7xtqF5rrN9uq92NphtmEfx+CJZgO+bEpMYdnooCMxf C4gRvw/X6O85xY2Cd1Y3EpC9zpPSs/Y= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=S957iSFf; spf=pass (imf22.hostedemail.com: domain of chengming.zhou@linux.dev designates 95.215.58.183 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Message-ID: <98763097-d05e-40cd-afe0-4df65083d104@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1701598548; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=G6CU/v4ke8mEGph34JAii61ix6sVUsEQddIUs3JPpFw=; b=S957iSFfCQ8IrigoSv6dwUjz59NuVIiOQ8tK4dqBhUk9XgA6L12UmA/QCnDQ9lu02XfSMs 1+2RqX48puqJ5qh8JgYknWp5HDrNRIIqW696EJ3/G+QLhCYxEG32ayoxYA/3V45+A7/rBU ybr/KVZgrKnybRbFq4SIvmzG4AEJjlg= Date: Sun, 3 Dec 2023 18:15:16 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v5 6/9] slub: Delay freezing of partial slabs To: Hyeonggon Yoo <42.hyeyoo@gmail.com>, vbabka@suse.cz Cc: cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, akpm@linux-foundation.org, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Chengming Zhou References: <20231102032330.1036151-1-chengming.zhou@linux.dev> <20231102032330.1036151-7-chengming.zhou@linux.dev> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Stat-Signature: nfxuhk5b8hnxsrc58jgkys6x46rr5j69 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 57C88C000F X-Rspam-User: X-HE-Tag: 1701598550-770711 X-HE-Meta: U2FsdGVkX1+poR/hYahA/9t8IU8CNeNzGSK5njd2FFL/SEp1qE4y7XpO+dDtw2uFb6JoCLxZvqFCbZRzSDBgjocA9V0eghxwBY+rYH3UJpEeU1d58XGdTXhoLcNE99TvNFEZFhlQxgPiET1wAuhVQGh/CunSTbfKoWIKffXg6iqXY/FGiA9X5NdH0cgl27pCAFVkvKbEjSY+v5C/t6Aw3EFVwh0F1nafH0HcMo0JVBDELKF5Tvc+ms627luHH5vbtSCN8cmFrWwBqUdyRiJ2GS831jyDqnOK2a2JcKeyvlUIghgjrdw4Q51D+ORFL+C0FUjC30whd5jwNOEsakt6wxkuGPGZBt2w+keyqODKQARI4JoWpuNtMLEOsuipCCNAX/dV9tbXBiHfEn2KG3PUViJ7Q3TRpUWycBc9DZEXGoYawEIb8RicJAmJd7gPWKIkAxBkF7WoCq5Hx+qcF/F8JyF1KQYUg/0dP+1qLq5T2iGKIuoggslk2tFzgOUfBFWTiyuY+jpQFNC5HorD3QPa6pk6fK6fKmj32zHAE86TfntdXzeao3MI32+u/UUyUMU4wx/fidqU8wPq9tViii5RoG6zdsPowPu3csFSEUG5HYUPeYhZo5kRuxLcU2gja316HpXFHIWkISgGV8dyV0IwsR5B5fro/ignbUI+H3WsprLjBvbvd1ZULg0e5It7IottplCZRYYX5uSDEYAQ5Vx9c2+4lXLMqzHiGf9NIsYuaHUhcOOc9v27eT9/KH2eXwgnUwNbLVqUb7jD7AjOfqY8I2+uqe7DCJqkyrGgdUOr97Ru6hYHbGHXIAeV0pA3tVP+d3lU0aZo5vxRkBv+h14jREVDwTfIlRIKEg+ONQJcb3XICH9ND9I6eHTQmbMzJhPhNe9ibUVyO9UsLHf0v2diOPhsb94nFUXe7qcdTDZzcXqru6agro21HzaNQX7gbWaa19SeI5hzWU5dEbQN5sA FBq38n1Y zJ8ZlOh6ZB4O2Sqmk7CMUgJdaHm81PRwieIq0oZ411nTQo0O35JbP5Cgw5myf1f43Nk8dwTmSUuHy98CHxp+62Qej6T/LEMN1b4Lp3MzD78gvm1QF1YngL5VwpS7I0WdTgScT/+jMcs+HpoFm6XmqIB8uJO1XEfLEwMvOVgtHF4ugHkfLi84bd8EHKByitSF2O9tv+phusKjWMH1QmwhyHGG+WGMTJ+JGCADVz2zpdY1ACtrP683OJuH2Eg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2023/12/3 14:53, Hyeonggon Yoo wrote: > On Thu, Nov 2, 2023 at 12:25 PM wrote: >> >> From: Chengming Zhou >> >> Now we will freeze slabs when moving them out of node partial list to >> cpu partial list, this method needs two cmpxchg_double operations: >> >> 1. freeze slab (acquire_slab()) under the node list_lock >> 2. get_freelist() when pick used in ___slab_alloc() >> >> Actually we don't need to freeze when moving slabs out of node partial >> list, we can delay freezing to when use slab freelist in ___slab_alloc(), >> so we can save one cmpxchg_double(). >> >> And there are other good points: >> - The moving of slabs between node partial list and cpu partial list >> becomes simpler, since we don't need to freeze or unfreeze at all. >> >> - The node list_lock contention would be less, since we don't need to >> freeze any slab under the node list_lock. >> >> We can achieve this because there is no concurrent path would manipulate >> the partial slab list except the __slab_free() path, which is now >> serialized by slab_test_node_partial() under the list_lock. >> >> Since the slab returned by get_partial() interfaces is not frozen anymore >> and no freelist is returned in the partial_context, so we need to use the >> introduced freeze_slab() to freeze it and get its freelist. >> >> Similarly, the slabs on the CPU partial list are not frozen anymore, >> we need to freeze_slab() on it before use. >> >> We can now delete acquire_slab() as it became unused. >> >> Signed-off-by: Chengming Zhou >> Reviewed-by: Vlastimil Babka >> Tested-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> >> --- >> mm/slub.c | 113 +++++++++++------------------------------------------- >> 1 file changed, 23 insertions(+), 90 deletions(-) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index edf567971679..bcb5b2c4e213 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -2234,51 +2234,6 @@ static void *alloc_single_from_new_slab(struct kmem_cache *s, >> return object; >> } >> >> -/* >> - * Remove slab from the partial list, freeze it and >> - * return the pointer to the freelist. >> - * >> - * Returns a list of objects or NULL if it fails. >> - */ >> -static inline void *acquire_slab(struct kmem_cache *s, >> - struct kmem_cache_node *n, struct slab *slab, >> - int mode) > > Nit: alloc_single_from_partial()'s comment still refers to acquire_slab(). > Ah, right! It should be changed to remove_partial(). diff --git a/mm/slub.c b/mm/slub.c index 437485a2408d..623c17a4cdd6 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2463,7 +2463,7 @@ static inline void remove_partial(struct kmem_cache_node *n, } /* - * Called only for kmem_cache_debug() caches instead of acquire_slab(), with a + * Called only for kmem_cache_debug() caches instead of remove_partial(), with a * slab from the n->partial list. Remove only a single object from the slab, do * the alloc_debug_processing() checks and leave the slab on the list, or move * it to full list if it was the last free object. Hi Vlastimil, could you please help to fold it? Thanks! >> -{ >> - void *freelist; >> - unsigned long counters; >> - struct slab new; >> - >> - lockdep_assert_held(&n->list_lock); >> - >> - /* >> - * Zap the freelist and set the frozen bit. >> - * The old freelist is the list of objects for the >> - * per cpu allocation list. >> - */ >> - freelist = slab->freelist; >> - counters = slab->counters; >> - new.counters = counters; >> - if (mode) { >> - new.inuse = slab->objects; >> - new.freelist = NULL; >> - } else { >> - new.freelist = freelist; >> - } >> - >> - VM_BUG_ON(new.frozen); >> - new.frozen = 1; >> - >> - if (!__slab_update_freelist(s, slab, >> - freelist, counters, >> - new.freelist, new.counters, >> - "acquire_slab")) >> - return NULL; >> - >> - remove_partial(n, slab); >> - WARN_ON(!freelist); >> - return freelist; >> -} >> - >> #ifdef CONFIG_SLUB_CPU_PARTIAL >> static void put_cpu_partial(struct kmem_cache *s, struct slab *slab, int drain); >> #else >> @@ -2295,7 +2250,6 @@ static struct slab *get_partial_node(struct kmem_cache *s, >> struct partial_context *pc) >> { >> struct slab *slab, *slab2, *partial = NULL; >> - void *object = NULL; >> unsigned long flags; >> unsigned int partial_slabs = 0; >> >> @@ -2314,7 +2268,7 @@ static struct slab *get_partial_node(struct kmem_cache *s, >> continue; >> >> if (IS_ENABLED(CONFIG_SLUB_TINY) || kmem_cache_debug(s)) { >> - object = alloc_single_from_partial(s, n, slab, >> + void *object = alloc_single_from_partial(s, n, slab, >> pc->orig_size); >> if (object) { >> partial = slab; >> @@ -2324,13 +2278,10 @@ static struct slab *get_partial_node(struct kmem_cache *s, >> continue; >> } >> >> - object = acquire_slab(s, n, slab, object == NULL); >> - if (!object) >> - break; >> + remove_partial(n, slab); >> >> if (!partial) { >> partial = slab; >> - pc->object = object; >> stat(s, ALLOC_FROM_PARTIAL); >> } else { >> put_cpu_partial(s, slab, 0); >> @@ -2629,9 +2580,6 @@ static void __unfreeze_partials(struct kmem_cache *s, struct slab *partial_slab) >> unsigned long flags = 0; >> >> while (partial_slab) { >> - struct slab new; >> - struct slab old; >> - >> slab = partial_slab; >> partial_slab = slab->next; >> >> @@ -2644,23 +2592,7 @@ static void __unfreeze_partials(struct kmem_cache *s, struct slab *partial_slab) >> spin_lock_irqsave(&n->list_lock, flags); >> } >> >> - do { >> - >> - old.freelist = slab->freelist; >> - old.counters = slab->counters; >> - VM_BUG_ON(!old.frozen); >> - >> - new.counters = old.counters; >> - new.freelist = old.freelist; >> - >> - new.frozen = 0; >> - >> - } while (!__slab_update_freelist(s, slab, >> - old.freelist, old.counters, >> - new.freelist, new.counters, >> - "unfreezing slab")); >> - >> - if (unlikely(!new.inuse && n->nr_partial >= s->min_partial)) { >> + if (unlikely(!slab->inuse && n->nr_partial >= s->min_partial)) { >> slab->next = slab_to_discard; >> slab_to_discard = slab; >> } else { >> @@ -3167,7 +3099,6 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, >> node = NUMA_NO_NODE; >> goto new_slab; >> } >> -redo: >> >> if (unlikely(!node_match(slab, node))) { >> /* >> @@ -3243,7 +3174,8 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, >> >> new_slab: >> >> - if (slub_percpu_partial(c)) { >> +#ifdef CONFIG_SLUB_CPU_PARTIAL >> + while (slub_percpu_partial(c)) { >> local_lock_irqsave(&s->cpu_slab->lock, flags); >> if (unlikely(c->slab)) { >> local_unlock_irqrestore(&s->cpu_slab->lock, flags); >> @@ -3255,12 +3187,22 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, >> goto new_objects; >> } >> >> - slab = c->slab = slub_percpu_partial(c); >> + slab = slub_percpu_partial(c); >> slub_set_percpu_partial(c, slab); >> local_unlock_irqrestore(&s->cpu_slab->lock, flags); >> stat(s, CPU_PARTIAL_ALLOC); >> - goto redo; >> + >> + if (unlikely(!node_match(slab, node) || >> + !pfmemalloc_match(slab, gfpflags))) { >> + slab->next = NULL; >> + __unfreeze_partials(s, slab); >> + continue; >> + } >> + >> + freelist = freeze_slab(s, slab); >> + goto retry_load_slab; >> } >> +#endif >> >> new_objects: >> >> @@ -3268,8 +3210,8 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, >> pc.orig_size = orig_size; >> slab = get_partial(s, node, &pc); >> if (slab) { >> - freelist = pc.object; >> if (kmem_cache_debug(s)) { >> + freelist = pc.object; >> /* >> * For debug caches here we had to go through >> * alloc_single_from_partial() so just store the >> @@ -3281,6 +3223,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, >> return freelist; >> } >> >> + freelist = freeze_slab(s, slab); >> goto retry_load_slab; >> } >> >> @@ -3682,18 +3625,8 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, >> was_frozen = new.frozen; >> new.inuse -= cnt; >> if ((!new.inuse || !prior) && !was_frozen) { >> - >> - if (kmem_cache_has_cpu_partial(s) && !prior) { >> - >> - /* >> - * Slab was on no list before and will be >> - * partially empty >> - * We can defer the list move and instead >> - * freeze it. >> - */ >> - new.frozen = 1; >> - >> - } else { /* Needs to be taken off a list */ >> + /* Needs to be taken off a list */ >> + if (!kmem_cache_has_cpu_partial(s) || prior) { >> >> n = get_node(s, slab_nid(slab)); >> /* >> @@ -3723,9 +3656,9 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, >> * activity can be necessary. >> */ >> stat(s, FREE_FROZEN); >> - } else if (new.frozen) { >> + } else if (kmem_cache_has_cpu_partial(s) && !prior) { >> /* >> - * If we just froze the slab then put it onto the >> + * If we started with a full slab then put it onto the >> * per cpu partial list. >> */ >> put_cpu_partial(s, slab, 1); >> -- > > Looks good to me, > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > Thanks! > >> 2.20.1 >>