Message ID | 20230527103126.398267-1-linmiaohe@huawei.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp107061vqr; Fri, 26 May 2023 20:31:09 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5nleAovxNlDEMyRXzYGbwJAKaMTrTT1YeaxG+1aV17eJWG9S57LUqSI3U3l2hm/xSeRPvi X-Received: by 2002:a05:6a20:1581:b0:10c:1378:f2ae with SMTP id h1-20020a056a20158100b0010c1378f2aemr1912611pzj.46.1685158268667; Fri, 26 May 2023 20:31:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685158268; cv=none; d=google.com; s=arc-20160816; b=V6+i0Xdyn1zcMpXR+uHOonIjrHvdl1SGNqRRW0xoK/ogW6JrVd3EFPSL6fyUX8fIyh LbBDRaBrjkd0KmPuX35Uyk917C6diucWntihYJJtXMaM+JbTiV13wxG/iNwJRx6+SFdt +MbXEwllWcVm+8Q083Nd6KxWQ8SgjICfmWJbzpUsLUzUyypVwcftZMwvKdlDpJndw8S/ a5VlTgd/OEIzbQDCTdGouT+iRVnQDptVztfVx9NIln68ngsGiZj6CwAvVfW4JVnZ8/Yl UFmWZyEEL/GamTV/apFfcjxUxNjn5euFnxfZ0Sc/n0oSFCc/apIyFNbGk2sswAxHZgdJ iTiA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=9wYbgQqJ9PMyfu/YyM/zhnpAUEaukpLO/SmBNOHu/PE=; b=d5bFVh9twVQRr8LNjw62LYXfLYU7widH0vg8DfuIkZnITwpkfO6WNlfVMYBxr4mMGO n3PxHcnVxmAWYaYZSkefPI8CqmoVKhpaj3VIEhc0LlE3qVJFLOP6M5MdpZeMH6ClJQ+U ywwG41mFcaqjh6VJQ5RykqSNDipjk8X+oWOtPX87mArdTdsqNGepzWczwwlTDtyBdT0Q BDx3WzMUzodWtfBIUGRcbZgAUemyUlmvvHUeRfxIZHVflXqd/cMEeYB9lRfMT87ob1BU BmxfBGgmPC2bvvU5TzoMccq/cIDzsnZG5sztAYJ8T/iVhFvvwCejDVmqcKlnI3xrQi4K ByHw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id mg10-20020a17090b370a00b00233f3034302si5634505pjb.46.2023.05.26.20.30.31; Fri, 26 May 2023 20:31:08 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230110AbjE0CkO (ORCPT <rfc822;zhanglyra.2023@gmail.com> + 99 others); Fri, 26 May 2023 22:40:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50588 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229715AbjE0CkN (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 26 May 2023 22:40:13 -0400 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B9FDFC9; Fri, 26 May 2023 19:40:11 -0700 (PDT) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4QSm9f52TQzqSHN; Sat, 27 May 2023 10:35:34 +0800 (CST) Received: from huawei.com (10.175.104.170) by canpemm500002.china.huawei.com (7.192.104.244) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Sat, 27 May 2023 10:40:07 +0800 From: Miaohe Lin <linmiaohe@huawei.com> To: <hannes@cmpxchg.org>, <mhocko@kernel.org>, <roman.gushchin@linux.dev>, <shakeelb@google.com>, <akpm@linux-foundation.org> CC: <muchun.song@linux.dev>, <cgroups@vger.kernel.org>, <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>, <linmiaohe@huawei.com> Subject: [PATCH] memcg: remove unused mem_cgroup_from_obj() Date: Sat, 27 May 2023 18:31:26 +0800 Message-ID: <20230527103126.398267-1-linmiaohe@huawei.com> X-Mailer: git-send-email 2.23.0 MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII X-Originating-IP: [10.175.104.170] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To canpemm500002.china.huawei.com (7.192.104.244) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00,DATE_IN_FUTURE_06_12, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1767016516575641130?= X-GMAIL-MSGID: =?utf-8?q?1767016516575641130?= |
Series |
memcg: remove unused mem_cgroup_from_obj()
|
|
Commit Message
Miaohe Lin
May 27, 2023, 10:31 a.m. UTC
The function mem_cgroup_from_obj() is not used anymore. Remove it and
clean up relevant comments.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
include/linux/memcontrol.h | 6 ------
mm/memcontrol.c | 31 -------------------------------
2 files changed, 37 deletions(-)
Comments
On Fri, May 26, 2023 at 7:40 PM Miaohe Lin <linmiaohe@huawei.com> wrote: > > The function mem_cgroup_from_obj() is not used anymore. Remove it and > clean up relevant comments. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > include/linux/memcontrol.h | 6 ------ > mm/memcontrol.c | 31 ------------------------------- > 2 files changed, 37 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 00a88cf947e1..ce8c2355ed9f 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1813,7 +1813,6 @@ static inline int memcg_kmem_id(struct mem_cgroup *memcg) > return memcg ? memcg->kmemcg_id : -1; > } > > -struct mem_cgroup *mem_cgroup_from_obj(void *p); > struct mem_cgroup *mem_cgroup_from_slab_obj(void *p); > > static inline void count_objcg_event(struct obj_cgroup *objcg, > @@ -1876,11 +1875,6 @@ static inline int memcg_kmem_id(struct mem_cgroup *memcg) > return -1; > } > > -static inline struct mem_cgroup *mem_cgroup_from_obj(void *p) > -{ > - return NULL; > -} > - > static inline struct mem_cgroup *mem_cgroup_from_slab_obj(void *p) > { > return NULL; > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 6a3d4ce87b8a..532b29c9a0fe 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2972,37 +2972,6 @@ struct mem_cgroup *mem_cgroup_from_obj_folio(struct folio *folio, void *p) > /* > * Returns a pointer to the memory cgroup to which the kernel object is charged. > * > - * A passed kernel object can be a slab object, vmalloc object or a generic > - * kernel page, so different mechanisms for getting the memory cgroup pointer > - * should be used. > - * > - * In certain cases (e.g. kernel stacks or large kmallocs with SLUB) the caller > - * can not know for sure how the kernel object is implemented. > - * mem_cgroup_from_obj() can be safely used in such cases. > - * > - * The caller must ensure the memcg lifetime, e.g. by taking rcu_read_lock(), > - * cgroup_mutex, etc. > - */ > -struct mem_cgroup *mem_cgroup_from_obj(void *p) > -{ > - struct folio *folio; > - > - if (mem_cgroup_disabled()) > - return NULL; > - > - if (unlikely(is_vmalloc_addr(p))) > - folio = page_folio(vmalloc_to_page(p)); > - else > - folio = virt_to_folio(p); > - > - return mem_cgroup_from_obj_folio(folio, p); > -} > - > -/* > - * Returns a pointer to the memory cgroup to which the kernel object is charged. > - * Similar to mem_cgroup_from_obj(), but faster and not suitable for objects, > - * allocated using vmalloc(). Perhaps keep the line about not being suitable for objects allocated using vmalloc()? To be fair it's obvious from the function name, but I am guessing whoever added it did for a reason. I don't feel strongly either way, LGTM. I can't see any references in Linus's tree or mm-unstable. Reviewed-by: Yosry Ahmed <yosryahmed@google.com> > - * > * A passed kernel object must be a slab object or a generic kernel page. > * > * The caller must ensure the memcg lifetime, e.g. by taking rcu_read_lock(), > -- > 2.27.0 >
On Sat, May 27, 2023 at 06:31:26PM +0800, Miaohe Lin wrote: > The function mem_cgroup_from_obj() is not used anymore. Remove it and > clean up relevant comments. You should have looked at the git history to see why it was created and who used it. Shakeel, Vasily, are you going to retry adding commit 1d0403d20f6c?
On Fri, May 26, 2023 at 9:01 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Sat, May 27, 2023 at 06:31:26PM +0800, Miaohe Lin wrote: > > The function mem_cgroup_from_obj() is not used anymore. Remove it and > > clean up relevant comments. > > You should have looked at the git history to see why it was created > and who used it. > > Shakeel, Vasily, are you going to retry adding commit 1d0403d20f6c? That commit did not introduce the function though, no? It was introduced before it and replaced by other variants over time (like mem_cgroup_from_slab_obj()). It looks like that commit was reverted ~9 months ago. We can always bring it back if/when needed. It also looks to me that 1d0403d20f6c was using mem_cgroup_from_obj() on a struct net object, which is allocated in net_alloc() from a slab cache, so mem_cgroup_from_slab_obj() should be sufficient, no? > >
On Fri, May 26, 2023 at 09:13:05PM -0700, Yosry Ahmed wrote: > On Fri, May 26, 2023 at 9:01 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Sat, May 27, 2023 at 06:31:26PM +0800, Miaohe Lin wrote: > > > The function mem_cgroup_from_obj() is not used anymore. Remove it and > > > clean up relevant comments. > > > > You should have looked at the git history to see why it was created > > and who used it. > > > > Shakeel, Vasily, are you going to retry adding commit 1d0403d20f6c? > > That commit did not introduce the function though, no? It was > introduced before it and replaced by other variants over time (like > mem_cgroup_from_slab_obj()). It looks like that commit was reverted ~9 > months ago. We can always bring it back if/when needed. The commit immediately preceding it is fc4db90fe71e. Of course we can bring it back. It's just code. But avoiding unnecessary churn is also good. Let's wait to hear from Vasily. > It also looks to me that 1d0403d20f6c was using mem_cgroup_from_obj() > on a struct net object, which is allocated in net_alloc() from a slab > cache, so mem_cgroup_from_slab_obj() should be sufficient, no? Clearly not.
On Sat, May 27, 2023 at 8:07 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Fri, May 26, 2023 at 09:13:05PM -0700, Yosry Ahmed wrote: > > On Fri, May 26, 2023 at 9:01 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > On Sat, May 27, 2023 at 06:31:26PM +0800, Miaohe Lin wrote: > > > > The function mem_cgroup_from_obj() is not used anymore. Remove it and > > > > clean up relevant comments. > > > > > > You should have looked at the git history to see why it was created > > > and who used it. > > > > > > Shakeel, Vasily, are you going to retry adding commit 1d0403d20f6c? > > > > That commit did not introduce the function though, no? It was > > introduced before it and replaced by other variants over time (like > > mem_cgroup_from_slab_obj()). It looks like that commit was reverted ~9 > > months ago. We can always bring it back if/when needed. > > The commit immediately preceding it is fc4db90fe71e. > > Of course we can bring it back. It's just code. But avoiding > unnecessary churn is also good. Let's wait to hear from Vasily. > > > It also looks to me that 1d0403d20f6c was using mem_cgroup_from_obj() > > on a struct net object, which is allocated in net_alloc() from a slab > > cache, so mem_cgroup_from_slab_obj() should be sufficient, no? > > Clearly not. I dived deeper into the history on LKML, and you are right: https://lore.kernel.org/all/Yp4F6n2Ie32re7Ed@qian/ I still do not understand why mem_cgroup_from_slab_obj() would not be sufficient, so I am hoping Vasily or Shakeel can help me understand here. Seems to be something arch-specific. Thanks for digging this up, Matthew.
> On May 28, 2023, at 02:54, Yosry Ahmed <yosryahmed@google.com> wrote: > > On Sat, May 27, 2023 at 8:07 AM Matthew Wilcox <willy@infradead.org> wrote: >> >> On Fri, May 26, 2023 at 09:13:05PM -0700, Yosry Ahmed wrote: >>> On Fri, May 26, 2023 at 9:01 PM Matthew Wilcox <willy@infradead.org> wrote: >>>> >>>> On Sat, May 27, 2023 at 06:31:26PM +0800, Miaohe Lin wrote: >>>>> The function mem_cgroup_from_obj() is not used anymore. Remove it and >>>>> clean up relevant comments. >>>> >>>> You should have looked at the git history to see why it was created >>>> and who used it. >>>> >>>> Shakeel, Vasily, are you going to retry adding commit 1d0403d20f6c? >>> >>> That commit did not introduce the function though, no? It was >>> introduced before it and replaced by other variants over time (like >>> mem_cgroup_from_slab_obj()). It looks like that commit was reverted ~9 >>> months ago. We can always bring it back if/when needed. >> >> The commit immediately preceding it is fc4db90fe71e. >> >> Of course we can bring it back. It's just code. But avoiding >> unnecessary churn is also good. Let's wait to hear from Vasily. >> >>> It also looks to me that 1d0403d20f6c was using mem_cgroup_from_obj() >>> on a struct net object, which is allocated in net_alloc() from a slab >>> cache, so mem_cgroup_from_slab_obj() should be sufficient, no? >> >> Clearly not. > > I dived deeper into the history on LKML, and you are right: > https://lore.kernel.org/all/Yp4F6n2Ie32re7Ed@qian/ > > I still do not understand why mem_cgroup_from_slab_obj() would not be > sufficient, so I am hoping Vasily or Shakeel can help me understand > here. Seems to be something arch-specific. I think it is because *init_net* which is not allocated from slab meant its address does not belong to linear mapping addresses on arm64. However, virt_to_page() is only applicable to linear mapping addresses. So, mem_cgroup_from_slab_obj() is not sufficient. mem_cgroup_from_obj() is used in this case, which will use vmalloc_to_page() for the page associated with *init_net*. If Vasily does not want to bring commit 1d0403d20f6c back, this patch LGTM. Otherwise, let's wait for Vasily. Thanks.
On Sun, May 28, 2023 at 09:01:37PM +0800, Muchun Song wrote: > with *init_net*. If Vasily does not want to bring commit 1d0403d20f6c back, > this patch LGTM. Otherwise, let's wait for Vasily. If we're not going to bring back 1d0403d20f6c then we should simply revert fc4db90fe71e instead of applying this patch.
On Sun, May 28, 2023 at 6:02 AM Muchun Song <muchun.song@linux.dev> wrote: > > > > > On May 28, 2023, at 02:54, Yosry Ahmed <yosryahmed@google.com> wrote: > > > > On Sat, May 27, 2023 at 8:07 AM Matthew Wilcox <willy@infradead.org> wrote: > >> > >> On Fri, May 26, 2023 at 09:13:05PM -0700, Yosry Ahmed wrote: > >>> On Fri, May 26, 2023 at 9:01 PM Matthew Wilcox <willy@infradead.org> wrote: > >>>> > >>>> On Sat, May 27, 2023 at 06:31:26PM +0800, Miaohe Lin wrote: > >>>>> The function mem_cgroup_from_obj() is not used anymore. Remove it and > >>>>> clean up relevant comments. > >>>> > >>>> You should have looked at the git history to see why it was created > >>>> and who used it. > >>>> > >>>> Shakeel, Vasily, are you going to retry adding commit 1d0403d20f6c? > >>> > >>> That commit did not introduce the function though, no? It was > >>> introduced before it and replaced by other variants over time (like > >>> mem_cgroup_from_slab_obj()). It looks like that commit was reverted ~9 > >>> months ago. We can always bring it back if/when needed. > >> > >> The commit immediately preceding it is fc4db90fe71e. > >> > >> Of course we can bring it back. It's just code. But avoiding > >> unnecessary churn is also good. Let's wait to hear from Vasily. > >> > >>> It also looks to me that 1d0403d20f6c was using mem_cgroup_from_obj() > >>> on a struct net object, which is allocated in net_alloc() from a slab > >>> cache, so mem_cgroup_from_slab_obj() should be sufficient, no? > >> > >> Clearly not. > > > > I dived deeper into the history on LKML, and you are right: > > https://lore.kernel.org/all/Yp4F6n2Ie32re7Ed@qian/ > > > > I still do not understand why mem_cgroup_from_slab_obj() would not be > > sufficient, so I am hoping Vasily or Shakeel can help me understand > > here. Seems to be something arch-specific. > > I think it is because *init_net* which is not allocated from slab meant > its address does not belong to linear mapping addresses on arm64. However, > virt_to_page() is only applicable to linear mapping addresses. So, > mem_cgroup_from_slab_obj() is not sufficient. mem_cgroup_from_obj() is used > in this case, which will use vmalloc_to_page() for the page associated > with *init_net*. If Vasily does not want to bring commit 1d0403d20f6c back, > this patch LGTM. Otherwise, let's wait for Vasily. I see, thanks for the context, Muchun! > > Thanks. >
On Sun, May 28, 2023 at 08:36:43PM +0100, Matthew Wilcox wrote: > On Sun, May 28, 2023 at 09:01:37PM +0800, Muchun Song wrote: > > with *init_net*. If Vasily does not want to bring commit 1d0403d20f6c back, > > this patch LGTM. Otherwise, let's wait for Vasily. > > If we're not going to bring back 1d0403d20f6c then we should > simply revert fc4db90fe71e instead of applying this patch. Initially I was thinking of adding virt_addr_valid() check in the mem_cgroup_from_obj() but it seems like that check is not cheap on arm64. I don't have any quick solutions other than adding a check against init_net in __register_pernet_operations(). I will wait for couple of days for Vasily otherwise I will retry 1d0403d20f6c with the init_net check in __register_pernet_operations().
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 00a88cf947e1..ce8c2355ed9f 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -1813,7 +1813,6 @@ static inline int memcg_kmem_id(struct mem_cgroup *memcg) return memcg ? memcg->kmemcg_id : -1; } -struct mem_cgroup *mem_cgroup_from_obj(void *p); struct mem_cgroup *mem_cgroup_from_slab_obj(void *p); static inline void count_objcg_event(struct obj_cgroup *objcg, @@ -1876,11 +1875,6 @@ static inline int memcg_kmem_id(struct mem_cgroup *memcg) return -1; } -static inline struct mem_cgroup *mem_cgroup_from_obj(void *p) -{ - return NULL; -} - static inline struct mem_cgroup *mem_cgroup_from_slab_obj(void *p) { return NULL; diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 6a3d4ce87b8a..532b29c9a0fe 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2972,37 +2972,6 @@ struct mem_cgroup *mem_cgroup_from_obj_folio(struct folio *folio, void *p) /* * Returns a pointer to the memory cgroup to which the kernel object is charged. * - * A passed kernel object can be a slab object, vmalloc object or a generic - * kernel page, so different mechanisms for getting the memory cgroup pointer - * should be used. - * - * In certain cases (e.g. kernel stacks or large kmallocs with SLUB) the caller - * can not know for sure how the kernel object is implemented. - * mem_cgroup_from_obj() can be safely used in such cases. - * - * The caller must ensure the memcg lifetime, e.g. by taking rcu_read_lock(), - * cgroup_mutex, etc. - */ -struct mem_cgroup *mem_cgroup_from_obj(void *p) -{ - struct folio *folio; - - if (mem_cgroup_disabled()) - return NULL; - - if (unlikely(is_vmalloc_addr(p))) - folio = page_folio(vmalloc_to_page(p)); - else - folio = virt_to_folio(p); - - return mem_cgroup_from_obj_folio(folio, p); -} - -/* - * Returns a pointer to the memory cgroup to which the kernel object is charged. - * Similar to mem_cgroup_from_obj(), but faster and not suitable for objects, - * allocated using vmalloc(). - * * A passed kernel object must be a slab object or a generic kernel page. * * The caller must ensure the memcg lifetime, e.g. by taking rcu_read_lock(),