linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children
@ 2012-07-05 10:55 Wanpeng Li
  2012-07-09  4:43 ` Kamezawa Hiroyuki
  2012-07-09 12:01 ` Michal Hocko
  0 siblings, 2 replies; 4+ messages in thread
From: Wanpeng Li @ 2012-07-05 10:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Johannes Weiner, Michal Hocko, KAMEZAWA Hiroyuki, linux-mm,
	linux-kernel, Wanpeng Li

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

When oom-kill-disable modified by the user and current memcg use_hierarchy,
the change can occur, provided the current memcg has no children. If it
has children, return -EBUSY is enough.

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

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 63e36e7..4b64fe0 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4521,11 +4521,14 @@ static int mem_cgroup_oom_control_write(struct cgroup *cgrp,
 
 	cgroup_lock();
 	/* oom-kill-disable is a flag for subhierarchy. */
-	if ((parent->use_hierarchy) ||
-	    (memcg->use_hierarchy && !list_empty(&cgrp->children))) {
+	if (parent->use_hierarchy) {
 		cgroup_unlock();
 		return -EINVAL;
+	} else if (memcg->use_hierarchy && !list_empty(&cgrp->children)) {
+		cgroup_unlock();
+		return -EBUSY;
 	}
+
 	memcg->oom_kill_disable = val;
 	if (!val)
 		memcg_oom_recover(memcg);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children
  2012-07-05 10:55 [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children Wanpeng Li
@ 2012-07-09  4:43 ` Kamezawa Hiroyuki
  2012-07-09  5:10   ` Wanpeng Li
  2012-07-09 12:01 ` Michal Hocko
  1 sibling, 1 reply; 4+ messages in thread
From: Kamezawa Hiroyuki @ 2012-07-09  4:43 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Andrew Morton, Johannes Weiner, Michal Hocko, linux-mm,
	linux-kernel

(2012/07/05 19:55), Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> When oom-kill-disable modified by the user and current memcg use_hierarchy,
> the change can occur, provided the current memcg has no children. If it
> has children, return -EBUSY is enough.
> 
> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>

I'm sorry what is the point ? You think -EBUSY should be returned in this case 
rather than -EINVAl ? Then, why ?


> ---
>   mm/memcontrol.c |    7 +++++--
>   1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 63e36e7..4b64fe0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4521,11 +4521,14 @@ static int mem_cgroup_oom_control_write(struct cgroup *cgrp,
>   
>   	cgroup_lock();
>   	/* oom-kill-disable is a flag for subhierarchy. */
> -	if ((parent->use_hierarchy) ||
> -	    (memcg->use_hierarchy && !list_empty(&cgrp->children))) {
> +	if (parent->use_hierarchy) {
>   		cgroup_unlock();
>   		return -EINVAL;
> +	} else if (memcg->use_hierarchy && !list_empty(&cgrp->children)) {
> +		cgroup_unlock();
> +		return -EBUSY;
>   	}
> +
>   	memcg->oom_kill_disable = val;
>   	if (!val)
>   		memcg_oom_recover(memcg);
> 




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children
  2012-07-09  4:43 ` Kamezawa Hiroyuki
@ 2012-07-09  5:10   ` Wanpeng Li
  0 siblings, 0 replies; 4+ messages in thread
From: Wanpeng Li @ 2012-07-09  5:10 UTC (permalink / raw)
  To: Kamezawa Hiroyuki
  Cc: Johannes Weiner, Michal Hocko, Andrew Morton, linux-mm,
	linux-kernel, Wanpeng Li

On Mon, Jul 09, 2012 at 01:43:23PM +0900, Kamezawa Hiroyuki wrote:
>(2012/07/05 19:55), Wanpeng Li wrote:
>> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> 
>> When oom-kill-disable modified by the user and current memcg use_hierarchy,
>> the change can occur, provided the current memcg has no children. If it
>> has children, return -EBUSY is enough.
>> 
>> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
>
>I'm sorry what is the point ? You think -EBUSY should be returned in this case 
>rather than -EINVAl ? Then, why ?

just like in function cmem_cgroup_hierarchy_write:

if((!parent_memcg || !parent_memcg->use_hierarchy) &&
		(val == 1 || val == 0) {
		if (list_empty(&cont->children))
			memcg->use_hierarchy = val;
		else
			return -EBUSY;
} else
		return = -EINVAL;

If memcg->use_hierarchy && has children memcg, the user can try again
if children memcg disappear. Or I miss something ....

Regards,
Wanpeng Li

>
>
>> ---
>>   mm/memcontrol.c |    7 +++++--
>>   1 files changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 63e36e7..4b64fe0 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4521,11 +4521,14 @@ static int mem_cgroup_oom_control_write(struct cgroup *cgrp,
>>   
>>   	cgroup_lock();
>>   	/* oom-kill-disable is a flag for subhierarchy. */
>> -	if ((parent->use_hierarchy) ||
>> -	    (memcg->use_hierarchy && !list_empty(&cgrp->children))) {
>> +	if (parent->use_hierarchy) {
>>   		cgroup_unlock();
>>   		return -EINVAL;
>> +	} else if (memcg->use_hierarchy && !list_empty(&cgrp->children)) {
>> +		cgroup_unlock();
>> +		return -EBUSY;
>>   	}
>> +
>>   	memcg->oom_kill_disable = val;
>>   	if (!val)
>>   		memcg_oom_recover(memcg);
>> 
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children
  2012-07-05 10:55 [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children Wanpeng Li
  2012-07-09  4:43 ` Kamezawa Hiroyuki
@ 2012-07-09 12:01 ` Michal Hocko
  1 sibling, 0 replies; 4+ messages in thread
From: Michal Hocko @ 2012-07-09 12:01 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Andrew Morton, Johannes Weiner, KAMEZAWA Hiroyuki, linux-mm,
	linux-kernel

On Thu 05-07-12 18:55:08, Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> When oom-kill-disable modified by the user and current memcg use_hierarchy,
> the change can occur, provided the current memcg has no children. If it
> has children, return -EBUSY is enough.

I do not think EBUSY makes any difference. I would much rather see the
test go away...

> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
> ---
>  mm/memcontrol.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 63e36e7..4b64fe0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4521,11 +4521,14 @@ static int mem_cgroup_oom_control_write(struct cgroup *cgrp,
>  
>  	cgroup_lock();
>  	/* oom-kill-disable is a flag for subhierarchy. */
> -	if ((parent->use_hierarchy) ||
> -	    (memcg->use_hierarchy && !list_empty(&cgrp->children))) {
> +	if (parent->use_hierarchy) {
>  		cgroup_unlock();
>  		return -EINVAL;
> +	} else if (memcg->use_hierarchy && !list_empty(&cgrp->children)) {
> +		cgroup_unlock();
> +		return -EBUSY;
>  	}
> +
>  	memcg->oom_kill_disable = val;
>  	if (!val)
>  		memcg_oom_recover(memcg);
> -- 
> 1.7.5.4
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-07-09 12:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-05 10:55 [PATCH] mm/memcg: return -EBUSY when oom-kill-disable modified and memcg use_hierarchy, has children Wanpeng Li
2012-07-09  4:43 ` Kamezawa Hiroyuki
2012-07-09  5:10   ` Wanpeng Li
2012-07-09 12:01 ` Michal Hocko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).