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 F2072CEACD5 for ; Tue, 1 Oct 2024 14:50:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6EEC528008F; Tue, 1 Oct 2024 10:50:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 69EBB280068; Tue, 1 Oct 2024 10:50:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5670428008F; Tue, 1 Oct 2024 10:50:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 379FD280068 for ; Tue, 1 Oct 2024 10:50:28 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id A0E7BA0E91 for ; Tue, 1 Oct 2024 14:50:27 +0000 (UTC) X-FDA: 82625319294.20.C3C052A Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) by imf15.hostedemail.com (Postfix) with ESMTP id BE368A0006 for ; Tue, 1 Oct 2024 14:50:24 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=jtMPFSGp; spf=pass (imf15.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.167.49 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727794098; 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=3cSyeR9abJDVNh3IbE3iMwWTMv0KT43vGwZgDBM7Wbg=; b=tl5LNE/t2WJFR/l9gC+rK/HYjeg2HySsdBID4IcPv2PV5MuLN7wIsom8Ik9DT8498cBi9B KIuMdYibiJjblQsdF+quDKKE8xO68cHRtRo7kx2hyvIma6WCNV82OmjrQRsUY1X3bYfqbY PL+yq1eKI+gzYdTlvfHXJiu9OV4UwMs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727794098; a=rsa-sha256; cv=none; b=iN1PMD53Xil1GLWS3u68TnBqQYFh3FZF7mW3NGLovvsRO2ywlRHLqbdDdYRhEirHH2VWRs ugERvflAhHCfYfsDpOqS3lb3P5wuLyar//jFSigetTbUkC5nRpiX/1Ov8+UMQcVcojp5SR /b3p6/U2X4z33OcFQOvr7pwPxguH71Q= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=jtMPFSGp; spf=pass (imf15.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.167.49 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lf1-f49.google.com with SMTP id 2adb3069b0e04-53993c115cfso2845134e87.2 for ; Tue, 01 Oct 2024 07:50:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727794223; x=1728399023; 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=3cSyeR9abJDVNh3IbE3iMwWTMv0KT43vGwZgDBM7Wbg=; b=jtMPFSGpMalhK58Q2x6uaX2qEO4gOCRnKh5G9oRH6EIITAc4su/BQ5kobjkWRl66tC kkO12F+Id+U8po5FZge02GhI38mfhieMdzd54pbyXRDcboHHAbRvtdXPgNNLYv83coEA +DVFc+78pmZwcW0CIJDd+5saOC+K1bwyJ+BKWe9z9rpBaZTpCvb53t8/e21Tc4GMGiDv Vgbv2L032OZFTyrdjiXNMvRnQ35ooyBzJTd/wcIy2G15I+2xgkW1+7Hjs49V+CKyh92x 3WlFbXFaqZ0eZrscUk+KUzGlfqWUz+oEeb1AC6y7pi7dZt3RV/RAAbFzPDnvNZhYbl5q 3qBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727794223; x=1728399023; 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=3cSyeR9abJDVNh3IbE3iMwWTMv0KT43vGwZgDBM7Wbg=; b=UX4nPMzuJLKkKvBzKiREHRni7+mKXznm0aC40xx/zviZBzNUBPuUUdXUZ1tSvMYUr7 4OK6RS7As5J17J4XNVwyc5+3+7WClmNGM+rqIy/xibBRrSc37ZLLwlotIh3VbEAqVMWN eEU7832iGRd69ORtr2+fDnBdM1N1hbVvSiNpxq+9+ALlg3+TlTUIiGxaFsqYAluJbDIS +PFClOYFkBByUDXJ9rmpkBPTFLRpLQFJKlR7YYcxOnyMJluVacMQWZlb57ocAuzcCeEp Zzvu1lo289JHRpBprTn0CA74GUbUyngd9mvWeKWIupWX8h6VfH23BUTJWq1mMXgNkuem KvsA== X-Gm-Message-State: AOJu0YzrlbFTFy3wDKQdc9fX0CV1piaRAtMnBgTassWjaXdb6vbRA02/ TUEQ+hxbwXTmBEGJbJS+dxQo45XL5hcA9aPDKDnGiswtPACDt/BHKV0gT/m7JdjZ/wSm8vlRpu6 FKZ1J8FD4Rp1Hj/ohRLT9kdQ2djA= X-Google-Smtp-Source: AGHT+IHjd7sSKcg/ku9VJaZ3qqa0ntw0KjgjijNTNSnxnRp33GMi/Fp+m2pyySF0Hqc8zWcZ4+YnGRENriOE71Izr24= X-Received: by 2002:a05:6512:3089:b0:52e:7542:f469 with SMTP id 2adb3069b0e04-5389fbc6f0amr7029314e87.0.1727794222640; Tue, 01 Oct 2024 07:50:22 -0700 (PDT) MIME-Version: 1.0 References: <20241001140245.306087-1-nilay@linux.ibm.com> In-Reply-To: <20241001140245.306087-1-nilay@linux.ibm.com> From: Hyeonggon Yoo <42.hyeyoo@gmail.com> Date: Tue, 1 Oct 2024 23:50:10 +0900 Message-ID: Subject: Re: [PATCH] mm, slab: fix use of SLAB_SUPPORTS_SYSFS in kmem_cache_release() To: Nilay Shroff Cc: linux-mm@kvack.org, linux-block@vger.kernel.org, vbabka@suse.cz, akpm@linux-foundation.org, cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, roman.gushchin@linux.dev, yi.zhang@redhat.com, shinichiro.kawasaki@wdc.com, axboe@kernel.dk, gjoyce@linux.ibm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: BE368A0006 X-Stat-Signature: qy6yzazoy79ew9t9tpcfp86mn89sgej1 X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1727794224-664327 X-HE-Meta: U2FsdGVkX1/cdQA8+Gndv6UVqR+Q3vdOq+greFTIL11YaKrfevdpoqvoCBIwW7TzqQWEhRs8bBiVd1uTSZeySAMcuvcLZtN6tE0I6wk1t9Yqr6cRyTjops8UC5SskhlRfKpEEWT2uWKv3IoPvsG9XUhwu4tj6IT71B4xs2iCmp+vYw6OwaiQGKrYCl95KYVFh59DAAToAAHT64XHknmiyEf80llyR21imWj6U/1klEnONDO1aT80+wt9jAutqEespDYjcb0qREq9CGK8nQU3ol6xvbxKZPR2TkRaPru9Mf7w5KYDDpDAVsfpySwi+vqKi/Nj8A1WxfymNS1XphuUdj9dv5l55/pzRC+LQIFVHVoMBbRoOubn8Eon9cUvw5eJxYX1lgM2h67gimES+7SiF2vjkxdPcC2yupGQaiH/gM/8Y3ohcA59+uLVBI1ZZ/hSTesOAydvXUM48sm7Wa0AUhkTYymobe0gnYOheoXgc/FNEruQk3lReXHd2fdmKaIr6Fn2Nu+gzlgtnQTLp6F0JiKMcg8eSqRsqOebdynUt7oRoLYaiXZ36sfpDRpq1ANEXV3JiF4HITiYL6ALBxGUs2SjKBoh3pdjxB63UU88SqseuEaxn1TmFdX1Kdp5Rlj6bfydVNB0vutvhm3lo3tER4vDpcwwWMaTe2u7i1ZzgA6d3Wku4YgceUO3vxVTVEC+Uvm8hmafKgAOwfDz2eGXKXGkCFlInqehavzDU2mtmSIiYypycUSlePzUhPxcVJrBt0wpknUZGWjoFmazN2ZOxk2t4LR+VUBoxO5h2MAun8Gl0a/1npOBG/tYHtwo4U99U4bW2Dd/Nxhkj/szwt3zOryO286zS2BBnGLAmkvoyZq+JLBa9gDLKmMA2V7Eoh1gCzwBrfwgolSMZYW9bNc0hqC5r7aypN90LfsD19WaQvgN1q/3zzCB05rW+BwyYNTfKeojIGvKbhNjOCYb+uV KECVZcKG t27e5tXt7K8o266UqOZochGVvH9gNpdcxkd11XzSxLSzwa9STtHnChKKalybwcFr7YWePEEQWYBgRie1VzaTfXmWCqhS1T8qrqMQomJQ235Zj2YI5W3BuCDdy843sL8UBxQwWK9N1kXrxBbctgL9gJ5xvXvUuECynEc2h6yigJcneMVBWCT9UDW5Mq9ZO8yOs746DX2vevAG5juXIOksO8XubtQj0l3Yj5lo20H4Gzf44IcRB/hiQbPeKeGakL5+9O2Eycf5wLK0IRHDAAj8XKf8WuXqd5lXFypEM6jig+pd7Q4wz6JsASRyhMpXj/JTq9K+T7xX8nFYkp+xCrqJcqhnhdkIKYenyIMsuUNAVOtlsUXXkPIJ40Vq8B0AtJVaU9+MWSB5a58h3AF83hTf8Ks0MJsBtgT+8w8Ze6YPJCaYAbYBGt5jVa0YtVnHp03jAi9mlJPxoR8iMU4/5agosEvm36C3q+Wpc47UMt5hr3eF8D6g= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000242, 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 Tue, Oct 1, 2024 at 11:02=E2=80=AFPM Nilay Shroff = wrote: > > The fix implemented in commit 4ec10268ed98 ("mm, slab: unlink slabinfo, > sysfs and debugfs immediately") caused a subtle side effect due to which > while destroying the kmem cache, the code path would never get into > sysfs_slab_release() function even though SLAB_SUPPORTS_SYSFS is defined > and slab state is FULL. Due to this side effect, we would never release > kobject defined for kmem cache and leak the associated memory. > > The issue here's with the use of __is_defined() macro in kmem_cache_ > release(). The __is_defined() macro expands to __take_second_arg( > arg1_or_junk 1, 0). If "arg1_or_junk" is defined to 1 then it expands to > __take_second_arg(0, 1, 0) and returns 1. If "arg1_or_junk" is NOT define= d > to any value then it expands to __take_second_arg(... 1, 0) and returns 0= . > > In this particular issue, SLAB_SUPPORTS_SYSFS is defined without any > associated value and that causes __is_defined(SLAB_SUPPORTS_SYSFS) to > always evaluate to 0 and hence it would never invoke sysfs_slab_release()= . > > This patch helps fix this issue by defining SLAB_SUPPORTS_SYSFS to 1. Hi Nilay, Thanks for your effort in investigating the issue and fixing it! This makes sense to me, but is there any reason the code avoids using IS_ENABLED()? I think technically either IS_ENABLED() or __is_defined() (with your fix) would work in this case, but it made me think "What is the difference between IS_ENABLED() and __is_defined()?" IS_ENABLED() is already frequently used in mm and only few code snippets us= e __is_defined() directly. Best, Hyeonggon > Fixes: 4ec10268ed98 ("mm, slab: unlink slabinfo, sysfs and debugfs immedi= ately") > Reported-by: Yi Zhang > Closes: https://lore.kernel.org/all/CAHj4cs9YCCcfmdxN43-9H3HnTYQsRtTYw1Kz= q-L468GfLKAENA@mail.gmail.com/ > Signed-off-by: Nilay Shroff > --- > mm/slab.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/slab.h b/mm/slab.h > index f22fb760b286..3e0a08ea4c42 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -310,7 +310,7 @@ struct kmem_cache { > }; > > #if defined(CONFIG_SYSFS) && !defined(CONFIG_SLUB_TINY) > -#define SLAB_SUPPORTS_SYSFS > +#define SLAB_SUPPORTS_SYSFS 1 > void sysfs_slab_unlink(struct kmem_cache *s); > void sysfs_slab_release(struct kmem_cache *s); > #else > -- > 2.45.2 >