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 A2965C2BD09 for ; Fri, 12 Jul 2024 04:21:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F0F2B6B009A; Fri, 12 Jul 2024 00:21:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E97DA6B009C; Fri, 12 Jul 2024 00:21:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D11876B009E; Fri, 12 Jul 2024 00:21:52 -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 AE7C16B009A for ; Fri, 12 Jul 2024 00:21:52 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 2DDCBC0913 for ; Fri, 12 Jul 2024 04:21:52 +0000 (UTC) X-FDA: 82329802464.14.2BA650D Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) by imf13.hostedemail.com (Postfix) with ESMTP id 438CB2001E for ; Fri, 12 Jul 2024 04:21:50 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=lyQQ0dEs; spf=pass (imf13.hostedemail.com: domain of seakeel@gmail.com designates 209.85.210.182 as permitted sender) smtp.mailfrom=seakeel@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=1720758065; 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=0bBLxjaqNYm/aQaWvWvN6K2lxER4ZUjX7/wDViAyzH0=; b=EaRVzdrxgWkBmWbYx1eZ5wq2GQMM24o3IY6ZTxXn8oy0MlTLtFrq7bOYieUvGqVT0sM3f/ OPaHAlC/DvUWiLITTAw0W6W1dxBjjIcOP96zJRB1MsPZ8h8TH49RTbidTSPK/njS0t7lF5 lRGtEES02ILcGT2yPohy+rd5ZRtQ3gQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=lyQQ0dEs; spf=pass (imf13.hostedemail.com: domain of seakeel@gmail.com designates 209.85.210.182 as permitted sender) smtp.mailfrom=seakeel@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720758065; a=rsa-sha256; cv=none; b=USxkTVEya1REo0fwmaIxhzyYWeJAHoVwtY+muznZJ7fcHgTcg4OweAI5sPRW9HbScez/Nu NWgo1kF/OqHm0NI0qtNP6MDRB4kfhwn1ZrTq1ZnOmqIp/LhpFXZg7CGKTPdYUhE1STZbRR EE+X85vsysK3+uq5eofz6vqX6iBfCnk= Received: by mail-pf1-f182.google.com with SMTP id d2e1a72fcca58-70b04cb28acso1314216b3a.0 for ; Thu, 11 Jul 2024 21:21:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720758109; x=1721362909; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=0bBLxjaqNYm/aQaWvWvN6K2lxER4ZUjX7/wDViAyzH0=; b=lyQQ0dEsc9RPo7WBSyWsTc6wYX5IlFsmmc831JUYnIbRlWBdcD7ZKxiJ2PGMOhXAGt g/7x8ZPwc26glCTdRzqEiW2ta+pKYTWVKrDL82DA3ueSm9tt2PI8C8WsOp94XWqIai0j P8se1wSdgJbYMFaZ7LdzpYAn3vCNfKAR0sp1onmcpDzvBtYTZm4aUYg2rRwNSmZlLyF6 ldZ20ctFUa4UJClIArrsE5YHXoJRnUSli/AjRwwgRZcOWwoJxSWuG9uHWLHplJd05G74 6vOZhci6A5H906a2ea0OcyLZqZkywmqN/R+0XWRkX1qacJn+/VkgEtB/A+qSJeiQ8n08 O6Xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720758109; x=1721362909; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=0bBLxjaqNYm/aQaWvWvN6K2lxER4ZUjX7/wDViAyzH0=; b=LZWNgrIWKI/NVsGdonLwsmgveuqhNgFWB2H2G8SdNLmTOdVEErxDGpUj4sRHhC3mJQ EARKikXLYE97q+dhafqf2nebD6caHsmPHmk+Ltp76RxFrnoI5RPtq//13Nsw7vnOIKhT U7L0V16o7ub05tYfd9fI39ChvR5Git2OrzSFjYq6DVjoTO+lryGYAh0i6J3+Cq3YOMTU eem80N8hEqPnCoxPTPtamV3IaBDjJumlIVuWhaqFNdYFk1XIYUn91zrSD6smpCo9CLHm Wao6eXkcFAJcvwDTa8z8N8dBgoOC7QVodL4NwePPwMWDFb9tJJt9Z1+0o+6539p9NudO Nq1g== X-Forwarded-Encrypted: i=1; AJvYcCWmkcyJNKBHBdQGHAxiTdhy3Pd/xhXBQS5naT3dc/Lt5doqLYKrYkFE78hwMuuJoXAInVfvAaDaZYQBFjiLWkGqBu4= X-Gm-Message-State: AOJu0Yzqgic4KGkC9Th/NKAzSA9b68YF5Li455bx6u/Sk1pNG2OpO/Zw DYvSbAnBuY7o8lRSHPYpBt0xHneKikD4FHOkmKoa402svnTJHrVw X-Google-Smtp-Source: AGHT+IG5/dVmgjHUAUbfksEHuRudvz+2wssKf3QfELdS2lWraGP8RWFqGnn7PciJazFufV7eILtAeg== X-Received: by 2002:a05:6a00:188b:b0:706:5daf:efa5 with SMTP id d2e1a72fcca58-70b6c8b4bdemr3288344b3a.9.1720758108940; Thu, 11 Jul 2024 21:21:48 -0700 (PDT) Received: from [192.168.255.10] ([43.132.141.20]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-70b43968688sm6465867b3a.114.2024.07.11.21.21.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 Jul 2024 21:21:48 -0700 (PDT) Message-ID: <059634b2-7346-4072-b5c2-5b1180bae694@gmail.com> Date: Fri, 12 Jul 2024 12:21:44 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [REF PATCH v3 2/2] mm/slab: decouple the SLAB_OBJ_EXT from MEMCG To: Suren Baghdasaryan , roman.gushchin@linux.dev Cc: Vlastimil Babka , alexs@kernel.org, Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Randy Dunlap , Yoann Congal , Masahiro Yamada , Petr Mladek References: <20240710054336.190410-1-alexs@kernel.org> <20240710054336.190410-2-alexs@kernel.org> <9b1384e7-e75d-4d71-8798-0d47c33cece6@gmail.com> Content-Language: en-US From: Alex Shi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 438CB2001E X-Stat-Signature: hijm4m5k9ttxro94kf7556ytyzzb5coi X-Rspam-User: X-HE-Tag: 1720758110-937607 X-HE-Meta: U2FsdGVkX19lO0t6DbH1/lD6e3YFzVLoqsiLTnJVKObbi20r2byyRW33/SMvK4RvXFW/5leQjrNdsthP2BE1MKc5yNCV641U85zLhsq07OPDqwYPjwlkOUS2hu/sHfAhY40ybMzLw1wwF6hbk0xR1QBnxZFOI10LHroB49GYwvNfi8fkzWPIw50KmY6XPw6fV0rRKtp4+B1dJ6sPx8Xs/3o6MR0f4ePK4EfNxRsl1GB1YbTCJdheZnKn7skj8GFpw+ZKrTFQF3OUtqZiZ4S5Ln4yKSkd0wDPvt9AWQdgZ1t3c2/j6S23tv08ubIsCw0hxer0Jd+JTm6uP79yOM+rwMt/gKnhEt1IQklQs2YH2IGWtF7vKtECgcFAQeQfTXXPZBcuC+fttzyIMZKORQT5/Mb34SjTchIaosq0c4FWGXdO1PY9jYfyUeMHCyCfz9urW+cUEu+TH0y11WoULzCjuFyBRTldA37B96DlFxWytFrZOKRCYj86/kqU/lCQb7vT30c3nWV34Td+i0U4l213PpIRFI0RXNQeJ4didaXDvehTGTHHYDLuQ6mcS8Sx+kWthAy3cnQPo7hZJTh7ka4hYMas8Us3y3T1obKbeIj5TqSFhDaFZ3/3ULvf2QGrVfgRbJo5bb/cTrYndz1l1mPEuxc7A46SFeZsA0+pbtfYKgOkqr50R5dsnFWfxvgj1GfzOAYEM/EXn0qsWq5mWeOrhGmsOA6kole344GMAiW4WZsML9FqGwu8MMPVwaSmFH87YfmaFcWLkj53WEomZUOnwAOuma6NYhFVR8yz9xuR+LGeeGtXzXXeZmtwVVmSmEv2tB8kGYfm2HnZOKr/xcmETsWUA4OhOeEcSNCZQruP9foEBarjYyjb3KMdZyN2KrJmBwF+sipXDUuAZHGch5G9FSv2yP5FvMjXJy0vNcCEEaeGdEJfHkqdEzbXswm54IhZQdteoZbgskBrz3iM99J xGZVmK+g MuheO7vFK9onq58vS3PBEhY0f9f9pQ1JfxxegVgxsgo+WpF8EXHRO01XllctP4hJeNamhJx5/dHOcUDHZ4e2t6NzPv4ftAFFHct51Q/b/zovK4gIHoLHS4a3PMwqUDQeDeH2vf4qitZesGQ8FAdoCB/X4rwTVSpWYZLxqukiV8lt2JUNu8YEdenHL74wuDmBo8NWHrStxI2MXQHJ/8A76hy43fkN4iuoqRvSCd9SL5cMl1JISZ9/FsfsuEMTHzCqHEN1+vcQYOdV00B/GF6azggZrh//HpYvjhycU8JARnPg6BobH8LQvGawtN9JB2hIMh0esh2xUoJLEUpRyju0wlG4dD+hx7lq30NRQARn9gjhhnVwEXEu6CjVERNjz3M0bgM6VXi6jIM9Wv/9tG1u/hUACbFCBkANuDDS79TGRrJq9Zykm3ZDkR1JTz0JGIf1wRphkd5az7VUDB1DFeXiimElRXxXOvi/7vkxZ6EEMskafzfI1naSTQyt3CeOCKSnrfUMS+Md6dvPjC1glhQGTZ4lAmSutj+hrnxKVom3eU6P7Dc9Q2oXGTWh8W68bsA2AZ347wVer2ZFYHbIJhlDUvb8Vp/TGUGVoPY/exJ3dj1waAnF4g+bRN/mQtidmE7eDsUlnU8dMSbcq9yhFgG+Mz8HcruWoIYsBQxFomCCmb7Y2xN3UNHOombNyHC3LGEnnHvsliiM1NZ/znmp4jsJWXvdTUuxe2Me5Dao/rp3eWv5EeyRNBc5ECkAY6LfAEdP9oYvNv12yO25Ui8JkElygmYGuZA== 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 7/11/24 9:55 PM, Suren Baghdasaryan wrote: > On Thu, Jul 11, 2024 at 4:49 AM Alex Shi wrote: >> >> >> >> On 7/11/24 4:11 PM, Vlastimil Babka wrote: >>> On 7/10/24 7:43 AM, alexs@kernel.org wrote: >>>> From: "Alex Shi (Tencent)" >>>> >>>> commit 21c690a349ba ("mm: introduce slabobj_ext to support slab object >>>> extensions") selected SLAB_OBJ_EXT on MEMCG just for SLAB_MATCH >>>> memcg_data, that included SLAB_OBJ_EXT for MEMCG. In fact, I didn't see >>>> the necessary to enable SLAB_OBJ_EXT for MEMCG. >>>> >>>> Let's decouple the SLAB_OBJ_EXT from MEMCG and move out >>>> alloc_slab_obj_exts() definition from SLAB_OBJ_EXT only. To alignment >>>> the alloc_slab_obj_exts() return 0 for good. change its return value to >>>> '-1' for always failed with !SLAB_OBJ_EXT. Now we could save unnecessary >>>> code from MEMCG but !SLAB_OBJ_EXT. >>>> >>>> Signed-off-by: Alex Shi (Tencent) >>> >>> This seems just wrong to me. The memcg hooks for slab do use obj_ext. You >>> made alloc_slab_obj_exts() return -1 and that will just fail all memcg >>> charging (unless alloc profiling selects obj_ext). The kernel will appear to >>> work, but memcg charging for slab won't happen at all. >>> >>> So no, it can't be decoupled for slab, only for pages/folios (patch 1). >> >> Hi Vlastimil, >> >> Thanks a lot for clarification! Yes, the patch isn't correct. >> >> Just forgive my stupidity, why the memcg needs SLAB_OBJ_EXT? > > Because when CONFIG_MEMCG_KMEM=y, slabobj_ext contains obj_cgroup > (see: https://elixir.bootlin.com/linux/v6.10-rc7/source/include/linux/memcontrol.h#L1593) Thanks for comments. Yes, if the obj_cg is sth we must have in MEMCG, then MEMCG should take OBJ_EXT. > and that's used for memcg accounting. Look into this call chain: > > kfree > slab_free > memcg_slab_free_hook > __memcg_slab_free_hook > obj_cgroup_uncharge> >> >> And why we need to alloc_slab_obj_exts() at line 3019 with !slab_obj_exts? I checked the history of slab for this part. It introduced from commit 10befea91b61c ("mm: memcg/slab: use a single set of kmem_caches for all allocations") But still don't know why !page_has_obj_cgroups followed by memcg_alloc_page_obj_cgroups. Anyone like to give a hints? page = virt_to_head_page(p[i]); + + if (!page_has_obj_cgroups(page) && + memcg_alloc_page_obj_cgroups(page, s, flags)) { + obj_cgroup_uncharge(objcg, obj_full_size(s)); + continue; + } Thanks a lot Alex >> 3015 for (i = 0; i < size; i++) { >> 3016 slab = virt_to_slab(p[i]); >> 3017 >> 3018 if (!slab_obj_exts(slab) && >> 3019 alloc_slab_obj_exts(slab, s, flags, false)) { >> 3020 obj_cgroup_uncharge(objcg, obj_full_size(s)); >> 3021 continue; >> 3022 } >> >> Thanks! >> Alex >> >>> >>> >>>> Cc: Randy Dunlap >>>> Cc: Yoann Congal >>>> Cc: Masahiro Yamada >>>> Cc: Petr Mladek >>>> --- >>>> init/Kconfig | 1 - >>>> mm/slab.h | 6 +++--- >>>> mm/slub.c | 6 +++--- >>>> 3 files changed, 6 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/init/Kconfig b/init/Kconfig >>>> index 26bf8bb0a7ce..61e43ac9fe75 100644 >>>> --- a/init/Kconfig >>>> +++ b/init/Kconfig >>>> @@ -965,7 +965,6 @@ config MEMCG >>>> bool "Memory controller" >>>> select PAGE_COUNTER >>>> select EVENTFD >>>> - select SLAB_OBJ_EXT >>>> help >>>> Provides control over the memory footprint of tasks in a cgroup. >>>> >>>> diff --git a/mm/slab.h b/mm/slab.h >>>> index 8ffdd4f315f8..6c727ecc1068 100644 >>>> --- a/mm/slab.h >>>> +++ b/mm/slab.h >>>> @@ -559,9 +559,6 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab) >>>> return (struct slabobj_ext *)(obj_exts & ~OBJEXTS_FLAGS_MASK); >>>> } >>>> >>>> -int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, >>>> - gfp_t gfp, bool new_slab); >>>> - >>>> #else /* CONFIG_SLAB_OBJ_EXT */ >>>> >>>> static inline struct slabobj_ext *slab_obj_exts(struct slab *slab) >>>> @@ -571,6 +568,9 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab) >>>> >>>> #endif /* CONFIG_SLAB_OBJ_EXT */ >>>> >>>> +int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, >>>> + gfp_t gfp, bool new_slab); >>>> + >>>> static inline enum node_stat_item cache_vmstat_idx(struct kmem_cache *s) >>>> { >>>> return (s->flags & SLAB_RECLAIM_ACCOUNT) ? >>>> diff --git a/mm/slub.c b/mm/slub.c >>>> index cc11f3869cc6..f531c2d67238 100644 >>>> --- a/mm/slub.c >>>> +++ b/mm/slub.c >>>> @@ -2075,10 +2075,10 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, >>>> >>>> #else /* CONFIG_SLAB_OBJ_EXT */ >>>> >>>> -static int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, >>>> - gfp_t gfp, bool new_slab) >>>> +int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s, >>>> + gfp_t gfp, bool new_slab) >>>> { >>>> - return 0; >>>> + return -1; >>>> } >>>> >>>> static inline void free_slab_obj_exts(struct slab *slab) >>>