2021-10-05 20:27:18

by Waiman Long

Subject: [PATCH v2] mm/memcg: Remove obsolete memcg_free_kmem()

Since commit d648bcc7fe65 ("mm: kmem: make memcg_kmem_enabled()
irreversible"), the only thing memcg_free_kmem() does is to call
memcg_offline_kmem() when the memcg is still online which can happen when
online_css() fails due to -ENOMEM. However, the name memcg_free_kmem()
is confusing and it is more clear and straight forward to call
memcg_offline_kmem() directly from mem_cgroup_css_free().

Suggested-by: Roman Gushchin <[email protected]>
Signed-off-by: Waiman Long <[email protected]>
---
mm/memcontrol.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6da5020a8656..96a93c608d80 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3656,13 +3656,6 @@ static void memcg_offline_kmem(struct mem_cgroup *memcg)

memcg_free_cache_id(kmemcg_id);
}
-
-static void memcg_free_kmem(struct mem_cgroup *memcg)
-{
- /* css_alloc() failed, offlining didn't happen */
- if (unlikely(memcg->kmem_state == KMEM_ONLINE))
- memcg_offline_kmem(memcg);
-}
#else
static int memcg_online_kmem(struct mem_cgroup *memcg)
{
@@ -3671,9 +3664,6 @@ static int memcg_online_kmem(struct mem_cgroup *memcg)
static void memcg_offline_kmem(struct mem_cgroup *memcg)
{
}
-static void memcg_free_kmem(struct mem_cgroup *memcg)
-{
-}
#endif /* CONFIG_MEMCG_KMEM */

static int memcg_update_kmem_max(struct mem_cgroup *memcg,
@@ -5308,7 +5298,9 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
cancel_work_sync(&memcg->high_work);
mem_cgroup_remove_from_trees(memcg);
free_shrinker_info(memcg);
- memcg_free_kmem(memcg);
+
+ /* Need to offline kmem if online_css() fails */
+ memcg_offline_kmem(memcg);
mem_cgroup_free(memcg);
}

--
2.18.1

2021-10-06 01:17:58

by Roman Gushchin

Subject: Re: [PATCH v2] mm/memcg: Remove obsolete memcg_free_kmem()

On Tue, Oct 05, 2021 at 04:24:50PM -0400, Waiman Long wrote:
> Since commit d648bcc7fe65 ("mm: kmem: make memcg_kmem_enabled()
> irreversible"), the only thing memcg_free_kmem() does is to call
> memcg_offline_kmem() when the memcg is still online which can happen when
> online_css() fails due to -ENOMEM. However, the name memcg_free_kmem()
> is confusing and it is more clear and straight forward to call
> memcg_offline_kmem() directly from mem_cgroup_css_free().
>
> Suggested-by: Roman Gushchin <[email protected]>
> Signed-off-by: Waiman Long <[email protected]>

Reviewed-by: Roman Gushchin <[email protected]>

Thanks, Waiman!

2021-10-06 05:24:04

by Shakeel Butt

Subject: Re: [PATCH v2] mm/memcg: Remove obsolete memcg_free_kmem()

On Tue, Oct 5, 2021 at 1:25 PM Waiman Long <[email protected]> wrote:
>
> Since commit d648bcc7fe65 ("mm: kmem: make memcg_kmem_enabled()
> irreversible"), the only thing memcg_free_kmem() does is to call
> memcg_offline_kmem() when the memcg is still online which can happen when
> online_css() fails due to -ENOMEM. However, the name memcg_free_kmem()
> is confusing and it is more clear and straight forward to call
> memcg_offline_kmem() directly from mem_cgroup_css_free().
>
> Suggested-by: Roman Gushchin <[email protected]>
> Signed-off-by: Waiman Long <[email protected]>

Reviewed-by: Shakeel Butt <[email protected]>

2021-10-06 19:10:12

by Aaron Tomlin

Subject: Re: [PATCH v2] mm/memcg: Remove obsolete memcg_free_kmem()

On Tue 2021-10-05 16:24 -0400, Waiman Long wrote:
> Since commit d648bcc7fe65 ("mm: kmem: make memcg_kmem_enabled()
> irreversible"), the only thing memcg_free_kmem() does is to call
> memcg_offline_kmem() when the memcg is still online which can happen when
> online_css() fails due to -ENOMEM. However, the name memcg_free_kmem()
> is confusing and it is more clear and straight forward to call
> memcg_offline_kmem() directly from mem_cgroup_css_free().
>
> Suggested-by: Roman Gushchin <[email protected]>
> Signed-off-by: Waiman Long <[email protected]>

Reviewed-by: Aaron Tomlin <[email protected]>

--
Aaron Tomlin