Skip to content

Commit ca0dde9

Browse files
lizf-ostorvalds
authored andcommitted
memcg: take reference before releasing rcu_read_lock
The memcg is not referenced, so it can be destroyed at anytime right after we exit rcu read section, so it's not safe to access it. To fix this, we call css_tryget() to get a reference while we're still in rcu read section. This also removes a bogus comment above __memcg_create_cache_enqueue(). Signed-off-by: Li Zefan <lizefan@huawei.com> Acked-by: Glauber Costa <glommer@parallels.com> Acked-by: Michal Hocko <mhocko@suse.cz> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent ebff7d8 commit ca0dde9

File tree

1 file changed

+33
-30
lines changed

1 file changed

+33
-30
lines changed

mm/memcontrol.c

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3483,20 +3483,15 @@ static void memcg_create_cache_work_func(struct work_struct *w)
34833483

34843484
/*
34853485
* Enqueue the creation of a per-memcg kmem_cache.
3486-
* Called with rcu_read_lock.
34873486
*/
34883487
static void __memcg_create_cache_enqueue(struct mem_cgroup *memcg,
34893488
struct kmem_cache *cachep)
34903489
{
34913490
struct create_work *cw;
34923491

34933492
cw = kmalloc(sizeof(struct create_work), GFP_NOWAIT);
3494-
if (cw == NULL)
3495-
return;
3496-
3497-
/* The corresponding put will be done in the workqueue. */
3498-
if (!css_tryget(&memcg->css)) {
3499-
kfree(cw);
3493+
if (cw == NULL) {
3494+
css_put(&memcg->css);
35003495
return;
35013496
}
35023497

@@ -3552,10 +3547,9 @@ struct kmem_cache *__memcg_kmem_get_cache(struct kmem_cache *cachep,
35523547

35533548
rcu_read_lock();
35543549
memcg = mem_cgroup_from_task(rcu_dereference(current->mm->owner));
3555-
rcu_read_unlock();
35563550

35573551
if (!memcg_can_account_kmem(memcg))
3558-
return cachep;
3552+
goto out;
35593553

35603554
idx = memcg_cache_id(memcg);
35613555

@@ -3564,29 +3558,38 @@ struct kmem_cache *__memcg_kmem_get_cache(struct kmem_cache *cachep,
35643558
* code updating memcg_caches will issue a write barrier to match this.
35653559
*/
35663560
read_barrier_depends();
3567-
if (unlikely(cachep->memcg_params->memcg_caches[idx] == NULL)) {
3568-
/*
3569-
* If we are in a safe context (can wait, and not in interrupt
3570-
* context), we could be be predictable and return right away.
3571-
* This would guarantee that the allocation being performed
3572-
* already belongs in the new cache.
3573-
*
3574-
* However, there are some clashes that can arrive from locking.
3575-
* For instance, because we acquire the slab_mutex while doing
3576-
* kmem_cache_dup, this means no further allocation could happen
3577-
* with the slab_mutex held.
3578-
*
3579-
* Also, because cache creation issue get_online_cpus(), this
3580-
* creates a lock chain: memcg_slab_mutex -> cpu_hotplug_mutex,
3581-
* that ends up reversed during cpu hotplug. (cpuset allocates
3582-
* a bunch of GFP_KERNEL memory during cpuup). Due to all that,
3583-
* better to defer everything.
3584-
*/
3585-
memcg_create_cache_enqueue(memcg, cachep);
3586-
return cachep;
3561+
if (likely(cachep->memcg_params->memcg_caches[idx])) {
3562+
cachep = cachep->memcg_params->memcg_caches[idx];
3563+
goto out;
35873564
}
35883565

3589-
return cachep->memcg_params->memcg_caches[idx];
3566+
/* The corresponding put will be done in the workqueue. */
3567+
if (!css_tryget(&memcg->css))
3568+
goto out;
3569+
rcu_read_unlock();
3570+
3571+
/*
3572+
* If we are in a safe context (can wait, and not in interrupt
3573+
* context), we could be be predictable and return right away.
3574+
* This would guarantee that the allocation being performed
3575+
* already belongs in the new cache.
3576+
*
3577+
* However, there are some clashes that can arrive from locking.
3578+
* For instance, because we acquire the slab_mutex while doing
3579+
* kmem_cache_dup, this means no further allocation could happen
3580+
* with the slab_mutex held.
3581+
*
3582+
* Also, because cache creation issue get_online_cpus(), this
3583+
* creates a lock chain: memcg_slab_mutex -> cpu_hotplug_mutex,
3584+
* that ends up reversed during cpu hotplug. (cpuset allocates
3585+
* a bunch of GFP_KERNEL memory during cpuup). Due to all that,
3586+
* better to defer everything.
3587+
*/
3588+
memcg_create_cache_enqueue(memcg, cachep);
3589+
return cachep;
3590+
out:
3591+
rcu_read_unlock();
3592+
return cachep;
35903593
}
35913594
EXPORT_SYMBOL(__memcg_kmem_get_cache);
35923595

0 commit comments

Comments
 (0)