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 EEA10CEACDD for ; Tue, 1 Oct 2024 15:51:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 804F4280098; Tue, 1 Oct 2024 11:51:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 78C31280068; Tue, 1 Oct 2024 11:51:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 65548280098; Tue, 1 Oct 2024 11:51:23 -0400 (EDT) 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 42752280068 for ; Tue, 1 Oct 2024 11:51:23 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9ADAD4159B for ; Tue, 1 Oct 2024 15:51:22 +0000 (UTC) X-FDA: 82625472804.21.A9BA178 Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by imf21.hostedemail.com (Postfix) with ESMTP id B79171C0016 for ; Tue, 1 Oct 2024 15:51:20 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=GIc9+mqe; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf21.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.167.44 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727797860; a=rsa-sha256; cv=none; b=SvDXrlgzh0nV0qTaY1P4ZByUe3JkFtRh4JpIhMkz7dyTwxz1eOrg+02m/mWRkRIFYnicck CUicKYXjvofBPbFfEbofafxXKtCce8OmIE7/HMAkE9cVNot14x0uUtJ0IHiJf+a1HZLC/W BRTXljAkuWXq6/FUsNKPJLPFZ0XnD64= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=GIc9+mqe; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf21.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.167.44 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727797860; 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=6LcFytHts4BnD6TJGHZBuIYtm74DnB7NL+eN3GOhUgo=; b=FSfWNTx3NIRoWlrnvnJsdiIHk1ffvMoYOoUpZbRE66Z094LT11IkGQzfy0+bPzAfa64f4z K7YRkyMUPU/ELgLtylw5Xx1O9ldZhHFc7puxl/61zBVqOM6brGDh6at1lVOTaHjqTCpoOn kriSfBclooQDv2C7EzYrYddR+potafw= Received: by mail-lf1-f44.google.com with SMTP id 2adb3069b0e04-5398f3be400so3276486e87.0 for ; Tue, 01 Oct 2024 08:51:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727797879; x=1728402679; 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=6LcFytHts4BnD6TJGHZBuIYtm74DnB7NL+eN3GOhUgo=; b=GIc9+mqePBCHutkEUz3LLzGrSJJ2M5aWADs7p6vncly17HDomGc4osa32p4tZTznFU Mk9935u9O7I0+MhjOxV0PXtnckspzB+Z4YTZUxo7gbyGBcVcJeNxQ2tvL/RF65q8bdRn VeNb6eNvWC+q5H9bFimPatchMaZ8gz3I0ufYPq0djKF5Ztie0iwW0P0GX5kkgUWiBu3T KXVSMi1KoZAZe+e0rF4grZyIS5Y6E9qu7qofGyhTlxAE3ubXYYJNG/GrfM+G453IY1D3 pyQL7/OUd4vGKYJHI1Rd3BvhzSjL7fVOWhZfbO15QIYaZDXV7cRK1/TiaxZLRahzuJzM x/Hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727797879; x=1728402679; 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=6LcFytHts4BnD6TJGHZBuIYtm74DnB7NL+eN3GOhUgo=; b=nNFc8ZM8YlMOVG4vmoSiXv0cZgxGrdSn875NCYeDeomnfhPR5+/2aMNHWPwmDp253m FnWN+YLQfjjKfNcyKX2Cuh1hrY3lg/A76aOQBLKFVZwUDB8OYWSR7eEe5aX4dqTpH2Pd mEgJIyQX+AMPuBegRKmq/p6KX9NzwJcoPOvrdiksAM/sSuV69qmomZ0ov0Z+f66GoiVX UJKjh4vnpRstOXVU7TBROGqHjFCCX8zugS1WaQ165B9yfKczkmC4MEly+KJph7geBSJV em36Uq9/Q0lGCJpsOLtX5E78es+wT5RJxGCkj2joqrn0O6W17CzNUqZUDDBqbw5ZLsPr XQNw== X-Forwarded-Encrypted: i=1; AJvYcCXNBAEVKU74+/AeJMXXPOIdGrZgWXExRXeacd0I6PYdNhyd19DoR0Di7G8RkvKaNwRI5f4RV/wI2Q==@kvack.org X-Gm-Message-State: AOJu0YyKWjq2w9hHoXnddMQs44Gw7IIeA4IrzchDisQ6tNzn9wkQsXUq jTMp1S+tqHMrEm/Mc0qLur9Z/FKx29s+rW+Qc6zX5oCvtGQr5hE0tcsXgPw1MFrDktY/j1UzBlh bkogTtCzGSu2YwUAM9J//rCYmVQU= X-Google-Smtp-Source: AGHT+IGYUosvjqy+TahmXf0yE0JcV9EQPkhMBKu30bZKmaEaFqD/fVW10d/wn3WrEueJEc2en4G+5aHhPXb+QzTZnxo= X-Received: by 2002:a05:6512:3c8f:b0:536:53b2:1d0d with SMTP id 2adb3069b0e04-5389fc30f11mr7941711e87.9.1727797878683; Tue, 01 Oct 2024 08:51:18 -0700 (PDT) MIME-Version: 1.0 References: <20241001140245.306087-1-nilay@linux.ibm.com> <73e499a7-6237-4f67-84c1-d6434b89df26@suse.cz> In-Reply-To: <73e499a7-6237-4f67-84c1-d6434b89df26@suse.cz> From: Hyeonggon Yoo <42.hyeyoo@gmail.com> Date: Wed, 2 Oct 2024 00:51:05 +0900 Message-ID: Subject: Re: [PATCH] mm, slab: fix use of SLAB_SUPPORTS_SYSFS in kmem_cache_release() To: Vlastimil Babka Cc: Nilay Shroff , linux-mm@kvack.org, linux-block@vger.kernel.org, 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-Rspam-User: X-Stat-Signature: urm7re6zb7hhmxf5qegsz36w3x88hcbi X-Rspamd-Queue-Id: B79171C0016 X-Rspamd-Server: rspam02 X-HE-Tag: 1727797880-408452 X-HE-Meta: U2FsdGVkX19Jo3ORPFvpWb6+J7xoDpgKp6lhnrCB8okAcvS1uKUccUrZCDXVneDW5fotF7+JwgSTMvGMcKfzF7+jWRPybCRkYvGlUdwbfH905NvDhlEs+wyqyJTB26PFmJMbtSP1Uv/yb3qCiotwGwYwA5F8O/FgzDVm1jtf0tqE2l3vOGItQFf2UBj5xScXnFmVUvTMn5MixRaEqbqumIlbmy48K+35vFBwTAWapJJhAQ1xZ9vK3lwRlphDW38O/+MOWohgGP81sp5JkwpyIGMAIVCRCCNgh8K4Vvv0pIRc2DZjvaqEbBuVrZTm6/C1SfXOD6tmtwr0aKEDjUDLIRo4Gh0lvj1mLqiibdoVe4vtVfTYyEQismT3A7thP46WG5d7NHKlURUM0Ky8fGhd7TaBDweDJKqod9qOupihZi6Dohpd05Bs3EPu4P9kssHBOcfG4noEcrghck3rF+bzmscV3kk18SBZw3Fu5bVw7kr9fKIFOlcZO9NnzHF/ALJGAbL7St/ycl+QbDk+aLyLMfUOj3lDwJg+/ReofwqYDnhCpx8IcUy5HU7jQLGaIcsssxVN/j5etrC+zIY5i56c2xfl2Fhhc3uOQ7rwla0aMTdn/Jj/QPt9vJyofn+vapBGpwoNUb+WYthWN2PFOI3O2IBKQSN/UkaSPRojUNFMY+atxyGk3hN/VDIFcLuA9r8wRbHtvkd03YM8Oum2Da/noBAOE2Ecme8U3VW0H2SaRS0cZSoaGfY2mfAFJHI/78sugPShmQoIg3OwUjESOEAjsVSRI7LBYK5o1Y3ufM2mfse5YtQJH3SRcJhgg2/5jrAjO7x+TCJckM6Uheu3kbNi6cKUgCWMZSIV+huM3fVEICafBPn9JLE/g5gUC9oe7ph3da7RCmAoAxnQhK2XcSenwLOGn/2h0MKO0NmzDxb14SttastAwD2balrOiakznftuS406eAEyTA8jtqzxnBO aHxSUi2Q SuoD+zEnvfpbztOepksKWedexDy8ZjIgdByJ1sxsrIKjKQIDLa5BIkRzKe0rHCjPieBHQM4cc9Qig/dlemvaHT0Zdq3yfA1wlWkGoBDAX36qUnRsnsr5TtjPq+wO/C2vDQ7Adz7xavTjrehZuxrGfkeqVR8xRfe1AhPf9RS0PAZNMEyfXZwCuopnm7nzOW0ul8GStIvr2dAKB99tdeKABu/jhSDw+6cLdv/GIxW9w8ImyiUpxXYk8p3Yhr4oeLuglyug/945aeM1PEc3/3EsPx64CbX1ZeTXxOrY+KFKEJTzQKl6mdf7TyqwY0zVFrBU1s5LrcUwg5tnUPiV9DbB6DmSCnPGga0TVcClogaYB/Tg2CfBIdRsdqmgODuv8HsHF+KfbxzFZGB8ov0OzuFpbghaqw9947AZbn8hw6M8JZNU27Icft3kd/RUX2SZTUxlWK7oMgIcRR4lYuc2tNT7j5qxDi7NMvLdla4z6SifGYKDiImnTeO+hf7/jFA== 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 Wed, Oct 2, 2024 at 12:39=E2=80=AFAM Vlastimil Babka wr= ote: > > On 10/1/24 16:50, Hyeonggon Yoo wrote: > > 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 whi= ch > >> while destroying the kmem cache, the code path would never get into > >> sysfs_slab_release() function even though SLAB_SUPPORTS_SYSFS is defin= ed > >> and slab state is FULL. Due to this side effect, we would never releas= e > >> 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 def= ined > >> to any value then it expands to __take_second_arg(... 1, 0) and return= s 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_releas= e(). > >> > >> This patch helps fix this issue by defining SLAB_SUPPORTS_SYSFS to 1. > > Oops, thanks a lot for debugging and fixing this! > > > > > 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 snippet= s use > > __is_defined() directly. > > I was wary of using IS_ENABLED() because that's intended for CONFIG_ macr= os > and SLAB_SUPPORTS_SYSFS isn't one, so even if it worked now, it wouldn't = be > guaranteed to stay working. Oh, you are right. After looking into the history, __is_defined() is actually intended for non-config macros. With that in mind, the fix looks good to me. Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > Best, > > Hyeonggon > > > >> Fixes: 4ec10268ed98 ("mm, slab: unlink slabinfo, sysfs and debugfs imm= ediately") > >> Reported-by: Yi Zhang > >> Closes: https://lore.kernel.org/all/CAHj4cs9YCCcfmdxN43-9H3HnTYQsRtTYw= 1Kzq-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 > >> >