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 CB719C3DA4A for ; Fri, 16 Aug 2024 19:21:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2FE9F6B03A7; Fri, 16 Aug 2024 15:21:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 289126B03A8; Fri, 16 Aug 2024 15:21:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 101266B03A9; Fri, 16 Aug 2024 15:21:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id E19DF6B03A7 for ; Fri, 16 Aug 2024 15:21:10 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 8D314160271 for ; Fri, 16 Aug 2024 19:21:10 +0000 (UTC) X-FDA: 82459076700.10.537396D Received: from mail-yw1-f169.google.com (mail-yw1-f169.google.com [209.85.128.169]) by imf04.hostedemail.com (Postfix) with ESMTP id B493840007 for ; Fri, 16 Aug 2024 19:21:07 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Bf41wLqw; spf=pass (imf04.hostedemail.com: domain of surenb@google.com designates 209.85.128.169 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723835993; 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=XAkZOqNhnveajHipNOMhpr0ZJRkMRtizq6QmxIcG1So=; b=X6y6VXH40uoRkyowTROOo1mu30KXU3dduaH8atykj1bX51n5/eKNntsXaWcYFUk2/G1Nck e5bn8YboEy/K2Kvv15/Vt3yrnkl8psa3sH8WaoLD8+TQMPvnTEPSkmvhMNXpDiezfEmhqf Ugt1AeP7X3HzA/sc2DpaHEH1iLNUeUQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723835993; a=rsa-sha256; cv=none; b=mrlczSnOtvEBsSyUmYdf20HjVT2bN4PFbKsrh0UNqycjywfUZe+xAQlHcVRNJHKNP8v8oA U9kV9ZyNAcNiGAF3IyEL5VaEBEDBXSY/Y6CpssK1kgt1NLYL2p/06y/RctTu1pw7skUXKT 9LCL6gVnwRWKIPnNzBeGEH9qP0Q2i/g= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Bf41wLqw; spf=pass (imf04.hostedemail.com: domain of surenb@google.com designates 209.85.128.169 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yw1-f169.google.com with SMTP id 00721157ae682-690b6cbce11so22658487b3.2 for ; Fri, 16 Aug 2024 12:21:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723836067; x=1724440867; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=XAkZOqNhnveajHipNOMhpr0ZJRkMRtizq6QmxIcG1So=; b=Bf41wLqwNGBNssUYvC9XCajOwinrEs6e7+aSW+hmIAUmD0x2jdMS9rrNAtex1wTPvK qiE7JYfhGSz+wK/V8Oa4q+jhbVbs98sQIvo5L8XYJ2ZPn+aYkIP0e8BP/+jRYFYnNUUx 8R8LDXhZFstQQDvwk9ZNB560c30g7/fIekKJjzxuBzfosYzRJ0C3z6kDF/qyuYKiiiAI 47GqRRGojJ+t+M9QZgm9QNJS2YjiURO7gVwCMeeR9OAsHlyWnZbCG7C0pJk9UYoQZRuQ i9yVpcSczm87g9thrqYNW2zyHxH2hNiaSQ8WkN9keQTXCWwpS1HYHEw2jLEe9buUrGID ClCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723836067; x=1724440867; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XAkZOqNhnveajHipNOMhpr0ZJRkMRtizq6QmxIcG1So=; b=YN7ggywJ9fsA6KDBPiwAiiXlgfwV+WlHUvaCK1+sDNVQz5Ixzv26L1mPnwxrY1oPc5 6zHlv0AGm8i7/D7Af5ea1miUaI9EUhsUVddvKr2GsnHelxru7IDswDzsqBctUq8yNspA zcwOKIX/pYp2i5sI+Z8H/znZaeM61mOZEurMD091yGFFdsPZIJTIUfl55b7P9irEDMzo SUUuvBHog2289Hq+GUhvmS3bY/daZs9t6O0GzGHCg9LapEgmJNBTiF21UoymHsmMHlaF 2/i2f8qn793clZQNMf+CLgUArMkrJh6FuaVxBhnpBosz2BxdefjRSVl43XxyUr/tEwqE Zl2g== X-Forwarded-Encrypted: i=1; AJvYcCUqyZkC3nVesWDTt2NRnqJMBmRu0oG2ecOJWowbrSuCUsQONIwmfk9MMBXxS0NZ7xtxQSr9QPfRc9WozGwZuH+a/Yg= X-Gm-Message-State: AOJu0YwLaDamjtvhOh+i5M4LRdg+79CgWg/IdFtMJ2V3FwikoU1P3SlK NDxTbgncfwSEOfJfnt9l4KcZdQVKG3Db8BCy7a2vhYlXwJz+1Oi6Ypb0u1R3vDYgfMrJ7qV7OQ7 fXBSSrO1h3ngnWQMn+uTA1ApPESUbDE9iGqe0 X-Google-Smtp-Source: AGHT+IG12famSdUV7QWzpiG6d5wFEJ1L0urOdbl6yfxU1g/eEu5anDYt31tfyAV9GqApw8iQOqI6g+0Cf8ejNNMqEeA= X-Received: by 2002:a05:690c:6603:b0:6ad:8bbd:aecb with SMTP id 00721157ae682-6b1b8ddfd7cmr56760747b3.21.1723836066332; Fri, 16 Aug 2024 12:21:06 -0700 (PDT) MIME-Version: 1.0 References: <20240815072113.30288-1-hao.ge@linux.dev> <4a24c134-d4d3-20c5-45f5-4dfa961fbd73@linux.dev> In-Reply-To: <4a24c134-d4d3-20c5-45f5-4dfa961fbd73@linux.dev> From: Suren Baghdasaryan Date: Fri, 16 Aug 2024 12:20:53 -0700 Message-ID: Subject: Re: [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook To: Hao Ge Cc: cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, akpm@linux-foundation.org, vbabka@suse.cz, roman.gushchin@linux.dev, 42.hyeyoo@gmail.com, kees@kernel.org, kent.overstreet@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hao Ge Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B493840007 X-Stat-Signature: f7yod9dcuzqhdfn53ezkqaxa4ny1qcao X-HE-Tag: 1723836067-673481 X-HE-Meta: U2FsdGVkX1/bpdHhX5QEgFaQNW5pDvAJTT/t4mxG/qN0ADV7gABz2bPxBpQo+U4pl5KnwRHGOQRvNbTyazfCimZIWcwe90vL0AFeIlBFbgc+/Bi4PXjKaC5X6bhwb389n9XdW6s5qWG2AzGJ1nsBO7CPaO45dk6qJDktsFR4l6LENFfeRftY1s88HbraqmbuSjRurzxVpspbf29XkN1/cNQMnFD7JYWD8zm9aP9/w3SOnG/fPS4J9US2s0Jc6YwlQz4TbJp+aXz1j1zXgL7J4ZQiJgoGpeD198M6ofDzBZRWmVHfcFGCArNRwRy8QQDghKVIXJA3FglSqjGWD9KaJBL0ZBmt5pzrVwCqQ658Sp3f+b1N+LIE66xW2Udy9NdH6kIZ5yAbG+vmiW3cXAvLRvk4xQBpsw9SOH6QNxEQ6wqGeyW1BlM/+FIuojME58u0BJciq0A0S3Ne+3OFOZqhKYVYYdZRqs7P7/DtjMlU7Bjv/EzQ2b/fdzfRN6OB0e8AavEWGA9cZ53SRUVliCgLUUNdpFqBxn7l5E0wRAOE9eO/Lyqh0NlWQutach5W35kHBUJcbmYYO3UL9L4Ce9+x8f9tiXPtnU0xs/yhjl8FyhEMz0AEyV4vkzkV1AG+XdBV1mrbEFe/SaEcIYkpDTqAQsjk4ifs8tqXsWHjgjB3G72ZjpmnKjXTaTES3uDNVajVNZ3INuKvnpLGmIhqc0QZjF0B6dEVtpRsUzTRhzpScNDNCsguBxt0PVaAoCMIksDUyZyB1guPbv7XZ/IMBOg3qm7MsbHxuMEn3ExzabNtn9Obg/9rym0i+sCw0Bz9zErmkP5rTJ7pNjfdfS7pj5yHJsNzC//zQ5IoHDpdDXTHGcIkajQKcW1Lva9DlbU4Bbfp3p3x4QEE3+l9Z9E/i9yoApcs+BI0Y57EP2AZBA84lj5CGpR9KbbIKF7+PKlhJ5m3AQkgsV4OuQqkMrq92WR 4YB2z6ps WrTB8B52iFVivNOercd+jxe7jbcedyWqn+mB3xxOeWADQ1f27Yo2UQ2OZI1GFrpisptIw71U3hJ+n9iRh+vFTwduu0ffFGXwor+a5d2jVUccVQbsgy/4stuo6lpeSXDtyeWUIjirPIT68LcCHQMI3IRKjbi2T9lpPeIC/IS1i9+t2E5DT2j7d65YMbJ8us0DD0MbE2ar7/vQ1HXo76j/7WFzS/wY4rHd/7R1DhFqCUFOWhtTJgkzN9o2YB3QODsB+lGIjz2AuJEapOhFJNIak0WrQ0HHq+Kv6vo7jyFP0Qh5WShvwtnGARaiPFY03wVrSahzQyxbjI+D6bsfox17I1VB6SCRBLmsPRyfL7twmo18Lhi/TF0OQslCX4D4lc1u4wJT/sun6ZfAfHvlvTvRtkbdHSA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000008, 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 Thu, Aug 15, 2024 at 6:46=E2=80=AFPM Hao Ge wrote: > > Hi Suren > > > Thank you for taking the time to review this patch. > > > On 8/16/24 05:00, Suren Baghdasaryan wrote: > > On Thu, Aug 15, 2024 at 12:21=E2=80=AFAM Hao Ge wrot= e: > >> From: Hao Ge > >> > >> When enable CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK, > >> the following warning always occurs,This is because the > >> following call stack occurred: > >> mem_pool_alloc > >> kmem_cache_alloc_noprof > >> slab_alloc_node > >> kfence_alloc > >> > >> Once the kfence allocation is successful,slab->obj_exts will not be em= pty. > >> Since in the prepare_slab_obj_exts_hook function,we perform a check fo= r > >> s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add func= tion > >> will not be called as a result.Therefore,ref->ct remains NULL. > >> > >> However,when we call mem_pool_free,since obj_ext is not empty, > >> it eventually leads to the alloc_tag_sub scenario being invoked. > >> This is where the warning occurs. > > Ok, I was trying to understand why "Once the kfence allocation is > > successful,slab->obj_exts will not be empty.". alloc_slab_obj_exts() > > has to be called to create slab->obj_exts. Normally that is done in > > prepare_slab_obj_exts_hook() which has the s->flags & (SLAB_NO_OBJ_EXT > > | SLAB_NOLEAKTRACE) check and the cache that kfence_alloc() uses here > > is the object_cache (passed from mem_pool_alloc()) which has > > SLAB_NOLEAKTRACE: > > https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kmemleak.c#L453. > > Therefore, prepare_slab_obj_exts_hook() should have skipped it and did > > not create the slab->obj_exts. So, it must have been called from > > account_slab() or __memcg_slab_post_alloc_hook() to create > > slab->obj_exts for memcg accounting in slab->obj_exts.objcg. Ok, now I > > understand why you need the CONFIG_MEMCG & CONFIG_KFENCE & > > CONFIG_KMEMLEAK combination. > > Please confirm that slab->obj_exts creation is happening the way I > > described above and for those reasons and if so, please update the > > description of this patch to explain that. > > It's mostly correct, but in the context of kfence, the assignment > actually takes place within the kfence_init_pool function. > > https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kfence/core.c#L606 > > So in version 2, I have included this explain "because it has already > been assigned a value in kfence_init_pool." > > >> So we should add corresponding checks in the alloc_tagging_slab_free_h= ook. > >> For __GFP_NO_OBJ_EXT case,I didn't see the specific case where it's > >> using kfence,so I won't add the corresponding check in > >> alloc_tagging_slab_free_hook for now. > >> > >> [ 3.734349] ------------[ cut here ]------------ > >> [ 3.734807] alloc_tag was not set > >> [ 3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:= 130 kmem_cache_free+0x444/0x574 > >> [ 3.735866] Modules linked in: autofs4 > >> [ 3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G = W 6.11.0-rc3-dirty #1 > >> [ 3.736969] Tainted: [W]=3DWARN > >> [ 3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2= /2/2022 > >> [ 3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS B= TYPE=3D--) > >> [ 3.738501] pc : kmem_cache_free+0x444/0x574 > >> [ 3.738951] lr : kmem_cache_free+0x444/0x574 > >> [ 3.739361] sp : ffff80008357bb60 > >> [ 3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 000000= 0000000000 > >> [ 3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff00= 00c0005700 > >> [ 3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff80= 0081756000 > >> [ 3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffff= ffffffffff > >> [ 3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff80= 0081756df0 > >> [ 3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d37= 3038343337 > >> [ 3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 000000= 00ffffffd0 > >> [ 3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c00000= 00ffff7fff > >> [ 3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 000000= 0000000001 > >> [ 3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 000000= 0000000000 > >> [ 3.746255] Call trace: > >> [ 3.746530] kmem_cache_free+0x444/0x574 > >> [ 3.746931] mem_pool_free+0x44/0xf4 > >> [ 3.747306] free_object_rcu+0xc8/0xdc > >> [ 3.747693] rcu_do_batch+0x234/0x8a4 > >> [ 3.748075] rcu_core+0x230/0x3e4 > >> [ 3.748424] rcu_core_si+0x14/0x1c > >> [ 3.748780] handle_softirqs+0x134/0x378 > >> [ 3.749189] run_ksoftirqd+0x70/0x9c > >> [ 3.749560] smpboot_thread_fn+0x148/0x22c > >> [ 3.749978] kthread+0x10c/0x118 > >> [ 3.750323] ret_from_fork+0x10/0x20 > >> [ 3.750696] ---[ end trace 0000000000000000 ]--- > >> > >> Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab all= ocation and free paths") > >> Signed-off-by: Hao Ge > >> --- > >> mm/slub.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/mm/slub.c b/mm/slub.c > >> index c9d8a2497fd6..1f67621ba6dc 100644 > >> --- a/mm/slub.c > >> +++ b/mm/slub.c > >> @@ -2116,6 +2116,9 @@ alloc_tagging_slab_free_hook(struct kmem_cache *= s, struct slab *slab, void **p, > >> if (!mem_alloc_profiling_enabled()) > >> return; > >> > >> + if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE)) > >> + return; > >> + > > Please add a comment before this check saying something like: > > /* slab->obj_exts might not be NULL if it was created for MEMCG account= ing. */ > > > > Other than that the fix seems fine to me. > > Thanks, > > Suren. > > > OK,I have already made the modification in version 2. > >> obj_exts =3D slab_obj_exts(slab); > >> if (!obj_exts) > >> return; > >> -- > >> 2.25.1 > >> > Thanks For all fixes to allocation tagging please: Cc: stable@vger.kernel.org # v6.10 Thanks, Suren. > > best regards > > Hao >