[PATCH] mm/memcg: replace inexistence move_lock_page_cgroup() by move_lock_mem_cgroup() in comment

[PATCH] mm/memcg: replace inexistence move_lock_page_cgroup() by move_lock_mem_cgroup() in comment

[Wanpeng Li]

From: Wanpeng Li <liwp@linux.vnet.ibm.com>

Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
---
 mm/memcontrol.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3d318f6..63e36e7 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1899,7 +1899,7 @@ again:
 		return;
 	/*
 	 * If this memory cgroup is not under account moving, we don't
-	 * need to take move_lock_page_cgroup(). Because we already hold
+	 * need to take move_lock_mem_cgroup(). Because we already hold
 	 * rcu_read_lock(), any calls to move_account will be delayed until
 	 * rcu_read_unlock() if mem_cgroup_stolen() == true.
 	 */
@@ -1921,7 +1921,7 @@ void __mem_cgroup_end_update_page_stat(struct page *page, unsigned long *flags)
 	/*
 	 * It's guaranteed that pc->mem_cgroup never changes while
 	 * lock is held because a routine modifies pc->mem_cgroup
-	 * should take move_lock_page_cgroup().
+	 * should take move_lock_mem_cgroup().
 	 */
 	move_unlock_mem_cgroup(pc->mem_cgroup, flags);
 }
-- 
1.7.5.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: replace inexistence move_lock_page_cgroup() by move_lock_mem_cgroup() in comment

[Kamezawa Hiroyuki]

(2012/07/05 15:28), Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


> ---
>   mm/memcontrol.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 3d318f6..63e36e7 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1899,7 +1899,7 @@ again:
>   		return;
>   	/*
>   	 * If this memory cgroup is not under account moving, we don't
> -	 * need to take move_lock_page_cgroup(). Because we already hold
> +	 * need to take move_lock_mem_cgroup(). Because we already hold
>   	 * rcu_read_lock(), any calls to move_account will be delayed until
>   	 * rcu_read_unlock() if mem_cgroup_stolen() == true.
>   	 */
> @@ -1921,7 +1921,7 @@ void __mem_cgroup_end_update_page_stat(struct page *page, unsigned long *flags)
>   	/*
>   	 * It's guaranteed that pc->mem_cgroup never changes while
>   	 * lock is held because a routine modifies pc->mem_cgroup
> -	 * should take move_lock_page_cgroup().
> +	 * should take move_lock_mem_cgroup().
>   	 */
>   	move_unlock_mem_cgroup(pc->mem_cgroup, flags);
>   }
> 



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

[PATCH RFC] mm/memcg: calculate max hierarchy limit number instead of min

[Wanpeng Li]

From: Wanpeng Li <liwp@linux.vnet.ibm.com>

Since hierachical_memory_limit shows "of bytes of memory limit with
regard to hierarchy under which the memory cgroup is", the count should
calculate max hierarchy limit when use_hierarchy in order to show hierarchy
subtree limit. hierachical_memsw_limit is the same case.

Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
---
 mm/memcontrol.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 69a7d45..6392c0a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3929,10 +3929,10 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
 		unsigned long long *mem_limit, unsigned long long *memsw_limit)
 {
 	struct cgroup *cgroup;
-	unsigned long long min_limit, min_memsw_limit, tmp;
+	unsigned long long max_limit, max_memsw_limit, tmp;
 
-	min_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
-	min_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
+	max_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
+	max_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
 	cgroup = memcg->css.cgroup;
 	if (!memcg->use_hierarchy)
 		goto out;
@@ -3943,13 +3943,13 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
 		if (!memcg->use_hierarchy)
 			break;
 		tmp = res_counter_read_u64(&memcg->res, RES_LIMIT);
-		min_limit = min(min_limit, tmp);
+		max_limit = max(max_limit, tmp);
 		tmp = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
-		min_memsw_limit = min(min_memsw_limit, tmp);
+		max_memsw_limit = max(max_memsw_limit, tmp);
 	}
 out:
-	*mem_limit = min_limit;
-	*memsw_limit = min_memsw_limit;
+	*mem_limit = max_limit;
+	*memsw_limit = max_memsw_limit;
 }
 
 static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
-- 
1.7.5.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 RFC] mm/memcg: calculate max hierarchy limit number instead of min

[Michal Hocko]

On Wed 11-07-12 21:24:41, Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> Since hierachical_memory_limit shows "of bytes of memory limit with
> regard to hierarchy under which the memory cgroup is", the count should
> calculate max hierarchy limit when use_hierarchy in order to show hierarchy
> subtree limit. hierachical_memsw_limit is the same case.

No the patch is wrong. The hierarchical limit says when we start
reclaiming in the hierarchy and that one is triggered on smallest limit
up the way to the hierarchy root.

What are you trying to accomplish here?

> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
> ---
>  mm/memcontrol.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 69a7d45..6392c0a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3929,10 +3929,10 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>  		unsigned long long *mem_limit, unsigned long long *memsw_limit)
>  {
>  	struct cgroup *cgroup;
> -	unsigned long long min_limit, min_memsw_limit, tmp;
> +	unsigned long long max_limit, max_memsw_limit, tmp;
>  
> -	min_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> -	min_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> +	max_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> +	max_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>  	cgroup = memcg->css.cgroup;
>  	if (!memcg->use_hierarchy)
>  		goto out;
> @@ -3943,13 +3943,13 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>  		if (!memcg->use_hierarchy)
>  			break;
>  		tmp = res_counter_read_u64(&memcg->res, RES_LIMIT);
> -		min_limit = min(min_limit, tmp);
> +		max_limit = max(max_limit, tmp);
>  		tmp = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> -		min_memsw_limit = min(min_memsw_limit, tmp);
> +		max_memsw_limit = max(max_memsw_limit, tmp);
>  	}
>  out:
> -	*mem_limit = min_limit;
> -	*memsw_limit = min_memsw_limit;
> +	*mem_limit = max_limit;
> +	*memsw_limit = max_memsw_limit;
>  }
>  
>  static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
> -- 
> 1.7.5.4
> 

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
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 RFC] mm/memcg: calculate max hierarchy limit number instead of min

[Wanpeng Li]

On Wed, Jul 11, 2012 at 03:47:57PM +0200, Michal Hocko wrote:
>On Wed 11-07-12 21:24:41, Wanpeng Li wrote:
>> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> 
>> Since hierachical_memory_limit shows "of bytes of memory limit with
>> regard to hierarchy under which the memory cgroup is", the count should
>> calculate max hierarchy limit when use_hierarchy in order to show hierarchy
>> subtree limit. hierachical_memsw_limit is the same case.
>
>No the patch is wrong. The hierarchical limit says when we start
>reclaiming in the hierarchy and that one is triggered on smallest limit
>up the way to the hierarchy root.

I see function mem_cgroup_hierachy_reclaim is removal, and hierarchical
reclaim is still work? Could you explain me how it works in details, 
thank you for your time Michal.

Thanks & Best Regards,
Wanpeng Li
>
>What are you trying to accomplish here?
>
>> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
>> ---
>>  mm/memcontrol.c |   14 +++++++-------
>>  1 files changed, 7 insertions(+), 7 deletions(-)
>> 
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 69a7d45..6392c0a 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -3929,10 +3929,10 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>>  		unsigned long long *mem_limit, unsigned long long *memsw_limit)
>>  {
>>  	struct cgroup *cgroup;
>> -	unsigned long long min_limit, min_memsw_limit, tmp;
>> +	unsigned long long max_limit, max_memsw_limit, tmp;
>>  
>> -	min_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
>> -	min_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>> +	max_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
>> +	max_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>>  	cgroup = memcg->css.cgroup;
>>  	if (!memcg->use_hierarchy)
>>  		goto out;
>> @@ -3943,13 +3943,13 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>>  		if (!memcg->use_hierarchy)
>>  			break;
>>  		tmp = res_counter_read_u64(&memcg->res, RES_LIMIT);
>> -		min_limit = min(min_limit, tmp);
>> +		max_limit = max(max_limit, tmp);
>>  		tmp = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>> -		min_memsw_limit = min(min_memsw_limit, tmp);
>> +		max_memsw_limit = max(max_memsw_limit, tmp);
>>  	}
>>  out:
>> -	*mem_limit = min_limit;
>> -	*memsw_limit = min_memsw_limit;
>> +	*mem_limit = max_limit;
>> +	*memsw_limit = max_memsw_limit;
>>  }
>>  
>>  static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
>> -- 
>> 1.7.5.4
>> 
>
>-- 
>Michal Hocko
>SUSE Labs
>SUSE LINUX s.r.o.
>Lihovarska 1060/12
>190 00 Praha 9    
>Czech Republic

--
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 RFC] mm/memcg: calculate max hierarchy limit number instead of min

[Michal Hocko]

On Thu 12-07-12 17:32:11, Wanpeng Li wrote:
> On Wed, Jul 11, 2012 at 03:47:57PM +0200, Michal Hocko wrote:
> >On Wed 11-07-12 21:24:41, Wanpeng Li wrote:
> >> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> >> 
> >> Since hierachical_memory_limit shows "of bytes of memory limit with
> >> regard to hierarchy under which the memory cgroup is", the count should
> >> calculate max hierarchy limit when use_hierarchy in order to show hierarchy
> >> subtree limit. hierachical_memsw_limit is the same case.
> >
> >No the patch is wrong. The hierarchical limit says when we start
> >reclaiming in the hierarchy and that one is triggered on smallest limit
> >up the way to the hierarchy root.
> 
> I see function mem_cgroup_hierachy_reclaim is removal, and hierarchical
> reclaim is still work? 

I am not aware of that.

> Could you explain me how it works in details, thank you for your time
> Michal.

I am not sure I understand what you are interested in. 

