From patchwork Wed Jul 24 09:53:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Muchun Song X-Patchwork-Id: 13740806 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 CCB17C3DA63 for ; Wed, 24 Jul 2024 09:53:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 32FBF6B0088; Wed, 24 Jul 2024 05:53:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2B6086B0089; Wed, 24 Jul 2024 05:53:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1309A6B008C; Wed, 24 Jul 2024 05:53:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E84476B0088 for ; Wed, 24 Jul 2024 05:53:26 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8A040A3728 for ; Wed, 24 Jul 2024 09:53:26 +0000 (UTC) X-FDA: 82374183612.18.7E2D8FD Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) by imf10.hostedemail.com (Postfix) with ESMTP id 15155C0007 for ; Wed, 24 Jul 2024 09:53:23 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=LyiQxvHH; spf=pass (imf10.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.214.180 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721814781; 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-transfer-encoding:content-transfer-encoding: in-reply-to:references:dkim-signature; bh=/+Pi9ykJgDXDaJ4tze/4eRzB5JgzMNRwhMMIIYD9f6s=; b=k0YhLD6oQcwOAqoekhHe+3N2Oi5AXhoeiFTMbAeWY3DSIAZTLQYajYdYqHHp/rAv98nZKk D9vvtUlyTCzo91+OtCOmysv24pZ0udvrVroH5bnIvhB3ZeaLRbPp0EGKDgV/Y2ejJu9kn6 L+JCowAONhadBiBrM2E1qnx1qcYNfYQ= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=LyiQxvHH; spf=pass (imf10.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.214.180 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721814781; a=rsa-sha256; cv=none; b=YzTW1k3tXItF63j2j7vGjviZoBPDkOttXurF8sdPYOO+Ci9wE9ER4Q17EHjijlmukY7c+O 3oXYLy23pz+PnMsRlMFtiZ8xG3QSn+qCaspMkPSzoFuo4Rlzrk1WXtw7C89kAmeP1DtXwU 9fra/JzXkl6QC+bmFdBDsm2iL7ZytTQ= Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-1fc491f9b55so15419615ad.3 for ; Wed, 24 Jul 2024 02:53:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1721814802; x=1722419602; darn=kvack.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=/+Pi9ykJgDXDaJ4tze/4eRzB5JgzMNRwhMMIIYD9f6s=; b=LyiQxvHHdKKwf52tiLz8AVH8y3b6/3lZaNaXRnLfmn/4Yfr3p75iIkjEKDGCd8SQ12 GBgVRWx+RPJwJ//Bn+jZwc7U8/LsWLtuD7AGKx9xFmQhio3Wj0YPXGqJHhvGwUJ+xua6 tWucDZnu3DNNLn6VYHzDnCfNYuaX4c0OYt+RCuWUjD6Li7Sf2oKdmg9A5jDPpZpynQkr uvsS/M+HAZLpd2x1JNrpX6zPRRtIzFSMe2fTDhD5dRCeIhMQ5acQzjv/DxlokjZCN9tU 7vUwY7zpTN3HTdUWjmt2xibP7QnuzpXJ/+5fdn/7DOSPgeqs+zxgCnltu6R5ozbCAi77 qG5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721814802; x=1722419602; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=/+Pi9ykJgDXDaJ4tze/4eRzB5JgzMNRwhMMIIYD9f6s=; b=WPJcB5VC6nDkre/+77F/VabXx4ONnnB9dTWPTGu4/QXE+FxaPMcnQKKZY+WmhkmDNL rote7BmugynmT46M5nFfD8EDc3dMoLbjdfiRnFSD/7eLryvuFAHKmu0LaUF5DHIgDupv jUqhEB/hiNUPQoA/rkdAO97t7oyNTVbMp2Q16Qo+aOFDWzw8xDqrrplSkOzUp4u7WCfZ YcgIc7BMHqDHku7QNnjDeWTQc2L+Z29CkZ+PvmzFQ+NtxW75mTRTrVE5CAR1yaidRGxh Byc0Y5oC2TJIm47WXyMvI9rboRZTMt9D60NWbUtWi02OhIMTmrRaeEwQ6/OyBGR6QFJH VJhg== X-Forwarded-Encrypted: i=1; AJvYcCX8wSrCPSlWRStZEOGKbRwCj64hwLt3zs30IoDfK8SpwkeiQqy8xA7qV7EAH9uQT1TKjwivSXHWx7ke1k4vA5rhDTM= X-Gm-Message-State: AOJu0Yx5PFbvj9WSgeSuEHn6UJv3ynfmXZ2yV5cqtyXLyQLO/n+b5M6h UhaH1tUaqBw6AXJfrE+QMGRKGbmhYN8VKhrmxrnHKtlgd1KjOFhxABqti+7idkg= X-Google-Smtp-Source: AGHT+IHB3WwwOeRp8JWZP344FPZEZNd311bvbABQQU0UAKM6DV8pqhPHlbDVM2DQMrU5wlZOgwxKpQ== X-Received: by 2002:a17:903:645:b0:1fd:6766:684f with SMTP id d9443c01a7336-1fdd216e7b5mr19969225ad.31.1721814802424; Wed, 24 Jul 2024 02:53:22 -0700 (PDT) Received: from PXLDJ45XCM.bytedance.net ([139.177.225.240]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fd6f25b3a7sm90232325ad.60.2024.07.24.02.53.16 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Wed, 24 Jul 2024 02:53:21 -0700 (PDT) From: Muchun Song To: hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev, shakeel.butt@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org Cc: cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Muchun Song Subject: [PATCH v2] mm: kmem: add lockdep assertion to obj_cgroup_memcg Date: Wed, 24 Jul 2024 17:53:07 +0800 Message-Id: <20240724095307.81264-1-songmuchun@bytedance.com> X-Mailer: git-send-email 2.39.3 (Apple Git-146) MIME-Version: 1.0 X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 15155C0007 X-Stat-Signature: 1wp4pqm7gfh855knzra85rz8o9i9zrxt X-HE-Tag: 1721814803-140942 X-HE-Meta: U2FsdGVkX1+lGkCDfPkQI7yoOYoZRylZfdvJy38JOSMoonGxj1LRJRoWUOvurHuIJ65OpUkFhoFx7nZS95iqsWkpV/VpdE9w505sdRSzOuK3IrxSTQK8jXkLOlpFoEJMYSw7TlBC7B5UmTzCIS419oijf6F2FlX82PoyaHb+z3B9tbLH6/7ntmuptXiayJH6+PFrQV/vqJG1pmdMEzQHrXspGhMzJ0GqKLYW7vzUSIskRJIgqlR5v4DzALjmpMQHDbanudVwssvMjy0EU9mihzZRWCfJjALMUw2fBF6vUQ40yBkCkDI6zAvIYDKoicmOOWJeGz1wh+1C+Yfbtl4YiAHmQtYCBkPobenYwBveaIojgsQppummPMMx6q7SUyOWe5CT+GCaMWBYxizMozE5XMMsiU0SgTA8JB3UynpwYzG5+TkhPKUHno/vlRESnEvj1uFu2CQsTrNiH0jdSurALSyCFEnzIx5TBEhSYvNQB0MXXXCLmr7Qe58RXhUBYzSA5TpNcXLD9FI0WfPjEeFoTNbV21I/lAjFUSzmYYgnRUIzLCDUmEduH4I4w3/eQolgP0hzqMx8tZyPugHg9qxfFYSgM7dkaHTd3+JUC8Ceg7O0LmYWHBeMPfY2vS5YT1+jsk8f6It2G2rqpMDW4tHMzCMy6zAU6qdpxGwoykSa/Vg150JeGWrdvpoCYiza4yXGdhWmpWI+Lrb0lXvi+EJmWQ8kr+W8C7RMmoEjygWPNQRePH93hNnRn4ARCiq2k9UCFuo2Rsk9HmTCS8cup14ZJeui+7lu/SdhzZVzeHZmy6fB/4AENhKkmtUtyLKb1cUt3laK1q+vVmDQQh1iIRid56HNTCRupV9VoJCb4P67NkKxj+RTL0voRaHQO9tfkGHfhLmj+mLrRdrZ0SyYZkvqzS3VWRcGb9TRnDaCUrbmXDL7vkMm7wvdWUgkeMaSjAKaud1fOh1eTN6Nv7pmu8D zGJdLSOE oAhnIVtsFT3gyARSAk/62zH+XQe06xIDK2YMAKLGHgobWf4zzYLDiwGgyodPXiC5ERV9b51ExzoS3np0mJJvy3C/1kuiDbvrNXk4mwGiW/SBfcogkLqHhUP4C8hFkp0MTrzSzX4L8KuLQkcC08dWZxTwUyfH9t9B3z0RCYIIM0nhz0MFtCy8eyHFvMhJ0929EP1REiS9OvqgCD/ICz6oHOX/dDSfVlC1G26HbBny/zBRduXugimOIiY6ruLqk5KHtIaTIp4reux2cDPHQnTvoYoCewTod9EvGp5i9I8lT9E3vjls0NElIhrCKlpfehEDTqNFzR1DkYfRZhgnAZiWWaO8zA2XhcOmg3DSTR92XPBRjoQkOQOmSjBtQLbY0Aepv+OcxYOGjK9fOjo7QJ0RtYjmMSrxzVAQjHT++SPuQevzPQHTF2h9yzxsRVQ== 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: 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 --- 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..742351945f683 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) { + WARN_ON_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);