Message ID | 20231204-slub-cleanup-hooks-v1-2-88b65f7cd9d5@suse.cz |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp2991899vqy; Mon, 4 Dec 2023 11:35:10 -0800 (PST) X-Google-Smtp-Source: AGHT+IEhVgT2PY0N5ZwuLhMwjZM32oCuA4OHOnpahPJ5OnIOR0Vbay4gL+c3Ch8snnJZDfjvrOCh X-Received: by 2002:a17:902:c408:b0:1d0:1e49:3f60 with SMTP id k8-20020a170902c40800b001d01e493f60mr3042067plk.27.1701718509854; Mon, 04 Dec 2023 11:35:09 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701718509; cv=none; d=google.com; s=arc-20160816; b=d4Ye9EsIB/KzqRCpkveAIZDgAFa4BCRlizx++hBYlSFj6aTOw6youD3wcvW7314z93 zSzkcaKRE5w1x9H1ndmkLCJq3kNDYXo/4iUEQXyWAnCU8eCVBXR0ai0LRs9z/sl3XeW9 Yx2CoLVX9zBm1uq7jzernI6FsCzbtqwd2BUXR5Xku9viXqVDzeUsmIN4wy6y2NfQ/+24 fb/gr869ysEWkmFcguj8N3T1iRwEJo1JyF0gOZB4KNohHsh1wUt1+4uP2xI/yw68BGp4 Dkd1ic+TOBziFwNyCtO2fhEz980KoMsGLJD3vOIFXV68fRecwIQL3ctTC6kB1rwn3YRL x2pg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:in-reply-to:references:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature:dkim-signature; bh=4fSahCwhPLa3fIglvL1TRmtTOqYm3xhkbDoawZHdw9A=; fh=QGaHzSWEdTlyx4gN2Cp05xyAuU2J2OcL//unbyREezU=; b=WufmHg/P6QJ5hfIE8Z9ELxqx9ko/CsgAhGs5b0X2+QgLOXJfcqU/Ysk3IVJPzCMk40 AY8IXzlJB62bAcbkyGtYhXXSoX1v1HF2skLj299gf3oBfXuHdMzRVlnZ5iMJJ4LTWVFo cMRdGQcZjXCsTPpYG37dbHc818r/WN+IY/zEIlyBV/eLcDLm95L521M9h8R+HV8lrlvJ DI5fmzmZLTmtVZ8Z5xmBPr+LxnGxPM6FqVK+OjQurLv90Sibqd9CpmAxxlXI9Wpyn2BI LHiVpvxWG/gZASp3OgXAI0xO6I3JC7yDDTGJJvcLX64VWIlG6xZFRWPlO8GSzZJzT+Ga wMDw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.cz header.s=susede2_rsa header.b=cYuegURd; dkim=neutral (no key) header.i=@suse.cz header.s=susede2_ed25519 header.b=z8JNsCGQ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id jd9-20020a170903260900b001d00a56b03asi1580563plb.337.2023.12.04.11.35.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Dec 2023 11:35:09 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.cz header.s=susede2_rsa header.b=cYuegURd; dkim=neutral (no key) header.i=@suse.cz header.s=susede2_ed25519 header.b=z8JNsCGQ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 2708080C61B4; Mon, 4 Dec 2023 11:35:05 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233250AbjLDTex (ORCPT <rfc822;chrisfriedt@gmail.com> + 99 others); Mon, 4 Dec 2023 14:34:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45800 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230317AbjLDTes (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 4 Dec 2023 14:34:48 -0500 Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2a07:de40:b251:101:10:150:64:2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5DDE1E6 for <linux-kernel@vger.kernel.org>; Mon, 4 Dec 2023 11:34:54 -0800 (PST) Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A51611FE6E; Mon, 4 Dec 2023 19:34:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1701718492; h=from:from:reply-to: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=4fSahCwhPLa3fIglvL1TRmtTOqYm3xhkbDoawZHdw9A=; b=cYuegURdK6TarNJSSo68uWkNBOrhbmxZDmoXF/uS7znw/iEtrWLpnV32Dfv5L/PQtD6jMV QULfAq7/mAGtPm9Cbv6R2L7cGyXMaX4ps56QutuI/yGpFkZL7SUzA/8QS4L54o79rRjq4f NWPBKnLkVzZDYQTjYHbbwLf2ZFVPFKY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1701718492; h=from:from:reply-to: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=4fSahCwhPLa3fIglvL1TRmtTOqYm3xhkbDoawZHdw9A=; b=z8JNsCGQ0Xr3e3YcoHYBA6JiZnYEPfBf9mIKjtgLx1BoQFMrFN0F1iTKMetguIwoGikLxp evtQCq2pw+erbpCw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 8875313AC2; Mon, 4 Dec 2023 19:34:52 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id OCn7INwpbmUPMwAAD6G6ig (envelope-from <vbabka@suse.cz>); Mon, 04 Dec 2023 19:34:52 +0000 From: Vlastimil Babka <vbabka@suse.cz> Date: Mon, 04 Dec 2023 20:34:41 +0100 Subject: [PATCH 2/4] mm/slub: introduce __kmem_cache_free_bulk() without free hooks MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231204-slub-cleanup-hooks-v1-2-88b65f7cd9d5@suse.cz> References: <20231204-slub-cleanup-hooks-v1-0-88b65f7cd9d5@suse.cz> In-Reply-To: <20231204-slub-cleanup-hooks-v1-0-88b65f7cd9d5@suse.cz> To: Christoph Lameter <cl@linux.com>, Pekka Enberg <penberg@kernel.org>, David Rientjes <rientjes@google.com>, Joonsoo Kim <iamjoonsoo.kim@lge.com> Cc: Andrew Morton <akpm@linux-foundation.org>, Roman Gushchin <roman.gushchin@linux.dev>, Hyeonggon Yoo <42.hyeyoo@gmail.com>, Alexander Potapenko <glider@google.com>, Marco Elver <elver@google.com>, Dmitry Vyukov <dvyukov@google.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, Vlastimil Babka <vbabka@suse.cz> X-Mailer: b4 0.12.4 Authentication-Results: smtp-out2.suse.de; none X-Spam-Score: 3.33 X-Spamd-Result: default: False [3.33 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; BAYES_SPAM(2.93)[93.30%]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FREEMAIL_ENVRCPT(0.00)[gmail.com]; TAGGED_RCPT(0.00)[]; MIME_GOOD(-0.10)[text/plain]; NEURAL_HAM_LONG(-1.00)[-1.000]; R_RATELIMIT(0.00)[to_ip_from(RLtz7ce9b89hw8xzamye9qeynd)]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.cz:s=susede2_rsa,suse.cz:s=susede2_ed25519]; RCPT_COUNT_TWELVE(0.00)[14]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.cz:email]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_CC(0.00)[linux-foundation.org,linux.dev,gmail.com,google.com,kvack.org,vger.kernel.org,googlegroups.com,suse.cz]; RCVD_TLS_ALL(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; SUSPICIOUS_RECIPS(1.50)[] X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Mon, 04 Dec 2023 11:35:05 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784381188232377951 X-GMAIL-MSGID: 1784381188232377951 |
Series |
SLUB: cleanup hook processing
|
|
Commit Message
Vlastimil Babka
Dec. 4, 2023, 7:34 p.m. UTC
Currently, when __kmem_cache_alloc_bulk() fails, it frees back the
objects that were allocated before the failure, using
kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free
hooks (KASAN etc.) and those expect objects that were processed by the
post alloc hooks, slab_post_alloc_hook() is called before
kmem_cache_free_bulk().
This is wasteful, although not a big concern in practice for the rare
error path. But in order to efficiently handle percpu array batch refill
and free in the near future, we will also need a variant of
kmem_cache_free_bulk() that avoids the free hooks. So introduce it now
and use it for the failure path.
As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg
parameter, remove it.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/slub.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
Comments
On 2023/12/5 03:34, Vlastimil Babka wrote: > Currently, when __kmem_cache_alloc_bulk() fails, it frees back the > objects that were allocated before the failure, using > kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free > hooks (KASAN etc.) and those expect objects that were processed by the > post alloc hooks, slab_post_alloc_hook() is called before > kmem_cache_free_bulk(). > > This is wasteful, although not a big concern in practice for the rare > error path. But in order to efficiently handle percpu array batch refill > and free in the near future, we will also need a variant of > kmem_cache_free_bulk() that avoids the free hooks. So introduce it now > and use it for the failure path. > > As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg > parameter, remove it. The objects may have been charged before, but it seems __kmem_cache_alloc_bulk() forget to uncharge them? I can't find "uncharge" in do_slab_free(), or maybe the bulk interface won't be used on chargeable slab? Thanks. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/slub.c | 33 ++++++++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 7 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index d7b0ca6012e0..0742564c4538 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -4478,6 +4478,27 @@ int build_detached_freelist(struct kmem_cache *s, size_t size, > return same; > } > > +/* > + * Internal bulk free of objects that were not initialised by the post alloc > + * hooks and thus should not be processed by the free hooks > + */ > +static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) > +{ > + if (!size) > + return; > + > + do { > + struct detached_freelist df; > + > + size = build_detached_freelist(s, size, p, &df); > + if (!df.slab) > + continue; > + > + do_slab_free(df.s, df.slab, df.freelist, df.tail, df.cnt, > + _RET_IP_); > + } while (likely(size)); > +} > + > /* Note that interrupts must be enabled when calling this function. */ > void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) > { > @@ -4499,7 +4520,7 @@ EXPORT_SYMBOL(kmem_cache_free_bulk); > > #ifndef CONFIG_SLUB_TINY > static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > - size_t size, void **p, struct obj_cgroup *objcg) > + size_t size, void **p) > { > struct kmem_cache_cpu *c; > unsigned long irqflags; > @@ -4563,14 +4584,13 @@ static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > > error: > slub_put_cpu_ptr(s->cpu_slab); > - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); > - kmem_cache_free_bulk(s, i, p); > + __kmem_cache_free_bulk(s, i, p); > return 0; > > } > #else /* CONFIG_SLUB_TINY */ > static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > - size_t size, void **p, struct obj_cgroup *objcg) > + size_t size, void **p) > { > int i; > > @@ -4593,8 +4613,7 @@ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > return i; > > error: > - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); > - kmem_cache_free_bulk(s, i, p); > + __kmem_cache_free_bulk(s, i, p); > return 0; > } > #endif /* CONFIG_SLUB_TINY */ > @@ -4614,7 +4633,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, > if (unlikely(!s)) > return 0; > > - i = __kmem_cache_alloc_bulk(s, flags, size, p, objcg); > + i = __kmem_cache_alloc_bulk(s, flags, size, p); > > /* > * memcg and kmem_cache debug support and memory initialization. >
On 12/5/23 09:19, Chengming Zhou wrote: > On 2023/12/5 03:34, Vlastimil Babka wrote: >> Currently, when __kmem_cache_alloc_bulk() fails, it frees back the >> objects that were allocated before the failure, using >> kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free >> hooks (KASAN etc.) and those expect objects that were processed by the >> post alloc hooks, slab_post_alloc_hook() is called before >> kmem_cache_free_bulk(). >> >> This is wasteful, although not a big concern in practice for the rare >> error path. But in order to efficiently handle percpu array batch refill >> and free in the near future, we will also need a variant of >> kmem_cache_free_bulk() that avoids the free hooks. So introduce it now >> and use it for the failure path. >> >> As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg >> parameter, remove it. > > The objects may have been charged before, but it seems __kmem_cache_alloc_bulk() > forget to uncharge them? I can't find "uncharge" in do_slab_free(), or maybe > the bulk interface won't be used on chargeable slab? You're right! I missed that the memcg_pre_alloc_hook() already does the charging, so we need to uncharge. How does this look? Thanks for noticing! ----8<---- From 52f8e77fdfeabffffdce6b761ba5508e940df3be Mon Sep 17 00:00:00 2001 From: Vlastimil Babka <vbabka@suse.cz> Date: Thu, 2 Nov 2023 16:34:39 +0100 Subject: [PATCH 2/4] mm/slub: introduce __kmem_cache_free_bulk() without free hooks Currently, when __kmem_cache_alloc_bulk() fails, it frees back the objects that were allocated before the failure, using kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free hooks (KASAN etc.) and those expect objects that were processed by the post alloc hooks, slab_post_alloc_hook() is called before kmem_cache_free_bulk(). This is wasteful, although not a big concern in practice for the rare error path. But in order to efficiently handle percpu array batch refill and free in the near future, we will also need a variant of kmem_cache_free_bulk() that avoids the free hooks. So introduce it now and use it for the failure path. In case of failure we however still need to perform memcg uncharge so handle that in a new memcg_slab_alloc_error_hook(). Thanks to Chengming Zhou for noticing the missing uncharge. As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg parameter, remove it. Signed-off-by: Vlastimil Babka <vbabka@suse.cz> --- mm/slub.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index d7b0ca6012e0..0a9e4bd0dd68 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2003,6 +2003,14 @@ void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, __memcg_slab_free_hook(s, slab, p, objects, objcgs); } + +static inline +void memcg_slab_alloc_error_hook(struct kmem_cache *s, int objects, + struct obj_cgroup *objcg) +{ + if (objcg) + obj_cgroup_uncharge(objcg, objects * obj_full_size(s)); +} #else /* CONFIG_MEMCG_KMEM */ static inline struct mem_cgroup *memcg_from_slab_obj(void *ptr) { @@ -2032,6 +2040,12 @@ static inline void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, int objects) { } + +static inline +void memcg_slab_alloc_error_hook(struct kmem_cache *s, int objects, + struct obj_cgroup *objcg) +{ +} #endif /* CONFIG_MEMCG_KMEM */ /* @@ -4478,6 +4492,27 @@ int build_detached_freelist(struct kmem_cache *s, size_t size, return same; } +/* + * Internal bulk free of objects that were not initialised by the post alloc + * hooks and thus should not be processed by the free hooks + */ +static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) +{ + if (!size) + return; + + do { + struct detached_freelist df; + + size = build_detached_freelist(s, size, p, &df); + if (!df.slab) + continue; + + do_slab_free(df.s, df.slab, df.freelist, df.tail, df.cnt, + _RET_IP_); + } while (likely(size)); +} + /* Note that interrupts must be enabled when calling this function. */ void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) { @@ -4498,8 +4533,9 @@ void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) EXPORT_SYMBOL(kmem_cache_free_bulk); #ifndef CONFIG_SLUB_TINY -static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, - size_t size, void **p, struct obj_cgroup *objcg) +static inline +int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, + void **p) { struct kmem_cache_cpu *c; unsigned long irqflags; @@ -4563,14 +4599,13 @@ static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, error: slub_put_cpu_ptr(s->cpu_slab); - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); - kmem_cache_free_bulk(s, i, p); + __kmem_cache_free_bulk(s, i, p); return 0; } #else /* CONFIG_SLUB_TINY */ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, - size_t size, void **p, struct obj_cgroup *objcg) + size_t size, void **p) { int i; @@ -4593,8 +4628,7 @@ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, return i; error: - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); - kmem_cache_free_bulk(s, i, p); + __kmem_cache_free_bulk(s, i, p); return 0; } #endif /* CONFIG_SLUB_TINY */ @@ -4614,15 +4648,19 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, if (unlikely(!s)) return 0; - i = __kmem_cache_alloc_bulk(s, flags, size, p, objcg); + i = __kmem_cache_alloc_bulk(s, flags, size, p); /* * memcg and kmem_cache debug support and memory initialization. * Done outside of the IRQ disabled fastpath loop. */ - if (i != 0) + if (likely(i != 0)) { slab_post_alloc_hook(s, objcg, flags, size, p, slab_want_init_on_alloc(flags, s), s->object_size); + } else { + memcg_slab_alloc_error_hook(s, size, objcg); + } + return i; } EXPORT_SYMBOL(kmem_cache_alloc_bulk);
On 2023/12/6 03:57, Vlastimil Babka wrote: > On 12/5/23 09:19, Chengming Zhou wrote: >> On 2023/12/5 03:34, Vlastimil Babka wrote: >>> Currently, when __kmem_cache_alloc_bulk() fails, it frees back the >>> objects that were allocated before the failure, using >>> kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free >>> hooks (KASAN etc.) and those expect objects that were processed by the >>> post alloc hooks, slab_post_alloc_hook() is called before >>> kmem_cache_free_bulk(). >>> >>> This is wasteful, although not a big concern in practice for the rare >>> error path. But in order to efficiently handle percpu array batch refill >>> and free in the near future, we will also need a variant of >>> kmem_cache_free_bulk() that avoids the free hooks. So introduce it now >>> and use it for the failure path. >>> >>> As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg >>> parameter, remove it. >> >> The objects may have been charged before, but it seems __kmem_cache_alloc_bulk() >> forget to uncharge them? I can't find "uncharge" in do_slab_free(), or maybe >> the bulk interface won't be used on chargeable slab? > > You're right! I missed that the memcg_pre_alloc_hook() already does the > charging, so we need to uncharge. How does this look? Thanks for noticing! > > ----8<---- > From 52f8e77fdfeabffffdce6b761ba5508e940df3be Mon Sep 17 00:00:00 2001 > From: Vlastimil Babka <vbabka@suse.cz> > Date: Thu, 2 Nov 2023 16:34:39 +0100 > Subject: [PATCH 2/4] mm/slub: introduce __kmem_cache_free_bulk() without free > hooks > > Currently, when __kmem_cache_alloc_bulk() fails, it frees back the > objects that were allocated before the failure, using > kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free > hooks (KASAN etc.) and those expect objects that were processed by the > post alloc hooks, slab_post_alloc_hook() is called before > kmem_cache_free_bulk(). > > This is wasteful, although not a big concern in practice for the rare > error path. But in order to efficiently handle percpu array batch refill > and free in the near future, we will also need a variant of > kmem_cache_free_bulk() that avoids the free hooks. So introduce it now > and use it for the failure path. > > In case of failure we however still need to perform memcg uncharge so > handle that in a new memcg_slab_alloc_error_hook(). Thanks to Chengming > Zhou for noticing the missing uncharge. > > As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg > parameter, remove it. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> Looks good to me! Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com> Thanks! > --- > mm/slub.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 47 insertions(+), 9 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index d7b0ca6012e0..0a9e4bd0dd68 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2003,6 +2003,14 @@ void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, > > __memcg_slab_free_hook(s, slab, p, objects, objcgs); > } > + > +static inline > +void memcg_slab_alloc_error_hook(struct kmem_cache *s, int objects, > + struct obj_cgroup *objcg) > +{ > + if (objcg) > + obj_cgroup_uncharge(objcg, objects * obj_full_size(s)); > +} > #else /* CONFIG_MEMCG_KMEM */ > static inline struct mem_cgroup *memcg_from_slab_obj(void *ptr) > { > @@ -2032,6 +2040,12 @@ static inline void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, > void **p, int objects) > { > } > + > +static inline > +void memcg_slab_alloc_error_hook(struct kmem_cache *s, int objects, > + struct obj_cgroup *objcg) > +{ > +} > #endif /* CONFIG_MEMCG_KMEM */ > > /* > @@ -4478,6 +4492,27 @@ int build_detached_freelist(struct kmem_cache *s, size_t size, > return same; > } > > +/* > + * Internal bulk free of objects that were not initialised by the post alloc > + * hooks and thus should not be processed by the free hooks > + */ > +static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) > +{ > + if (!size) > + return; > + > + do { > + struct detached_freelist df; > + > + size = build_detached_freelist(s, size, p, &df); > + if (!df.slab) > + continue; > + > + do_slab_free(df.s, df.slab, df.freelist, df.tail, df.cnt, > + _RET_IP_); > + } while (likely(size)); > +} > + > /* Note that interrupts must be enabled when calling this function. */ > void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) > { > @@ -4498,8 +4533,9 @@ void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) > EXPORT_SYMBOL(kmem_cache_free_bulk); > > #ifndef CONFIG_SLUB_TINY > -static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > - size_t size, void **p, struct obj_cgroup *objcg) > +static inline > +int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, > + void **p) > { > struct kmem_cache_cpu *c; > unsigned long irqflags; > @@ -4563,14 +4599,13 @@ static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > > error: > slub_put_cpu_ptr(s->cpu_slab); > - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); > - kmem_cache_free_bulk(s, i, p); > + __kmem_cache_free_bulk(s, i, p); > return 0; > > } > #else /* CONFIG_SLUB_TINY */ > static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > - size_t size, void **p, struct obj_cgroup *objcg) > + size_t size, void **p) > { > int i; > > @@ -4593,8 +4628,7 @@ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, > return i; > > error: > - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); > - kmem_cache_free_bulk(s, i, p); > + __kmem_cache_free_bulk(s, i, p); > return 0; > } > #endif /* CONFIG_SLUB_TINY */ > @@ -4614,15 +4648,19 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, > if (unlikely(!s)) > return 0; > > - i = __kmem_cache_alloc_bulk(s, flags, size, p, objcg); > + i = __kmem_cache_alloc_bulk(s, flags, size, p); > > /* > * memcg and kmem_cache debug support and memory initialization. > * Done outside of the IRQ disabled fastpath loop. > */ > - if (i != 0) > + if (likely(i != 0)) { > slab_post_alloc_hook(s, objcg, flags, size, p, > slab_want_init_on_alloc(flags, s), s->object_size); > + } else { > + memcg_slab_alloc_error_hook(s, size, objcg); > + } > + > return i; > } > EXPORT_SYMBOL(kmem_cache_alloc_bulk);
diff --git a/mm/slub.c b/mm/slub.c index d7b0ca6012e0..0742564c4538 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4478,6 +4478,27 @@ int build_detached_freelist(struct kmem_cache *s, size_t size, return same; } +/* + * Internal bulk free of objects that were not initialised by the post alloc + * hooks and thus should not be processed by the free hooks + */ +static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) +{ + if (!size) + return; + + do { + struct detached_freelist df; + + size = build_detached_freelist(s, size, p, &df); + if (!df.slab) + continue; + + do_slab_free(df.s, df.slab, df.freelist, df.tail, df.cnt, + _RET_IP_); + } while (likely(size)); +} + /* Note that interrupts must be enabled when calling this function. */ void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p) { @@ -4499,7 +4520,7 @@ EXPORT_SYMBOL(kmem_cache_free_bulk); #ifndef CONFIG_SLUB_TINY static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, - size_t size, void **p, struct obj_cgroup *objcg) + size_t size, void **p) { struct kmem_cache_cpu *c; unsigned long irqflags; @@ -4563,14 +4584,13 @@ static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, error: slub_put_cpu_ptr(s->cpu_slab); - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); - kmem_cache_free_bulk(s, i, p); + __kmem_cache_free_bulk(s, i, p); return 0; } #else /* CONFIG_SLUB_TINY */ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, - size_t size, void **p, struct obj_cgroup *objcg) + size_t size, void **p) { int i; @@ -4593,8 +4613,7 @@ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, return i; error: - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size); - kmem_cache_free_bulk(s, i, p); + __kmem_cache_free_bulk(s, i, p); return 0; } #endif /* CONFIG_SLUB_TINY */ @@ -4614,7 +4633,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, if (unlikely(!s)) return 0; - i = __kmem_cache_alloc_bulk(s, flags, size, p, objcg); + i = __kmem_cache_alloc_bulk(s, flags, size, p); /* * memcg and kmem_cache debug support and memory initialization.