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;
MIME-Version: 1.0
References: <20230527103126.398267-1-linmiaohe@huawei.com>
In-Reply-To: <20230527103126.398267-1-linmiaohe@huawei.com>
From:   Yosry Ahmed <yosryahmed@google.com>
Date:   Fri, 26 May 2023 20:55:22 -0700
Message-ID: <CAJD7tkZhUB9N6R-1mWAPGV=awEO0Y0cmi9OmGiVhjSfdFBCirQ@mail.gmail.com>
Subject: Re: [PATCH] memcg: remove unused mem_cgroup_from_obj()
To:     Miaohe Lin <linmiaohe@huawei.com>
Cc:     hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev,
        shakeelb@google.com, akpm@linux-foundation.org,
        muchun.song@linux.dev, cgroups@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, May 26, 2023 at 7:40=E2=80=AFPM Miaohe Lin <linmiaohe@huawei.com> w=
rote:
>
> 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(struc=
t 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 gene=
ric
> - * kernel page, so different mechanisms for getting the memory cgroup po=
inter
> - * 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_lo=
ck(),
> - * 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 =3D page_folio(vmalloc_to_page(p));
> -       else
> -               folio =3D 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 obj=
ects,
> - * 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_lo=
ck(),
> --
> 2.27.0
>