> 
> Thanks & Best Regards,
> Wanpeng Li
> >
> >What are you trying to accomplish here?
> >
> >> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
> >> ---
> >>  mm/memcontrol.c |   14 +++++++-------
> >>  1 files changed, 7 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index 69a7d45..6392c0a 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -3929,10 +3929,10 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
> >>  		unsigned long long *mem_limit, unsigned long long *memsw_limit)
> >>  {
> >>  	struct cgroup *cgroup;
> >> -	unsigned long long min_limit, min_memsw_limit, tmp;
> >> +	unsigned long long max_limit, max_memsw_limit, tmp;
> >>  
> >> -	min_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> >> -	min_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> >> +	max_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> >> +	max_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> >>  	cgroup = memcg->css.cgroup;
> >>  	if (!memcg->use_hierarchy)
> >>  		goto out;
> >> @@ -3943,13 +3943,13 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
> >>  		if (!memcg->use_hierarchy)
> >>  			break;
> >>  		tmp = res_counter_read_u64(&memcg->res, RES_LIMIT);
> >> -		min_limit = min(min_limit, tmp);
> >> +		max_limit = max(max_limit, tmp);
> >>  		tmp = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> >> -		min_memsw_limit = min(min_memsw_limit, tmp);
> >> +		max_memsw_limit = max(max_memsw_limit, tmp);
> >>  	}
> >>  out:
> >> -	*mem_limit = min_limit;
> >> -	*memsw_limit = min_memsw_limit;
> >> +	*mem_limit = max_limit;
> >> +	*memsw_limit = max_memsw_limit;
> >>  }
> >>  
> >>  static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
> >> -- 
> >> 1.7.5.4
> >> 
> >
> >-- 
> >Michal Hocko
> >SUSE Labs
> >SUSE LINUX s.r.o.
> >Lihovarska 1060/12
> >190 00 Praha 9    
> >Czech Republic

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
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 RFC] mm/memcg: calculate max hierarchy limit number instead of min

[Kamezawa Hiroyuki]

(2012/07/11 22:24), Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> Since hierachical_memory_limit shows "of bytes of memory limit with
> regard to hierarchy under which the memory cgroup is", the count should
> calculate max hierarchy limit when use_hierarchy in order to show hierarchy
> subtree limit. hierachical_memsw_limit is the same case.
> 
> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>

Hm ? What is the hierarchical limit for 'C' in following tree ?

A  ---  limit=1G 
 \
  B --  limit=500M
   \
    C - unlimtied

Thanks,
-Kame


> ---
>   mm/memcontrol.c |   14 +++++++-------
>   1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 69a7d45..6392c0a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3929,10 +3929,10 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>   		unsigned long long *mem_limit, unsigned long long *memsw_limit)
>   {
>   	struct cgroup *cgroup;
> -	unsigned long long min_limit, min_memsw_limit, tmp;
> +	unsigned long long max_limit, max_memsw_limit, tmp;
>   
> -	min_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> -	min_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> +	max_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> +	max_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>   	cgroup = memcg->css.cgroup;
>   	if (!memcg->use_hierarchy)
>   		goto out;
> @@ -3943,13 +3943,13 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>   		if (!memcg->use_hierarchy)
>   			break;
>   		tmp = res_counter_read_u64(&memcg->res, RES_LIMIT);
> -		min_limit = min(min_limit, tmp);
> +		max_limit = max(max_limit, tmp);
>   		tmp = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
> -		min_memsw_limit = min(min_memsw_limit, tmp);
> +		max_memsw_limit = max(max_memsw_limit, tmp);
>   	}
>   out:
> -	*mem_limit = min_limit;
> -	*memsw_limit = min_memsw_limit;
> +	*mem_limit = max_limit;
> +	*memsw_limit = max_memsw_limit;
>   }
>   
>   static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
> 



--
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 RFC] mm/memcg: calculate max hierarchy limit number instead of min

[Wanpeng Li]

On Thu, Jul 19, 2012 at 03:07:20PM +0900, Kamezawa Hiroyuki wrote:
>(2012/07/11 22:24), Wanpeng Li wrote:
>> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> 
>> Since hierachical_memory_limit shows "of bytes of memory limit with
>> regard to hierarchy under which the memory cgroup is", the count should
>> calculate max hierarchy limit when use_hierarchy in order to show hierarchy
>> subtree limit. hierachical_memsw_limit is the same case.
>> 
>> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
>
>Hm ? What is the hierarchical limit for 'C' in following tree ?
>
>A  ---  limit=1G 
> \
>  B --  limit=500M
>   \
>    C - unlimtied
>
Hmm, thank you Kame. :-)

Regards,
Wanpeng Li 

