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 11EB7C4167B for ; Tue, 5 Dec 2023 13:23:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A02CF6B008A; Tue, 5 Dec 2023 08:23:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9B2D36B008C; Tue, 5 Dec 2023 08:23:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 87BB76B0092; Tue, 5 Dec 2023 08:23:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 78B0F6B008A for ; Tue, 5 Dec 2023 08:23:52 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 52764A09AC for ; Tue, 5 Dec 2023 13:23:52 +0000 (UTC) X-FDA: 81532832304.03.8F4CFB7 Received: from out-186.mta0.migadu.com (out-186.mta0.migadu.com [91.218.175.186]) by imf30.hostedemail.com (Postfix) with ESMTP id 5F1318001C for ; Tue, 5 Dec 2023 13:23:49 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=mDZ6ph0o; spf=pass (imf30.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.186 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=1701782629; 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=6Y3aGufX1vl5A2ALGYiBVD+TybG4OsFZRCv9diM79A8=; b=3lyhxLZkfur1pSROG1zt9E4qnbMiMM3HuGIHHE4TrUBeA8hpbZEawwWbjQ/UbfpisVlqPP RUKBqdSA95GLUiYrYd/rA9mnR/mtply4BC28Ts53aVeOmOAdwx+m014fBvvPKew9nh9ZVU SCOr3AOwWfr+Npkh+yPhcyZiNHSz52s= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=mDZ6ph0o; spf=pass (imf30.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.186 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701782629; a=rsa-sha256; cv=none; b=JW/yJlSLMBDgah9KulrcZ1sUfHUC86u+gQ/wEMX+e43jmr38mgPIHksW9pomJAd9WumD8/ xBhSKcv9ppdFz6+oUcibRGLfhM8aHoyQKXse5/AdZI2pRMwCQ0VBgKMVUHlFeDkGQcujRA q6MwEnmuB5Skf0yqjmQ/AcgMOnHo2aw= Message-ID: <93adcdc0-6f32-45fa-b311-34a27ff94290@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1701782627; 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=6Y3aGufX1vl5A2ALGYiBVD+TybG4OsFZRCv9diM79A8=; b=mDZ6ph0oD6GN4VIyjlxG+xz5R+xV+/uKAA5//QVwR59UjvRThXVEbE3/QvAFOfUgypbOZA vkjBJxn1O6J1qOCwVw8BB4AVmDNXEdLfNgJWq1aN+0yguJxd7UP182RiYrh1Lgp0Bi918z G9pDHo0IQHxFoPXFTcxbQm2XEqmMN/Q= Date: Tue, 5 Dec 2023 21:23:38 +0800 MIME-Version: 1.0 Subject: Re: [PATCH 3/4] mm/slub: handle bulk and single object freeing separately Content-Language: en-US To: Vlastimil Babka , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim Cc: Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Alexander Potapenko , Marco Elver , Dmitry Vyukov , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com References: <20231204-slub-cleanup-hooks-v1-0-88b65f7cd9d5@suse.cz> <20231204-slub-cleanup-hooks-v1-3-88b65f7cd9d5@suse.cz> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: <20231204-slub-cleanup-hooks-v1-3-88b65f7cd9d5@suse.cz> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 5F1318001C X-Rspam-User: X-Stat-Signature: unfzijk7tdnzmrw5r7yr6dmh8id394zc X-Rspamd-Server: rspam01 X-HE-Tag: 1701782629-219201 X-HE-Meta: U2FsdGVkX1+S61IGHcDuhT4NPOWmCs+6xUVUdeRzbZ4bLocs+CLiMvqdAA43LbXSrDCCTCyXT7hK5gH1QsbOAtTQJM22ZqoTnX2ZgzEOmbaT2ZH1S2OP0KblZjLa30Ax0hJDwVbHrqQP9ryBK/oDOi2jDjLCLDJOCVWhOsdZoChnsu9oP1Be+/i3h7FQ6P/7Wk8UMWRxo2Xs+eA5H7SeTncrnTBparyR4hLriGd+6SXkPZo5UAZ4udwr1eHKdbTsfOwPjKWLzTRWJa/yL9sXq7H5WRd87/IXkKqrVBZcQRwOk1mZXHG39U1YCAdpuQMV+w5qPvVnVbJPTaw+snO48VepdipDv6Lld/LbRcZDLgJ1Zn8y9x+7sZZp/70l5U9wHG948zDUm73eTQTFkvU++isVGaFoQy1J8PHFRdhCl+8RBEwKCx8aN5d4uVWVJTbGq/DFL67GIqXyIDVKU2K8f1N8a4kqWQoXnP/K9v6UjLWxN6/DNXg79httrDp7o+gjroVhE0yljtHhCMch+6utqe8v95SG/LPDGUBOhQLm7FxqHTESRe5mAuazYCsYflVpsSA2rWHEPHdvND9ZWKj3F6xMwkGzqtqzJahqpY1iaHtzWQ3AI2wmOzI/lyOd+0xdXZCCkJvAgKLATQJ9k0Qry7wJIskJr1+O1hL2RTwvekJy1EeFHGoc/sdg/OeeSXh011Y0rf6/ymi8i7DwQ2Mpuf4TrviN+QWwZ4NRHFJADPtH5/wasTdqEH2WA+zcQTSXX/5AAP8C75I/aDAKdbZG/b66BWdM93M0yVPxqyu9KTkHfLTiyjHFtIpq89V02ahoc8W/Din+mUKGXxdLqfxCbhi5uAayCgvbeV/+aQdx8T9Av+tjW7F5PK2uk/727DzGg5TFue0cKU3Ov7cNebyOs4Umd8ihPZkHDpjIncJ+zOrTdKRBprOm71ETTS3qTHGJdkz281Ee9q5G5wKnOZY msudkxrY WEhtYH4CXGGRZCZUG3jXwZeBWdBbVLGGljsq6ojvHAY5qS3q/KrLmZU9A9IPFc6cZKWt37lGltJcBmwRTQUxHPMTcl1aDK2D/s7CRypKT5yFA+KwZSJeB4JomkRnQDdBR/r5p8wOsp3e5vcYrPHw6YeHWYiTEs+ZVi4A2mbt/wJR7zM1tC3KSbOIW+P+iSmxDKoq9CYMsW9yomQPmlAR7Y09fR16pJsFMNN4Z 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/5 03:34, Vlastimil Babka wrote: > Currently we have a single function slab_free() handling both single > object freeing and bulk freeing with necessary hooks, the latter case > requiring slab_free_freelist_hook(). It should be however better to > distinguish the two use cases for the following reasons: > > - code simpler to follow for the single object case > > - better code generation - although inlining should eliminate the > slab_free_freelist_hook() for single object freeing in case no > debugging options are enabled, it seems it's not perfect. When e.g. > KASAN is enabled, we're imposing additional unnecessary overhead for > single object freeing. > > - preparation to add percpu array caches in near future > > Therefore, simplify slab_free() for the single object case by dropping > unnecessary parameters and calling only slab_free_hook() instead of > slab_free_freelist_hook(). Rename the bulk variant to slab_free_bulk() > and adjust callers accordingly. > > While at it, flip (and document) slab_free_hook() return value so that > it returns true when the freeing can proceed, which matches the logic of > slab_free_freelist_hook() and is not confusingly the opposite. > > Additionally we can simplify a bit by changing the tail parameter of > do_slab_free() when freeing a single object - instead of NULL we can set > it equal to head. > > bloat-o-meter shows small code reduction with a .config that has KASAN > etc disabled: > > add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-118 (-118) > Function old new delta > kmem_cache_alloc_bulk 1203 1196 -7 > kmem_cache_free 861 835 -26 > __kmem_cache_free 741 704 -37 > kmem_cache_free_bulk 911 863 -48 > > Signed-off-by: Vlastimil Babka Looks good to me. Reviewed-by: Chengming Zhou Thanks! > --- > mm/slub.c | 59 +++++++++++++++++++++++++++++++++++------------------------ > 1 file changed, 35 insertions(+), 24 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 0742564c4538..ed2fa92e914c 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2037,9 +2037,12 @@ static inline void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, > /* > * Hooks for other subsystems that check memory allocations. In a typical > * production configuration these hooks all should produce no code at all. > + * > + * Returns true if freeing of the object can proceed, false if its reuse > + * was delayed by KASAN quarantine. > */ > -static __always_inline bool slab_free_hook(struct kmem_cache *s, > - void *x, bool init) > +static __always_inline > +bool slab_free_hook(struct kmem_cache *s, void *x, bool init) > { > kmemleak_free_recursive(x, s->flags); > kmsan_slab_free(s, x); > @@ -2072,7 +2075,7 @@ static __always_inline bool slab_free_hook(struct kmem_cache *s, > s->size - s->inuse - rsize); > } > /* KASAN might put x into memory quarantine, delaying its reuse. */ > - return kasan_slab_free(s, x, init); > + return !kasan_slab_free(s, x, init); > } > > static inline bool slab_free_freelist_hook(struct kmem_cache *s, > @@ -2082,7 +2085,7 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s, > > void *object; > void *next = *head; > - void *old_tail = *tail ? *tail : *head; > + void *old_tail = *tail; > > if (is_kfence_address(next)) { > slab_free_hook(s, next, false); > @@ -2098,8 +2101,8 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s, > next = get_freepointer(s, object); > > /* If object's reuse doesn't have to be delayed */ > - if (likely(!slab_free_hook(s, object, > - slab_want_init_on_free(s)))) { > + if (likely(slab_free_hook(s, object, > + slab_want_init_on_free(s)))) { > /* Move object to the new freelist */ > set_freepointer(s, object, *head); > *head = object; > @@ -2114,9 +2117,6 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s, > } > } while (object != old_tail); > > - if (*head == *tail) > - *tail = NULL; > - > return *head != NULL; > } > > @@ -4227,7 +4227,6 @@ static __always_inline void do_slab_free(struct kmem_cache *s, > struct slab *slab, void *head, void *tail, > int cnt, unsigned long addr) > { > - void *tail_obj = tail ? : head; > struct kmem_cache_cpu *c; > unsigned long tid; > void **freelist; > @@ -4246,14 +4245,14 @@ static __always_inline void do_slab_free(struct kmem_cache *s, > barrier(); > > if (unlikely(slab != c->slab)) { > - __slab_free(s, slab, head, tail_obj, cnt, addr); > + __slab_free(s, slab, head, tail, cnt, addr); > return; > } > > if (USE_LOCKLESS_FAST_PATH()) { > freelist = READ_ONCE(c->freelist); > > - set_freepointer(s, tail_obj, freelist); > + set_freepointer(s, tail, freelist); > > if (unlikely(!__update_cpu_freelist_fast(s, freelist, head, tid))) { > note_cmpxchg_failure("slab_free", s, tid); > @@ -4270,7 +4269,7 @@ static __always_inline void do_slab_free(struct kmem_cache *s, > tid = c->tid; > freelist = c->freelist; > > - set_freepointer(s, tail_obj, freelist); > + set_freepointer(s, tail, freelist); > c->freelist = head; > c->tid = next_tid(tid); > > @@ -4283,15 +4282,27 @@ static void do_slab_free(struct kmem_cache *s, > struct slab *slab, void *head, void *tail, > int cnt, unsigned long addr) > { > - void *tail_obj = tail ? : head; > - > - __slab_free(s, slab, head, tail_obj, cnt, addr); > + __slab_free(s, slab, head, tail, cnt, addr); > } > #endif /* CONFIG_SLUB_TINY */ > > -static __fastpath_inline void slab_free(struct kmem_cache *s, struct slab *slab, > - void *head, void *tail, void **p, int cnt, > - unsigned long addr) > +static __fastpath_inline > +void slab_free(struct kmem_cache *s, struct slab *slab, void *object, > + unsigned long addr) > +{ > + bool init; > + > + memcg_slab_free_hook(s, slab, &object, 1); > + > + init = !is_kfence_address(object) && slab_want_init_on_free(s); > + > + if (likely(slab_free_hook(s, object, init))) > + do_slab_free(s, slab, object, object, 1, addr); > +} > + > +static __fastpath_inline > +void slab_free_bulk(struct kmem_cache *s, struct slab *slab, void *head, > + void *tail, void **p, int cnt, unsigned long addr) > { > memcg_slab_free_hook(s, slab, p, cnt); > /* > @@ -4305,7 +4316,7 @@ static __fastpath_inline void slab_free(struct kmem_cache *s, struct slab *slab, > #ifdef CONFIG_KASAN_GENERIC > void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr) > { > - do_slab_free(cache, virt_to_slab(x), x, NULL, 1, addr); > + do_slab_free(cache, virt_to_slab(x), x, x, 1, addr); > } > #endif > > @@ -4349,7 +4360,7 @@ void kmem_cache_free(struct kmem_cache *s, void *x) > if (!s) > return; > trace_kmem_cache_free(_RET_IP_, x, s); > - slab_free(s, virt_to_slab(x), x, NULL, &x, 1, _RET_IP_); > + slab_free(s, virt_to_slab(x), x, _RET_IP_); > } > EXPORT_SYMBOL(kmem_cache_free); > > @@ -4395,7 +4406,7 @@ void kfree(const void *object) > > slab = folio_slab(folio); > s = slab->slab_cache; > - slab_free(s, slab, x, NULL, &x, 1, _RET_IP_); > + slab_free(s, slab, x, _RET_IP_); > } > EXPORT_SYMBOL(kfree); > > @@ -4512,8 +4523,8 @@ void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) > if (!df.slab) > continue; > > - slab_free(df.s, df.slab, df.freelist, df.tail, &p[size], df.cnt, > - _RET_IP_); > + slab_free_bulk(df.s, df.slab, df.freelist, df.tail, &p[size], > + df.cnt, _RET_IP_); > } while (likely(size)); > } > EXPORT_SYMBOL(kmem_cache_free_bulk); >