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 DE5A4C3DA70 for ; Tue, 30 Jul 2024 18:52:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 733426B0099; Tue, 30 Jul 2024 14:52:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6E3416B009A; Tue, 30 Jul 2024 14:52:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 55DC46B009B; Tue, 30 Jul 2024 14:52:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 37BBA6B0099 for ; Tue, 30 Jul 2024 14:52:13 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id DAC1A14047B for ; Tue, 30 Jul 2024 18:52:12 +0000 (UTC) X-FDA: 82397314104.24.3306EC0 Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by imf24.hostedemail.com (Postfix) with ESMTP id D3D8918000F for ; Tue, 30 Jul 2024 18:52:09 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=samsung.com header.s=mail20170921 header.b=dWuKcRby; spf=pass (imf24.hostedemail.com: domain of m.szyprowski@samsung.com designates 210.118.77.12 as permitted sender) smtp.mailfrom=m.szyprowski@samsung.com; dmarc=pass (policy=none) header.from=samsung.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722365475; 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=XUNVEVBUHu1MPJWO59vj8nIO541Z7GC1J8wR3j5KZH0=; b=4pxeRwvOM9t3JPkHglwYgECJYGdJRO7cHbc6SAzAm/kK6Acst2kDTqI9QDX0GiYjbvxNBx rrnrUISU3gRsN3n6vfv5hv8j2JlHAu1XctwMG/VW6tMxJeZmrGZKQJhjdSLp1f4RMR/LtQ W8AdEWyfuBNGWoOOG5Kafi8M6M+4ARo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722365475; a=rsa-sha256; cv=none; b=ccECbchimzHi6EDCEiAslVv/6BMu/dLWjG5lF0gYJG426CKJZuxJAxNQpzH4p0sSEEus7c Gct4VqwEgw/LgC0BneJpBEg0M+eRGdXnzibSOxogzlxewsp1tnaaEeED8MwkwClhyZkUOm B55W7LhgI+ReVyI4YvX6huo2661gwAk= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=samsung.com header.s=mail20170921 header.b=dWuKcRby; spf=pass (imf24.hostedemail.com: domain of m.szyprowski@samsung.com designates 210.118.77.12 as permitted sender) smtp.mailfrom=m.szyprowski@samsung.com; dmarc=pass (policy=none) header.from=samsung.com Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20240730185207euoutp026e5da0194abe5d4ea279fd3af2d0e4b5~nElJHLxg30676106761euoutp02D for ; Tue, 30 Jul 2024 18:52:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20240730185207euoutp026e5da0194abe5d4ea279fd3af2d0e4b5~nElJHLxg30676106761euoutp02D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1722365527; bh=XUNVEVBUHu1MPJWO59vj8nIO541Z7GC1J8wR3j5KZH0=; h=Date:Subject:To:Cc:From:In-Reply-To:References:From; b=dWuKcRbyK7oE4ShiHJAB/gfNCKccE9hMoqAKBUxctqefPzXYJWr9HWStN+N7MsL/i 0UWFGyUrormx/TbE/Y9G+hazj6O8nsPfoJbucR7J+30eTaKomrJ6VQPfLBtnhOA2A8 Z4v7PHnUvc+qa8x7zQnlGOEdjMVNs7AgEcPCzmLs= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20240730185206eucas1p1df436db2791401ea0be3eae5ebebaac2~nElI_oNaC2664426644eucas1p1c; Tue, 30 Jul 2024 18:52:06 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id A2.9C.09620.65639A66; Tue, 30 Jul 2024 19:52:06 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20240730185206eucas1p28b14a1d9802ce2703bd13edc75e1b55d~nElIXt5sj0112901129eucas1p2c; Tue, 30 Jul 2024 18:52:06 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20240730185206eusmtrp13b917d5a00e2d87d1ffa7e475320f451~nElIW_h_C2512925129eusmtrp1e; Tue, 30 Jul 2024 18:52:06 +0000 (GMT) X-AuditID: cbfec7f5-d31ff70000002594-9f-66a9365685ef Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 35.E4.08810.65639A66; Tue, 30 Jul 2024 19:52:06 +0100 (BST) Received: from [106.210.134.192] (unknown [106.210.134.192]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20240730185205eusmtip254116d38b5380f42798cb0aecb28208d~nElHe4d8e1554215542eusmtip2E; Tue, 30 Jul 2024 18:52:05 +0000 (GMT) Message-ID: <3c4b978b-b1fe-42d2-b1a7-a58609433f3c@samsung.com> Date: Tue, 30 Jul 2024 20:52:04 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] mm: kmem: add lockdep assertion to obj_cgroup_memcg To: Muchun Song , hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev, shakeel.butt@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org, vbabka@kernel.org Cc: cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Language: en-US From: Marek Szyprowski In-Reply-To: <20240725094330.72537-1-songmuchun@bytedance.com> Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTcRTH/e3ezW02uU3Ng5bCSMkyNynrUhIWlSuNpCgjglx5p5KabGlp GSsjH1ith83mqwep2Zwy0lKTcqWb6HxGqWUj0sTViLQkTaxt18r/Pud7fud8zzn82Bi/genF Tkg+QcmSJYkCFhevb5vuXr1/3QOpSKEVkcU1GhY5UFGIkw91u8j+xmIW+V7zm0l+nirHyPKG Dzh5VeNBjqlLGGSLuQsjc8bNeJiLeM6oYYlffPmKiXVVuSyxbuKas9hY+AsX31EUYOJJnU+U 80FuaCyVmJBGyYSbYrjxUwWjzJR76089aq1lKlDn6jzEYQOxFmrvXsLtzCcqEcx+jM9DXBt/ RzBccQOng0kE7S8LmH8rdO2lTDpRgeBS6XkWHXxD8HhK4ejFIzbBXPEHB+OEH9SoCpxpfTG0 3xpx6B6EL5iHCh26GxEBbZZeB7sTnQi01WftjBG74dabBgbNnjA0UuZgFhEMedY8lp05Nq93 Y1UY/cYXsuqKMPtAQGRzwHLR7EyPvRUuWEcRzW5gMTya15dCx/V8fL4Awe1fZgYdKBEoxobm KzbCu64Zmx3bZhEANY1COwKxGVRGjEZXGLAupmdwhWv1qnmZBzkX+XQPf1AbtP9cW3r6MCUS qBdcRb1gS/WCbdT/bW8jvAp5UqnypDhKviaZOhkklyTJU5Pjgo4eT9Ih2+/qmDP8eIIqLd+C 9IjBRnoEbEzgzjv86r6Uz4uVpGdQsuOHZamJlFyPvNm4wJPnF+tL8Yk4yQnqGEWlULK/WQab 46VgSLFPV4u7rTJ50aqYJo+ihh1Lwmezi175VaWXins+1ZvQg5+a9cOHuqNTcjGrdN9WbV/z 7zOm8P6lz1RaTkpdSFmaaNlU//PY19OMtxE/wkKCD3hFY0bhm+qZnXXZy8dP+mSoQ7a7NJaX SL1DmyZWBe69/FqQJMz0jq4uE+3LTA+b5Q42irRZvU0xaU+2ud5URi55OxjZETCS04wFuC0v EY6eK1GZlMv2bImMOOLvNKn0MO4p9/kITj2lqXdIcsuwW6j2dNSOrCOLKmdymTpRYKYlUaUy GJy0GeGua1sjq4V8YjjKFKjXT2RM529wb6td4XPF5E88tf4cMpnyDQECXB4vCV6JyeSSP33B 6e3MAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrDIsWRmVeSWpSXmKPExsVy+t/xe7phZivTDLb+Y7GYs34Nm8WN5TNY LFZv8rW4vGsOm8W9Nf9ZLV5/W8ZssWznQxaLiWtELZ7PmstkcfD+OWaLjpf3WRy4Pf6dWMPm cfjNe2aPTas62Tw2fZrE7nFixm8Wj4UNU5k9Pm+SC2CP0rMpyi8tSVXIyC8usVWKNrQw0jO0 tNAzMrHUMzQ2j7UyMlXSt7NJSc3JLEst0rdL0Mv4NvUpa8Fi84otRzewNjCe0e1i5OSQEDCR 2HRyHiuILSSwlFHi+2J3iLiMxMlpDawQtrDEn2tdbF2MXEA17xklbu3ZzwSS4BWwk/g35yEL iM0ioCqxfvpUdoi4oMTJmU/A4qIC8hL3b80AiwsLeEsce3WRHWSQiMAZRomnO9aDFTEL+Eo8 nn2RGeIKW4nv+3eyQ8TFJW49mQ+2jE3AUKLrLcgVnBycQIvvPF/FDFFjJtG1tYsRwpaXaN46 m3kCo9AsJHfMQjJqFpKWWUhaFjCyrGIUSS0tzk3PLTbUK07MLS7NS9dLzs/dxAiM1m3Hfm7e wTjv1Ue9Q4xMHIyHGCU4mJVEeOOvLE0T4k1JrKxKLcqPLyrNSS0+xGgKDIyJzFKiyfnAdJFX Em9oZmBqaGJmaWBqaWasJM7rWdCRKCSQnliSmp2aWpBaBNPHxMEp1cCUwO+620xuifgM3seV morR09886ar8o/FllUjyAoX2e54CnDuKj4ldMOecaaugk1U8Ra5eR+dJuIoee6nIpfr1D7e4 chvXFDFKtHxW3el/RNXp7r/dyW7fY3ddm5AjukL8+8lt5ZrTz2p/jLgwuf3x8vqnriayL9Xq 8t+8v3Do2bvbvUozJD/M3rVNfufTgKTWF5IbBeZ84S16/fe9qvVO39kK030kP057+MT3X9k7 1us+674t+rkqpH+i1dblIabLG7kUn+3y4K08ZHN9NrtMrdbz5aFzbM3mLfhX8aPUi2OC929t HjMnc7+LE7JzQn7cj733ovCMpuS8p85ft697sWrdm9khHx3dRa7dWfR7qRJLcUaioRZzUXEi AKFiKQBfAwAA X-CMS-MailID: 20240730185206eucas1p28b14a1d9802ce2703bd13edc75e1b55d X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20240730185206eucas1p28b14a1d9802ce2703bd13edc75e1b55d X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20240730185206eucas1p28b14a1d9802ce2703bd13edc75e1b55d References: <20240725094330.72537-1-songmuchun@bytedance.com> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: D3D8918000F X-Stat-Signature: tt9r6qo8ztba9airb4hnmx3gefhsyaya X-HE-Tag: 1722365529-545751 X-HE-Meta: U2FsdGVkX1+d0dv2SKoSHS805puHPAIGysrbzRsgCD7R/LoK8OgGov/IhsRQdjeZVjI7e7Vu2ioAF8ZcWFQiqj8rhTnRogWOYkdbe2GqI0O8U56PYWDbC2qc8zGbKCOBNI0Fnp8uEZpzR4ZXs8nkFm4gdvQ4sSP0AeOZRC6Ug0P95HXIysVbHI2tlRFZ0V/lLAlq9P0LDSc6XOkVuo+HutNsxxyB7dBiqDXeYiE0ulakiiM6s7aYbRHBu/+q1vuIB2AxWzM2kPn5JwaW1lN6AZIf4T84FVF3JRt44ogmQfWvTXVQZxyxWMn7mjCdeM1BIa5CLSLY54D8XWKCdB4A4AgK6cecudIGGoRlS9woaCWnoR17b8fLIffl9NPdD80YzfaiBbI6FRzmTlLdHbHiEJMjokovwgWpnFDwCnbKrdPDNLc6XKGMTIanVTwHTKahTz9n5vceNddgvMcJty+EdONyeBn49ksW9Z4MEeVO89o3qY1Nc87Ren4mhKNQOFe9A2wgTple33CegPiXf+AXVfBRGUwQtAopsu3dNJoo2Ew4tDQTk4VscRwrW2fkqrzbxKkHz26nbbjYJZCmyMjdpFBTNEyGGfubhpqWcxBuxUcOPCC8eNX1xR+95ztY7WCWEJ1KKlQi3rUeeEw6OT8bAT73CTp3wa+BPKiQ1ZdqC66zsgIPMnrpSaojHt/2wd9LZbl0mVwJY6RMnQvDNWjoSx8GwoC8xluWrETj4KTt8onx6LnCCT+iZBqahPV4gTIYYtPsO7/VDwAiVCwZ9MbUE/YmyjnAR8RmRM0X6HoXGav1QbcZhvQ0Tqq5D30Qi9dK0XW88aZiYW4fuV2krmsTbhDbxti+SSvwNomWcYb+X1hITPBKNVaj2cGGg2i7gwrmapdXtarmWwjlEafYuBSlPifk54ObM04/Jq8jtuadfLEyJ5IRHOf6f/eWjWot9/e3ju8KV9LVFj34L58qd6+ 2v9jxPPo xWwqr1MvoMmePH2I58+VCQedivZoFvytDtgkHJbzzwocQTUDi87qFt2tlRQnXG3roPF86Cj30/fijeSBcVq5eJ3JqdgfcHZFFqqA1RQZGcxaPs9vJzACYbdyKPCswVZA0FoXLRrXr8QkT+eZxLfEeUDo/hkHstqqsN4lfIpQ4zA7lj8b2ClCEzhWvDg== 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 25.07.2024 11:43, Muchun Song wrote: > The obj_cgroup_memcg() is supposed to safe to prevent the returned > memory cgroup from being freed only when the caller is holding the > rcu read lock or objcg_lock or cgroup_mutex. It is very easy to > ignore thoes conditions when users call some upper APIs which call > obj_cgroup_memcg() internally like mem_cgroup_from_slab_obj() (See > the link below). So it is better to add lockdep assertion to > obj_cgroup_memcg() to find those issues ASAP. > > Because there is no user of obj_cgroup_memcg() holding objcg_lock > to make the returned memory cgroup safe, do not add objcg_lock > assertion (We should export objcg_lock if we really want to do). > Additionally, this is some internal implementation detail of memcg > and should not be accessible outside memcg code. > > Some users like __mem_cgroup_uncharge() do not care the lifetime > of the returned memory cgroup, which just want to know if the > folio is charged to a memory cgroup, therefore, they do not need > to hold the needed locks. In which case, introduce a new helper > folio_memcg_charged() to do this. Compare it to folio_memcg(), it > could eliminate a memory access of objcg->memcg for kmem, actually, > a really small gain. > > Link: https://lore.kernel.org/all/20240718083607.42068-1-songmuchun@bytedance.com/ > Signed-off-by: Muchun Song This patch landed in today's linux-next as commit 230b2f1f31b9 ("mm: kmem: add lockdep assertion to obj_cgroup_memcg"). I my tests I found that it triggers the following warning on Debian bookworm/sid system image running under QEMU RISCV64: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at include/linux/memcontrol.h:373 mem_cgroup_from_slab_obj+0x13e/0x1ea Modules linked in: CPU: 0 UID: 0 PID: 1 Comm: systemd Not tainted 6.10.0+ #15154 Hardware name: riscv-virtio,qemu (DT) epc : mem_cgroup_from_slab_obj+0x13e/0x1ea  ra : mem_cgroup_from_slab_obj+0x13c/0x1ea ... [] mem_cgroup_from_slab_obj+0x13e/0x1ea [] list_lru_del_obj+0xa6/0xc2 [] d_lru_del+0x8c/0xa4 [] __dentry_kill+0x15e/0x17a [] dput.part.0+0x242/0x3e6 [] dput+0xe/0x18 [] lookup_fast+0x80/0xce [] walk_component+0x20/0x13c [] path_lookupat+0x64/0x16c [] filename_lookup+0x76/0x122 [] user_path_at+0x30/0x4a [] __riscv_sys_name_to_handle_at+0x52/0x1d8 [] do_trap_ecall_u+0x14e/0x1da [] handle_exception+0xca/0xd6 irq event stamp: 198187 hardirqs last  enabled at (198187): [] lookup_mnt+0x186/0x308 hardirqs last disabled at (198186): [] lookup_mnt+0x15c/0x308 softirqs last  enabled at (198172): [] cgroup_apply_control_enable+0x1f6/0x2fc softirqs last disabled at (198170): [] cgroup_apply_control_enable+0x1d8/0x2fc ---[ end trace 0000000000000000 ]--- Similar warning appears on ARM64 Debian bookworm system. Reverting it on top of linux-next hides the issue, but I assume this is not the best way to fix it. I'm testing kernel built from riscv/defconfig with PROVE_LOCKING, DEBUG_ATOMIC_SLEEP, DEBUG_DRIVER and DEBUG_DEVRES options enabled. > --- > v3: > - Use lockdep_assert_once(Vlastimil). > > v2: > - Remove mention of objcg_lock in obj_cgroup_memcg()(Shakeel Butt). > > include/linux/memcontrol.h | 20 +++++++++++++++++--- > mm/memcontrol.c | 6 +++--- > 2 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index fc94879db4dff..95f823deafeca 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -360,11 +360,11 @@ static inline bool folio_memcg_kmem(struct folio *folio); > * After the initialization objcg->memcg is always pointing at > * a valid memcg, but can be atomically swapped to the parent memcg. > * > - * The caller must ensure that the returned memcg won't be released: > - * e.g. acquire the rcu_read_lock or css_set_lock. > + * The caller must ensure that the returned memcg won't be released. > */ > static inline struct mem_cgroup *obj_cgroup_memcg(struct obj_cgroup *objcg) > { > + lockdep_assert_once(rcu_read_lock_held() || lockdep_is_held(&cgroup_mutex)); > return READ_ONCE(objcg->memcg); > } > > @@ -438,6 +438,19 @@ static inline struct mem_cgroup *folio_memcg(struct folio *folio) > return __folio_memcg(folio); > } > > +/* > + * folio_memcg_charged - If a folio is charged to a memory cgroup. > + * @folio: Pointer to the folio. > + * > + * Returns true if folio is charged to a memory cgroup, otherwise returns false. > + */ > +static inline bool folio_memcg_charged(struct folio *folio) > +{ > + if (folio_memcg_kmem(folio)) > + return __folio_objcg(folio) != NULL; > + return __folio_memcg(folio) != NULL; > +} > + > /** > * folio_memcg_rcu - Locklessly get the memory cgroup associated with a folio. > * @folio: Pointer to the folio. > @@ -454,7 +467,6 @@ static inline struct mem_cgroup *folio_memcg_rcu(struct folio *folio) > unsigned long memcg_data = READ_ONCE(folio->memcg_data); > > VM_BUG_ON_FOLIO(folio_test_slab(folio), folio); > - WARN_ON_ONCE(!rcu_read_lock_held()); > > if (memcg_data & MEMCG_DATA_KMEM) { > struct obj_cgroup *objcg; > @@ -463,6 +475,8 @@ static inline struct mem_cgroup *folio_memcg_rcu(struct folio *folio) > return obj_cgroup_memcg(objcg); > } > > + WARN_ON_ONCE(!rcu_read_lock_held()); > + > return (struct mem_cgroup *)(memcg_data & ~OBJEXTS_FLAGS_MASK); > } > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 622d4544edd24..3da0284573857 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2366,7 +2366,7 @@ void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages) > > static void commit_charge(struct folio *folio, struct mem_cgroup *memcg) > { > - VM_BUG_ON_FOLIO(folio_memcg(folio), folio); > + VM_BUG_ON_FOLIO(folio_memcg_charged(folio), folio); > /* > * Any of the following ensures page's memcg stability: > * > @@ -4617,7 +4617,7 @@ void __mem_cgroup_uncharge(struct folio *folio) > struct uncharge_gather ug; > > /* Don't touch folio->lru of any random page, pre-check: */ > - if (!folio_memcg(folio)) > + if (!folio_memcg_charged(folio)) > return; > > uncharge_gather_clear(&ug); > @@ -4662,7 +4662,7 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new) > return; > > /* Page cache replacement: new folio already charged? */ > - if (folio_memcg(new)) > + if (folio_memcg_charged(new)) > return; > > memcg = folio_memcg(old); Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland