Commit e1a366be authored by Roman Gushchin's avatar Roman Gushchin Committed by Linus Torvalds

mm: memcontrol: switch to rcu protection in drain_all_stock()

Commit 72f0184c ("mm, memcg: remove hotplug locking from try_charge")
introduced css_tryget()/css_put() calls in drain_all_stock(), which are
supposed to protect the target memory cgroup from being released during
the mem_cgroup_is_descendant() call.

However, it's not completely safe.  In theory, memcg can go away between
reading stock->cached pointer and calling css_tryget().

This can happen if drain_all_stock() races with drain_local_stock()
performed on the remote cpu as a result of a work, scheduled by the
previous invocation of drain_all_stock().

The race is a bit theoretical and there are few chances to trigger it, but
the current code looks a bit confusing, so it makes sense to fix it
anyway.  The code looks like as if css_tryget() and css_put() are used to
protect stocks drainage.  It's not necessary because stocked pages are
holding references to the cached cgroup.  And it obviously won't work for
works, scheduled on other cpus.

So, let's read the stock->cached pointer and evaluate the memory cgroup
inside a rcu read section, and get rid of css_tryget()/css_put() calls.

Link: http://lkml.kernel.org/r/20190802192241.3253165-1-guro@fb.comSigned-off-by: default avatarRoman Gushchin <guro@fb.com>
Acked-by: default avatarMichal Hocko <mhocko@suse.com>
Cc: Hillf Danton <hdanton@sina.com>
Cc: Johannes Weiner <hannes@cmpxchg.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 0e4b01df
...@@ -2271,21 +2271,22 @@ static void drain_all_stock(struct mem_cgroup *root_memcg) ...@@ -2271,21 +2271,22 @@ static void drain_all_stock(struct mem_cgroup *root_memcg)
for_each_online_cpu(cpu) { for_each_online_cpu(cpu) {
struct memcg_stock_pcp *stock = &per_cpu(memcg_stock, cpu); struct memcg_stock_pcp *stock = &per_cpu(memcg_stock, cpu);
struct mem_cgroup *memcg; struct mem_cgroup *memcg;
bool flush = false;
rcu_read_lock();
memcg = stock->cached; memcg = stock->cached;
if (!memcg || !stock->nr_pages || !css_tryget(&memcg->css)) if (memcg && stock->nr_pages &&
continue; mem_cgroup_is_descendant(memcg, root_memcg))
if (!mem_cgroup_is_descendant(memcg, root_memcg)) { flush = true;
css_put(&memcg->css); rcu_read_unlock();
continue;
} if (flush &&
if (!test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags)) { !test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags)) {
if (cpu == curcpu) if (cpu == curcpu)
drain_local_stock(&stock->work); drain_local_stock(&stock->work);
else else
schedule_work_on(cpu, &stock->work); schedule_work_on(cpu, &stock->work);
} }
css_put(&memcg->css);
} }
put_cpu(); put_cpu();
mutex_unlock(&percpu_charge_mutex); mutex_unlock(&percpu_charge_mutex);
......
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