>Thanks,
>-Kame
>
>
>> ---
>>   mm/memcontrol.c |   14 +++++++-------
>>   1 files changed, 7 insertions(+), 7 deletions(-)
>> 
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 69a7d45..6392c0a 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -3929,10 +3929,10 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>>   		unsigned long long *mem_limit, unsigned long long *memsw_limit)
>>   {
>>   	struct cgroup *cgroup;
>> -	unsigned long long min_limit, min_memsw_limit, tmp;
>> +	unsigned long long max_limit, max_memsw_limit, tmp;
>>   
>> -	min_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
>> -	min_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>> +	max_limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
>> +	max_memsw_limit = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>>   	cgroup = memcg->css.cgroup;
>>   	if (!memcg->use_hierarchy)
>>   		goto out;
>> @@ -3943,13 +3943,13 @@ static void memcg_get_hierarchical_limit(struct mem_cgroup *memcg,
>>   		if (!memcg->use_hierarchy)
>>   			break;
>>   		tmp = res_counter_read_u64(&memcg->res, RES_LIMIT);
>> -		min_limit = min(min_limit, tmp);
>> +		max_limit = max(max_limit, tmp);
>>   		tmp = res_counter_read_u64(&memcg->memsw, RES_LIMIT);
>> -		min_memsw_limit = min(min_memsw_limit, tmp);
>> +		max_memsw_limit = max(max_memsw_limit, tmp);
>>   	}
>>   out:
>> -	*mem_limit = min_limit;
>> -	*memsw_limit = min_memsw_limit;
>> +	*mem_limit = max_limit;
>> +	*memsw_limit = max_memsw_limit;
>>   }
>>   
>>   static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
>> 
>
>
>
>--
>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>

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

[PATCH] mm/memcg: wrap mem_cgroup_from_css function

[Wanpeng Li]

wrap mem_cgroup_from_css function to clarify get mem cgroup
from cgroup_subsys_state.

Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org
---
 mm/memcontrol.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 58a08fc..20f6a15 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
 #include <net/sock.h>
 #include <net/ip.h>
 
+static inline
+struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
+{
+	return container_of(s, struct mem_cgroup, css);
+}
+
 static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
 void sock_update_memcg(struct sock *sk)
 {
@@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
 
 struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
 {
-	return container_of(cgroup_subsys_state(cont,
+	return mem_cgroup_from_css(cgroup_subsys_state(cont,
 				mem_cgroup_subsys_id), struct mem_cgroup,
 				css);
 }
@@ -835,7 +841,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
 	if (unlikely(!p))
 		return NULL;
 
-	return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
+	return mem_cgroup_from_css(task_subsys_state(p, mem_cgroup_subsys_id),
 				struct mem_cgroup, css);
 }
 
@@ -922,7 +928,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
 		css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
 		if (css) {
 			if (css == &root->css || css_tryget(css))
-				memcg = container_of(css,
+				memcg = mem_cgroup_from_css(css,
 						     struct mem_cgroup, css);
 		} else
 			id = 0;
@@ -2406,7 +2412,7 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
 	css = css_lookup(&mem_cgroup_subsys, id);
 	if (!css)
 		return NULL;
-	return container_of(css, struct mem_cgroup, css);
+	return mem_cgroup_from_css(css, struct mem_cgroup, css);
 }
 
 struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
-- 
1.7.5.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: wrap mem_cgroup_from_css function

[Andrew Morton]

On Wed, 18 Jul 2012 11:05:30 +0800
Wanpeng Li <liwanp@linux.vnet.ibm.com> wrote:

> wrap mem_cgroup_from_css function to clarify get mem cgroup
> from cgroup_subsys_state.

This certainly adds clarity.

But it also adds a little more type-safety - these container_of() calls
can be invoked against *any* struct which has a field called "css". 
With your patch, we add a check that the code is indeed using a
cgroup_subsys_state*.  A small thing, but it's all good.


I changed the patch title to the more idiomatic "memcg: add
mem_cgroup_from_css() helper" and rewrote the changelog to

: Add a mem_cgroup_from_css() helper to replace open-coded invokations of
: container_of().  To clarify the code and to add a little more type safety.

> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>  #include <net/sock.h>
>  #include <net/ip.h>
>  
> +static inline
> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
> +{
> +	return container_of(s, struct mem_cgroup, css);
> +}

And with great self-control, I avoided renaming this to
memcg_from_css().  Sigh.  I guess all that extra typing has cardio
benefits.

--
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: wrap mem_cgroup_from_css function

[Wanpeng Li]

On Wed, Jul 18, 2012 at 02:36:12PM -0700, Andrew Morton wrote:
>On Wed, 18 Jul 2012 11:05:30 +0800
>Wanpeng Li <liwanp@linux.vnet.ibm.com> wrote:
>
>> wrap mem_cgroup_from_css function to clarify get mem cgroup
>> from cgroup_subsys_state.
>
>This certainly adds clarity.
>
>But it also adds a little more type-safety - these container_of() calls
>can be invoked against *any* struct which has a field called "css". 
>With your patch, we add a check that the code is indeed using a
>cgroup_subsys_state*.  A small thing, but it's all good.
>
>
>I changed the patch title to the more idiomatic "memcg: add
>mem_cgroup_from_css() helper" and rewrote the changelog to
>
>: Add a mem_cgroup_from_css() helper to replace open-coded invokations of
>: container_of().  To clarify the code and to add a little more type safety.
>
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>>  #include <net/sock.h>
>>  #include <net/ip.h>
>>  
>> +static inline
>> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>> +{
>> +	return container_of(s, struct mem_cgroup, css);
>> +}
>
>And with great self-control, I avoided renaming this to
>memcg_from_css().  Sigh.  I guess all that extra typing has cardio
>benefits.

Thank you for your time, Andrew. :-)

Thanks & Best Regards,
Wanpeng Li

--
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: wrap mem_cgroup_from_css function

[Kirill A. Shutemov]

On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
> wrap mem_cgroup_from_css function to clarify get mem cgroup
> from cgroup_subsys_state.
> 
> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  mm/memcontrol.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 58a08fc..20f6a15 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>  #include <net/sock.h>
>  #include <net/ip.h>
>  
> +static inline
> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
> +{
> +	return container_of(s, struct mem_cgroup, css);
> +}
> +
>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
>  void sock_update_memcg(struct sock *sk)
>  {
> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
>  
>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>  {
> -	return container_of(cgroup_subsys_state(cont,
> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
>  				mem_cgroup_subsys_id), struct mem_cgroup,
>  				css);

Hm?.. Here and below too many args to mem_cgroup_from_css().
Have you tested the code?

>  }
> @@ -835,7 +841,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>  	if (unlikely(!p))
>  		return NULL;
>  
> -	return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
> +	return mem_cgroup_from_css(task_subsys_state(p, mem_cgroup_subsys_id),
>  				struct mem_cgroup, css);
>  }
>  
> @@ -922,7 +928,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
>  		css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
>  		if (css) {
>  			if (css == &root->css || css_tryget(css))
> -				memcg = container_of(css,
> +				memcg = mem_cgroup_from_css(css,
>  						     struct mem_cgroup, css);
>  		} else
>  			id = 0;
> @@ -2406,7 +2412,7 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
>  	css = css_lookup(&mem_cgroup_subsys, id);
>  	if (!css)
>  		return NULL;
> -	return container_of(css, struct mem_cgroup, css);
> +	return mem_cgroup_from_css(css, struct mem_cgroup, css);
>  }
>  
>  struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
> -- 
> 1.7.5.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>

-- 
 Kirill A. Shutemov

--
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: wrap mem_cgroup_from_css function

[Wanpeng Li]

On Thu, Jul 19, 2012 at 12:14:20PM +0300, Kirill A. Shutemov wrote:
>On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
>> wrap mem_cgroup_from_css function to clarify get mem cgroup
>> from cgroup_subsys_state.
>> 
>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> Cc: Michal Hocko <mhocko@suse.cz>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
>> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  mm/memcontrol.c |   14 ++++++++++----
>>  1 files changed, 10 insertions(+), 4 deletions(-)
>> 
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 58a08fc..20f6a15 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>>  #include <net/sock.h>
>>  #include <net/ip.h>
>>  
>> +static inline
>> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>> +{
>> +	return container_of(s, struct mem_cgroup, css);
>> +}
>> +
>>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
>>  void sock_update_memcg(struct sock *sk)
>>  {
>> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
>>  
>>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>>  {
>> -	return container_of(cgroup_subsys_state(cont,
>> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
>>  				mem_cgroup_subsys_id), struct mem_cgroup,
>>  				css);
>
>Hm?.. Here and below too many args to mem_cgroup_from_css().
>Have you tested the code?

Hi, what's the meaning of "two many"?

cgroup_subsys_state(cont, mem_cgroup_subsys_id) and 
task_subsys_state(p, mem_cgroup_subsys_id) both are 
just one arg in mem_cgroup_from_css. :-)

>
>>  }
>> @@ -835,7 +841,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>>  	if (unlikely(!p))
>>  		return NULL;
>>  
>> -	return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
>> +	return mem_cgroup_from_css(task_subsys_state(p, mem_cgroup_subsys_id),
>>  				struct mem_cgroup, css);
>>  }
>>  
>> @@ -922,7 +928,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
>>  		css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
>>  		if (css) {
>>  			if (css == &root->css || css_tryget(css))
>> -				memcg = container_of(css,
>> +				memcg = mem_cgroup_from_css(css,
>>  						     struct mem_cgroup, css);
>>  		} else
>>  			id = 0;
>> @@ -2406,7 +2412,7 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
>>  	css = css_lookup(&mem_cgroup_subsys, id);
>>  	if (!css)
>>  		return NULL;
>> -	return container_of(css, struct mem_cgroup, css);
>> +	return mem_cgroup_from_css(css, struct mem_cgroup, css);
>>  }
>>  
>>  struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
>> -- 
>> 1.7.5.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>
>
>-- 
> Kirill A. Shutemov

--
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: wrap mem_cgroup_from_css function

[Kirill A. Shutemov]

