Commit c5c8b16b authored by Muchun Song's avatar Muchun Song Committed by Linus Torvalds

mm: memcontrol: fix root_mem_cgroup charging

The below scenario can cause the page counters of the root_mem_cgroup to
be out of balance.

CPU0:                                   CPU1:

objcg = get_obj_cgroup_from_current()
obj_cgroup_charge_pages(objcg)
                                        memcg_reparent_objcgs()
                                            // reparent to root_mem_cgroup
                                            WRITE_ONCE(iter->memcg, parent)
    // memcg == root_mem_cgroup
    memcg = get_mem_cgroup_from_objcg(objcg)
    // do not charge to the root_mem_cgroup
    try_charge(memcg)

obj_cgroup_uncharge_pages(objcg)
    memcg = get_mem_cgroup_from_objcg(objcg)
    // uncharge from the root_mem_cgroup
    refill_stock(memcg)
        drain_stock(memcg)
            page_counter_uncharge(&memcg->memory)

get_obj_cgroup_from_current() never returns a root_mem_cgroup's objcg, so
we never explicitly charge the root_mem_cgroup.  And it's not going to
change.  It's all about a race when we got an obj_cgroup pointing at some
non-root memcg, but before we were able to charge it, the cgroup was gone,
objcg was reparented to the root and so we're skipping the charging.  Then
we store the objcg pointer and later use to uncharge the root_mem_cgroup.

This can cause the page counter to be less than the actual value.
Although we do not display the value (mem_cgroup_usage) so there shouldn't
be any actual problem, but there is a WARN_ON_ONCE in the
page_counter_cancel().  Who knows if it will trigger?  So it is better to
fix it.

Link: https://lkml.kernel.org/r/20210425075410.19255-1-songmuchun@bytedance.comSigned-off-by: default avatarMuchun Song <songmuchun@bytedance.com>
Acked-by: default avatarRoman Gushchin <guro@fb.com>
Reviewed-by: default avatarShakeel Butt <shakeelb@google.com>
Cc: Xiongchun Duan <duanxiongchun@bytedance.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 13e680fb
...@@ -2568,8 +2568,8 @@ void mem_cgroup_handle_over_high(void) ...@@ -2568,8 +2568,8 @@ void mem_cgroup_handle_over_high(void)
css_put(&memcg->css); css_put(&memcg->css);
} }
static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
unsigned int nr_pages) unsigned int nr_pages)
{ {
unsigned int batch = max(MEMCG_CHARGE_BATCH, nr_pages); unsigned int batch = max(MEMCG_CHARGE_BATCH, nr_pages);
int nr_retries = MAX_RECLAIM_RETRIES; int nr_retries = MAX_RECLAIM_RETRIES;
...@@ -2581,8 +2581,6 @@ static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, ...@@ -2581,8 +2581,6 @@ static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask,
bool drained = false; bool drained = false;
unsigned long pflags; unsigned long pflags;
if (mem_cgroup_is_root(memcg))
return 0;
retry: retry:
if (consume_stock(memcg, nr_pages)) if (consume_stock(memcg, nr_pages))
return 0; return 0;
...@@ -2762,6 +2760,15 @@ static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, ...@@ -2762,6 +2760,15 @@ static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask,
return 0; return 0;
} }
static inline int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask,
unsigned int nr_pages)
{
if (mem_cgroup_is_root(memcg))
return 0;
return try_charge_memcg(memcg, gfp_mask, nr_pages);
}
#if defined(CONFIG_MEMCG_KMEM) || defined(CONFIG_MMU) #if defined(CONFIG_MEMCG_KMEM) || defined(CONFIG_MMU)
static void cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages) static void cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages)
{ {
...@@ -2997,7 +3004,7 @@ static int obj_cgroup_charge_pages(struct obj_cgroup *objcg, gfp_t gfp, ...@@ -2997,7 +3004,7 @@ static int obj_cgroup_charge_pages(struct obj_cgroup *objcg, gfp_t gfp,
memcg = get_mem_cgroup_from_objcg(objcg); memcg = get_mem_cgroup_from_objcg(objcg);
ret = try_charge(memcg, gfp, nr_pages); ret = try_charge_memcg(memcg, gfp, nr_pages);
if (ret) if (ret)
goto out; goto out;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment