[PATCH -mm] memcg: remove activate_kmem_mutex

[Vladimir Davydov]

The activate_kmem_mutex is used to serialize memcg.kmem.limit updates,
but we already serialize them with memcg_limit_mutex so let's remove the
former.

Signed-off-by: Vladimir Davydov <***@parallels.com>
---
mm/memcontrol.c | 24 +++++-------------------
1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3a203c7ec6c7..e957f0c80c6e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2618,8 +2618,6 @@ static void commit_charge(struct page *page, struct mem_cgroup *memcg,
*/
static DEFINE_MUTEX(memcg_slab_mutex);

-static DEFINE_MUTEX(activate_kmem_mutex);
-
/*
* This is a bit cumbersome, but it is rarely used and avoids a backpointer
* in the memcg_cache_params struct.
@@ -3756,9 +3754,8 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
}

#ifdef CONFIG_MEMCG_KMEM
-/* should be called with activate_kmem_mutex held */
-static int __memcg_activate_kmem(struct mem_cgroup *memcg,
- unsigned long nr_pages)
+static int memcg_activate_kmem(struct mem_cgroup *memcg,
+ unsigned long nr_pages)
{
int err = 0;
int memcg_id;
@@ -3820,17 +3817,6 @@ out:
return err;
}

-static int memcg_activate_kmem(struct mem_cgroup *memcg,
- unsigned long nr_pages)
-{
- int ret;
-
- mutex_lock(&activate_kmem_mutex);
- ret = __memcg_activate_kmem(memcg, nr_pages);
- mutex_unlock(&activate_kmem_mutex);
- return ret;
-}
-
static int memcg_update_kmem_limit(struct mem_cgroup *memcg,
unsigned long limit)
{
@@ -3853,14 +3839,14 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
if (!parent)
return 0;

- mutex_lock(&activate_kmem_mutex);
+ mutex_lock(&memcg_limit_mutex);
/*
* If the parent cgroup is not kmem-active now, it cannot be activated
* after this point, because it has at least one child already.
*/
if (memcg_kmem_is_active(parent))
- ret = __memcg_activate_kmem(memcg, PAGE_COUNTER_MAX);
- mutex_unlock(&activate_kmem_mutex);
+ ret = memcg_activate_kmem(memcg, PAGE_COUNTER_MAX);
+ mutex_unlock(&memcg_limit_mutex);
return ret;
}
#else

[Johannes Weiner]

Post by Vladimir Davydov
The activate_kmem_mutex is used to serialize memcg.kmem.limit updates,
but we already serialize them with memcg_limit_mutex so let's remove the
former.

It hid well behind this obscure name, but that's really all it seems
to do. Away it goes!

Acked-by: Johannes Weiner <***@cmpxchg.org>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to ***@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"***@kvack.org"> ***@kvack.org </a>

[Michal Hocko]

Post by Vladimir Davydov
The activate_kmem_mutex is used to serialize memcg.kmem.limit updates,
but we already serialize them with memcg_limit_mutex so let's remove the
former.

Is this the case since bd67314586a3 (memcg, slab: simplify
synchronization scheme)?

Anyway Looks good to me.

Post by Vladimir Davydov
---
mm/memcontrol.c | 24 +++++-------------------
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3a203c7ec6c7..e957f0c80c6e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2618,8 +2618,6 @@ static void commit_charge(struct page *page, struct mem_cgroup *memcg,
*/
static DEFINE_MUTEX(memcg_slab_mutex);
-static DEFINE_MUTEX(activate_kmem_mutex);
-
/*
* This is a bit cumbersome, but it is rarely used and avoids a backpointer
* in the memcg_cache_params struct.
@@ -3756,9 +3754,8 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
}
#ifdef CONFIG_MEMCG_KMEM
-/* should be called with activate_kmem_mutex held */
-static int __memcg_activate_kmem(struct mem_cgroup *memcg,
- unsigned long nr_pages)
+static int memcg_activate_kmem(struct mem_cgroup *memcg,
+ unsigned long nr_pages)
{
int err = 0;
int memcg_id;
return err;
}
-static int memcg_activate_kmem(struct mem_cgroup *memcg,
- unsigned long nr_pages)
-{
- int ret;
-
- mutex_lock(&activate_kmem_mutex);
- ret = __memcg_activate_kmem(memcg, nr_pages);
- mutex_unlock(&activate_kmem_mutex);
- return ret;
-}
-
static int memcg_update_kmem_limit(struct mem_cgroup *memcg,
unsigned long limit)
{
@@ -3853,14 +3839,14 @@ static int memcg_propagate_kmem(struct mem_cgroup *memcg)
if (!parent)
return 0;
- mutex_lock(&activate_kmem_mutex);
+ mutex_lock(&memcg_limit_mutex);
/*
* If the parent cgroup is not kmem-active now, it cannot be activated
* after this point, because it has at least one child already.
*/
if (memcg_kmem_is_active(parent))
- ret = __memcg_activate_kmem(memcg, PAGE_COUNTER_MAX);
- mutex_unlock(&activate_kmem_mutex);
+ ret = memcg_activate_kmem(memcg, PAGE_COUNTER_MAX);
+ mutex_unlock(&memcg_limit_mutex);
return ret;
}
#else
--
1.7.10.4

[Vladimir Davydov]

Post by Michal Hocko

Post by Vladimir Davydov
The activate_kmem_mutex is used to serialize memcg.kmem.limit updates,
but we already serialize them with memcg_limit_mutex so let's remove the
former.

Is this the case since bd67314586a3 (memcg, slab: simplify
synchronization scheme)?

No, it's since Johannes' lockless page counters patch where we have the
memcg_limit_mutex introduced to synchronize concurrent limit updates (mm
commit dc1815408849 "mm: memcontrol: lockless page counters").

Thanks,
Vladimir

Post by Michal Hocko
Anyway Looks good to me.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to ***@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"***@kvack.org"> ***@kvack.org </a>

[Michal Hocko]

Post by Vladimir Davydov

Post by Michal Hocko

Post by Vladimir Davydov
The activate_kmem_mutex is used to serialize memcg.kmem.limit updates,
but we already serialize them with memcg_limit_mutex so let's remove the
former.

Is this the case since bd67314586a3 (memcg, slab: simplify
synchronization scheme)?

No, it's since Johannes' lockless page counters patch where we have the
memcg_limit_mutex introduced to synchronize concurrent limit updates (mm
commit dc1815408849 "mm: memcontrol: lockless page counters").

Ahh, ok. Thanks for the clarification.