On Thu, Jul 19, 2012 at 05:23:09PM +0800, Wanpeng Li wrote:
> On Thu, Jul 19, 2012 at 12:14:20PM +0300, Kirill A. Shutemov wrote:
> >On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
> >> wrap mem_cgroup_from_css function to clarify get mem cgroup
> >> from cgroup_subsys_state.
> >> 
> >> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> >> Cc: Michal Hocko <mhocko@suse.cz>
> >> Cc: Johannes Weiner <hannes@cmpxchg.org>
> >> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
> >> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> >> Cc: linux-kernel@vger.kernel.org
> >> ---
> >>  mm/memcontrol.c |   14 ++++++++++----
> >>  1 files changed, 10 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index 58a08fc..20f6a15 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
> >>  #include <net/sock.h>
> >>  #include <net/ip.h>
> >>  
> >> +static inline
> >> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
> >> +{
> >> +	return container_of(s, struct mem_cgroup, css);
> >> +}
> >> +
> >>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
> >>  void sock_update_memcg(struct sock *sk)
> >>  {
> >> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
> >>  
> >>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
> >>  {
> >> -	return container_of(cgroup_subsys_state(cont,
> >> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
> >>  				mem_cgroup_subsys_id), struct mem_cgroup,
> >>  				css);
> >
> >Hm?.. Here and below too many args to mem_cgroup_from_css().
> >Have you tested the code?
> 
> Hi, what's the meaning of "two many"?
> 
> cgroup_subsys_state(cont, mem_cgroup_subsys_id) and 
> task_subsys_state(p, mem_cgroup_subsys_id) both are 
> just one arg in mem_cgroup_from_css. :-)

Em.. I guess my eyes are broken. %)

By the patch I see that mem_cgroup_from_css() here has tree arguments:

1. cgroup_subsys_state(cont, mem_cgroup_subsys_id)
2. struct mem_cgroup
3. css

Where's my parser is broken?

-- 
 Kirill A. Shutemov

--
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: wrap mem_cgroup_from_css function

[Gavin Shan]

On Thu, Jul 19, 2012 at 05:23:09PM +0800, Wanpeng Li wrote:
>On Thu, Jul 19, 2012 at 12:14:20PM +0300, Kirill A. Shutemov wrote:
>>On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
>>> wrap mem_cgroup_from_css function to clarify get mem cgroup
>>> from cgroup_subsys_state.
>>> 
>>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>> Cc: Michal Hocko <mhocko@suse.cz>
>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
>>> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>  mm/memcontrol.c |   14 ++++++++++----
>>>  1 files changed, 10 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 58a08fc..20f6a15 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>>>  #include <net/sock.h>
>>>  #include <net/ip.h>
>>>  
>>> +static inline
>>> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>>> +{
>>> +	return container_of(s, struct mem_cgroup, css);
>>> +}
>>> +
>>>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
>>>  void sock_update_memcg(struct sock *sk)
>>>  {
>>> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
>>>  
>>>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>>>  {
>>> -	return container_of(cgroup_subsys_state(cont,
>>> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
>>>  				mem_cgroup_subsys_id), struct mem_cgroup,
>>>  				css);
>>
>>Hm?.. Here and below too many args to mem_cgroup_from_css().
>>Have you tested the code?
>
>Hi, what's the meaning of "two many"?
>

It might be the typo for "two" here. I think it would be
"too". However, it seems that you had pass "two" more arguments
here to mem_cgroup_from_css() since the function only takes "one"
parameter as you implemented before.

+struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)

+   return mem_cgroup_from_css(cgroup_subsys_state(cont,
+		mem_cgroup_subsys_id), struct mem_cgroup,
+		css);

Thanks,
Gavin

>cgroup_subsys_state(cont, mem_cgroup_subsys_id) and 
>task_subsys_state(p, mem_cgroup_subsys_id) both are 
>just one arg in mem_cgroup_from_css. :-)
>
>>
>>>  }
>>> @@ -835,7 +841,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>>>  	if (unlikely(!p))
>>>  		return NULL;
>>>  
>>> -	return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
>>> +	return mem_cgroup_from_css(task_subsys_state(p, mem_cgroup_subsys_id),
>>>  				struct mem_cgroup, css);
>>>  }
>>>  
>>> @@ -922,7 +928,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
>>>  		css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
>>>  		if (css) {
>>>  			if (css == &root->css || css_tryget(css))
>>> -				memcg = container_of(css,
>>> +				memcg = mem_cgroup_from_css(css,
>>>  						     struct mem_cgroup, css);
>>>  		} else
>>>  			id = 0;
>>> @@ -2406,7 +2412,7 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
>>>  	css = css_lookup(&mem_cgroup_subsys, id);
>>>  	if (!css)
>>>  		return NULL;
>>> -	return container_of(css, struct mem_cgroup, css);
>>> +	return mem_cgroup_from_css(css, struct mem_cgroup, css);
>>>  }
>>>  
>>>  struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
>>> -- 
>>> 1.7.5.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>
>>
>>-- 
>> Kirill A. Shutemov

