[PATCH -mm] memcg: remove activate_kmem_mutex

[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 <vdavydov@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
-- 
1.7.10.4

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

Re: [PATCH -mm] memcg: remove activate_kmem_mutex

[Johannes Weiner]

On Mon, Oct 20, 2014 at 07:11:29PM +0400, Vladimir Davydov wrote:
> 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 <vdavydov@parallels.com>

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

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

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

Re: [PATCH -mm] memcg: remove activate_kmem_mutex

[Michal Hocko]

On Mon 20-10-14 19:11:29, Vladimir Davydov wrote:
> 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 <vdavydov@parallels.com>

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

Anyway Looks good to me.
Acked-by: Michal Hocko <mhocko@suse.cz>

> ---
>  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
> -- 
> 1.7.10.4
> 

-- 
Michal Hocko
SUSE Labs

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

Re: [PATCH -mm] memcg: remove activate_kmem_mutex

[Vladimir Davydov]

On Mon, Oct 20, 2014 at 08:53:06PM +0200, Michal Hocko wrote:
> On Mon 20-10-14 19:11:29, Vladimir Davydov wrote:
> > 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 <vdavydov@parallels.com>
> 
> 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

> Anyway Looks good to me.
> Acked-by: Michal Hocko <mhocko@suse.cz>

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

Re: [PATCH -mm] memcg: remove activate_kmem_mutex

[Michal Hocko]

On Tue 21-10-14 10:31:19, Vladimir Davydov wrote:
> On Mon, Oct 20, 2014 at 08:53:06PM +0200, Michal Hocko wrote:
> > On Mon 20-10-14 19:11:29, Vladimir Davydov wrote:
> > > 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 <vdavydov@parallels.com>
> > 
> > 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.

-- 
Michal Hocko
SUSE Labs

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