linux-kernel - Re: [PATCH] memcg: remove unused mem_cgroup_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJD7tkZhUB9N6R-1mWAPGV=awEO0Y0cmi9OmGiVhjSfdFBCirQ@mail.gmail.com>
Date:   Fri, 26 May 2023 20:55:22 -0700
From:   Yosry Ahmed <yosryahmed@...gle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     hannes@...xchg.org, mhocko@...nel.org, roman.gushchin@...ux.dev,
        shakeelb@...gle.com, akpm@...ux-foundation.org,
        muchun.song@...ux.dev, cgroups@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] memcg: remove unused mem_cgroup_from_obj()

On Fri, May 26, 2023 at 7:40 PM Miaohe Lin <linmiaohe@...wei.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@...wei.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@...gle.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
>