--
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: wrap mem_cgroup_from_css function

[Kirill A. Shutemov]

On Thu, Jul 19, 2012 at 05:38:35PM +0800, Gavin Shan wrote:
> On Thu, Jul 19, 2012 at 05:23:09PM +0800, Wanpeng Li wrote:
> >On Thu, Jul 19, 2012 at 12:14:20PM +0300, Kirill A. Shutemov wrote:
> >>On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
> >>> wrap mem_cgroup_from_css function to clarify get mem cgroup
> >>> from cgroup_subsys_state.
> >>> 
> >>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> >>> Cc: Michal Hocko <mhocko@suse.cz>
> >>> Cc: Johannes Weiner <hannes@cmpxchg.org>
> >>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >>> Cc: Andrew Morton <akpm@linux-foundation.org>
> >>> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
> >>> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> >>> Cc: linux-kernel@vger.kernel.org
> >>> ---
> >>>  mm/memcontrol.c |   14 ++++++++++----
> >>>  1 files changed, 10 insertions(+), 4 deletions(-)
> >>> 
> >>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >>> index 58a08fc..20f6a15 100644
> >>> --- a/mm/memcontrol.c
> >>> +++ b/mm/memcontrol.c
> >>> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
> >>>  #include <net/sock.h>
> >>>  #include <net/ip.h>
> >>>  
> >>> +static inline
> >>> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
> >>> +{
> >>> +	return container_of(s, struct mem_cgroup, css);
> >>> +}
> >>> +
> >>>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
> >>>  void sock_update_memcg(struct sock *sk)
> >>>  {
> >>> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
> >>>  
> >>>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
> >>>  {
> >>> -	return container_of(cgroup_subsys_state(cont,
> >>> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
> >>>  				mem_cgroup_subsys_id), struct mem_cgroup,
> >>>  				css);
> >>
> >>Hm?.. Here and below too many args to mem_cgroup_from_css().
> >>Have you tested the code?
> >
> >Hi, what's the meaning of "two many"?
> >
> 
> It might be the typo for "two" here.

Oops.. You're right.

-- 
 Kirill A. Shutemov

--
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: wrap mem_cgroup_from_css function

[Wanpeng Li]

[-- Attachment #1: Type: text/plain, Size: 4360 bytes --]

On Thu, Jul 19, 2012 at 05:38:35PM +0800, Gavin Shan wrote:
>On Thu, Jul 19, 2012 at 05:23:09PM +0800, Wanpeng Li wrote:
>>On Thu, Jul 19, 2012 at 12:14:20PM +0300, Kirill A. Shutemov wrote:
>>>On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
>>>> wrap mem_cgroup_from_css function to clarify get mem cgroup
>>>> from cgroup_subsys_state.
>>>> 
>>>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>>> Cc: Michal Hocko <mhocko@suse.cz>
>>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
>>>> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> ---
>>>>  mm/memcontrol.c |   14 ++++++++++----
>>>>  1 files changed, 10 insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>>> index 58a08fc..20f6a15 100644
>>>> --- a/mm/memcontrol.c
>>>> +++ b/mm/memcontrol.c
>>>> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>>>>  #include <net/sock.h>
>>>>  #include <net/ip.h>
>>>>  
>>>> +static inline
>>>> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>>>> +{
>>>> +	return container_of(s, struct mem_cgroup, css);
>>>> +}
>>>> +
>>>>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
>>>>  void sock_update_memcg(struct sock *sk)
>>>>  {
>>>> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
>>>>  
>>>>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>>>>  {
>>>> -	return container_of(cgroup_subsys_state(cont,
>>>> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
>>>>  				mem_cgroup_subsys_id), struct mem_cgroup,
>>>>  				css);
>>>
>>>Hm?.. Here and below too many args to mem_cgroup_from_css().
>>>Have you tested the code?
>>
>>Hi, what's the meaning of "two many"?
>>
>
>It might be the typo for "two" here. I think it would be
>"too". However, it seems that you had pass "two" more arguments
>here to mem_cgroup_from_css() since the function only takes "one"
>parameter as you implemented before.
>
>+struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>
>+   return mem_cgroup_from_css(cgroup_subsys_state(cont,
>+		mem_cgroup_subsys_id), struct mem_cgroup,
>+		css);
>

Hi Andrew, 

Sorry for make a mistake. Please drop this patch from linux-mm 
and merged the new one. BTW, thanks Gavin and Kirill.

Best Regards,
Wanpeng Li


>Thanks,
>Gavin
>
>>cgroup_subsys_state(cont, mem_cgroup_subsys_id) and 
>>task_subsys_state(p, mem_cgroup_subsys_id) both are 
>>just one arg in mem_cgroup_from_css. :-)
>>
>>>
>>>>  }
>>>> @@ -835,7 +841,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>>>>  	if (unlikely(!p))
>>>>  		return NULL;
>>>>  
>>>> -	return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
>>>> +	return mem_cgroup_from_css(task_subsys_state(p, mem_cgroup_subsys_id),
>>>>  				struct mem_cgroup, css);
>>>>  }
>>>>  
>>>> @@ -922,7 +928,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
>>>>  		css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
>>>>  		if (css) {
>>>>  			if (css == &root->css || css_tryget(css))
>>>> -				memcg = container_of(css,
>>>> +				memcg = mem_cgroup_from_css(css,
>>>>  						     struct mem_cgroup, css);
>>>>  		} else
>>>>  			id = 0;
>>>> @@ -2406,7 +2412,7 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
>>>>  	css = css_lookup(&mem_cgroup_subsys, id);
>>>>  	if (!css)
>>>>  		return NULL;
>>>> -	return container_of(css, struct mem_cgroup, css);
>>>> +	return mem_cgroup_from_css(css, struct mem_cgroup, css);
>>>>  }
>>>>  
>>>>  struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
>>>> -- 
>>>> 1.7.5.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>
>>>
>>>-- 
>>> Kirill A. Shutemov
>
>--
>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>

[-- Attachment #2: 0001-memcg-add-mem_cgroup_from_css-helper.patch --]
[-- Type: text/x-diff, Size: 0 bytes --]

Re: [PATCH] mm/memcg: wrap mem_cgroup_from_css function

[Gavin Shan]

On Thu, Jul 19, 2012 at 05:23:09PM +0800, Wanpeng Li wrote:
>On Thu, Jul 19, 2012 at 12:14:20PM +0300, Kirill A. Shutemov wrote:
>>On Wed, Jul 18, 2012 at 11:05:30AM +0800, Wanpeng Li wrote:
>>> wrap mem_cgroup_from_css function to clarify get mem cgroup
>>> from cgroup_subsys_state.
>>> 
>>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>> Cc: Michal Hocko <mhocko@suse.cz>
>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
>>> Cc: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> ---
>>>  mm/memcontrol.c |   14 ++++++++++----
>>>  1 files changed, 10 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 58a08fc..20f6a15 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -396,6 +396,12 @@ static void mem_cgroup_put(struct mem_cgroup *memcg);
>>>  #include <net/sock.h>
>>>  #include <net/ip.h>
>>>  
>>> +static inline
>>> +struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)
>>> +{
>>> +	return container_of(s, struct mem_cgroup, css);
>>> +}
>>> +
>>>  static bool mem_cgroup_is_root(struct mem_cgroup *memcg);
>>>  void sock_update_memcg(struct sock *sk)
>>>  {
>>> @@ -820,7 +826,7 @@ static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
>>>  
>>>  struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>>>  {
>>> -	return container_of(cgroup_subsys_state(cont,
>>> +	return mem_cgroup_from_css(cgroup_subsys_state(cont,
>>>  				mem_cgroup_subsys_id), struct mem_cgroup,
>>>  				css);
>>
>>Hm?.. Here and below too many args to mem_cgroup_from_css().
>>Have you tested the code?
>
>Hi, what's the meaning of "two many"?
>

It might be the typo for "two" here. I think it would be
"too". However, it seems that you had pass "two" more arguments
here to mem_cgroup_from_css() since the function only takes "one"
parameter as you implemented before.

+struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *s)

+   return mem_cgroup_from_css(cgroup_subsys_state(cont,
+		mem_cgroup_subsys_id), struct mem_cgroup,
+		css);

Thanks,
Gavin

>cgroup_subsys_state(cont, mem_cgroup_subsys_id) and 
>task_subsys_state(p, mem_cgroup_subsys_id) both are 
>just one arg in mem_cgroup_from_css. :-)
>
>>
>>>  }
>>> @@ -835,7 +841,7 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>>>  	if (unlikely(!p))
>>>  		return NULL;
>>>  
>>> -	return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
>>> +	return mem_cgroup_from_css(task_subsys_state(p, mem_cgroup_subsys_id),
>>>  				struct mem_cgroup, css);
>>>  }
>>>  
>>> @@ -922,7 +928,7 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
>>>  		css = css_get_next(&mem_cgroup_subsys, id + 1, &root->css, &id);
>>>  		if (css) {
>>>  			if (css == &root->css || css_tryget(css))
>>> -				memcg = container_of(css,
>>> +				memcg = mem_cgroup_from_css(css,
>>>  						     struct mem_cgroup, css);
>>>  		} else
>>>  			id = 0;
>>> @@ -2406,7 +2412,7 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
>>>  	css = css_lookup(&mem_cgroup_subsys, id);
>>>  	if (!css)
>>>  		return NULL;
>>> -	return container_of(css, struct mem_cgroup, css);
>>> +	return mem_cgroup_from_css(css, struct mem_cgroup, css);
>>>  }
>>>  
>>>  struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
>>> -- 
>>> 1.7.5.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>
>>
>>-- 
>> Kirill A. Shutemov

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