[PATCH 1/2][-mm][resend] res_counter limit change support ebusy

[PATCH 1/2][-mm][resend] res_counter limit change support ebusy

[KAMEZAWA Hiroyuki]

Add an interface to set limit. This is necessary to memory resource controller
because it shrinks usage at set limit.

(*) Other controller may not need this interface to shrink usage because
    shrinking is not necessary or impossible.

This is an enhancement.
named as res_counter-limit-change-ebusy.patch

Changelog:
  - fixed white space bug.

Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Acked-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


 include/linux/res_counter.h |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

Index: linux-2.6.26-rc8-mm1/include/linux/res_counter.h
===================================================================
--- linux-2.6.26-rc8-mm1.orig/include/linux/res_counter.h
+++ linux-2.6.26-rc8-mm1/include/linux/res_counter.h
@@ -176,4 +176,19 @@ static inline bool res_counter_can_add(s
 	return ret;
 }
 
+static inline int res_counter_set_limit(struct res_counter *cnt,
+	unsigned long long limit)
+{
+	unsigned long flags;
+	int ret = -EBUSY;
+
+	spin_lock_irqsave(&cnt->lock, flags);
+	if (cnt->usage < limit) {
+		cnt->limit = limit;
+		ret = 0;
+	}
+	spin_unlock_irqrestore(&cnt->lock, flags);
+	return ret;
+}
+
 #endif

--
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 2/2][-mm][resend] memcg limit change shrink usage.

[KAMEZAWA Hiroyuki]

Shrinking memory usage at limit change.

This is an enhancement (in TODO list).
based on res_counter-limit-change-ebusy.patch

Changelog: v2 -> v3
  - supported interrupt by signal. (A user can stop limit change by Ctrl-C.)
Changelog: v1 -> v2
  - adjusted to be based on write_string() patch set
  - removed backword goto.
  - removed unneccesary cond_resched().

Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Acked-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

 Documentation/controllers/memory.txt |    3 --
 mm/memcontrol.c                      |   48 ++++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 6 deletions(-)

Index: linux-2.6.26-rc8-mm1/mm/memcontrol.c
===================================================================
--- linux-2.6.26-rc8-mm1.orig/mm/memcontrol.c
+++ linux-2.6.26-rc8-mm1/mm/memcontrol.c
@@ -836,6 +836,30 @@ int mem_cgroup_shrink_usage(struct mm_st
 	return 0;
 }
 
+int mem_cgroup_resize_limit(struct mem_cgroup *memcg, unsigned long long val)
+{
+
+	int retry_count = MEM_CGROUP_RECLAIM_RETRIES;
+	int progress;
+	int ret = 0;
+
+	while (res_counter_set_limit(&memcg->res, val)) {
+		if (signal_pending(current)) {
+			ret = -EINTR;
+			break;
+		}
+		if (!retry_count) {
+			ret = -EBUSY;
+			break;
+		}
+		progress = try_to_free_mem_cgroup_pages(memcg, GFP_KERNEL);
+		if (!progress)
+			retry_count--;
+	}
+	return ret;
+}
+
+
 /*
  * This routine traverse page_cgroup in given list and drop them all.
  * *And* this routine doesn't reclaim page itself, just removes page_cgroup.
@@ -916,13 +940,29 @@ static u64 mem_cgroup_read(struct cgroup
 	return res_counter_read_u64(&mem_cgroup_from_cont(cont)->res,
 				    cft->private);
 }
-
+/*
+ * The user of this function is...
+ * RES_LIMIT.
+ */
 static int mem_cgroup_write(struct cgroup *cont, struct cftype *cft,
 			    const char *buffer)
 {
-	return res_counter_write(&mem_cgroup_from_cont(cont)->res,
-				 cft->private, buffer,
-				 res_counter_memparse_write_strategy);
+	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
+	unsigned long long val;
+	int ret;
+
+        switch (cft->private) {
+	case RES_LIMIT:
+		/* This function does all necessary parse...reuse it */
+		ret = res_counter_memparse_write_strategy(buffer, &val);
+		if (!ret)
+			ret = mem_cgroup_resize_limit(memcg, val);
+		break;
+	default:
+		ret = -EINVAL; /* should be BUG() ? */
+		break;
+	}
+	return ret;
 }
 
 static int mem_cgroup_reset(struct cgroup *cont, unsigned int event)
Index: linux-2.6.26-rc8-mm1/Documentation/controllers/memory.txt
===================================================================
--- linux-2.6.26-rc8-mm1.orig/Documentation/controllers/memory.txt
+++ linux-2.6.26-rc8-mm1/Documentation/controllers/memory.txt
@@ -242,8 +242,7 @@ rmdir() if there are no tasks.
 1. Add support for accounting huge pages (as a separate controller)
 2. Make per-cgroup scanner reclaim not-shared pages first
 3. Teach controller to account for shared-pages
-4. Start reclamation when the limit is lowered
-5. Start reclamation in the background when the limit is
+4. Start reclamation in the background when the limit is
    not yet hit but the usage is getting closer
 
 Summary

--
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 2/2][-mm][resend] memcg limit change shrink usage.

[Andrew Morton]

On Mon, 14 Jul 2008 17:15:22 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> Shrinking memory usage at limit change.

The above six words are all we really have as a changelog.  It is not
adequate.

> This is an enhancement (in TODO list).
> based on res_counter-limit-change-ebusy.patch
> 
> Changelog: v2 -> v3
>   - supported interrupt by signal. (A user can stop limit change by Ctrl-C.)
> Changelog: v1 -> v2
>   - adjusted to be based on write_string() patch set
>   - removed backword goto.
>   - removed unneccesary cond_resched().
> 
> Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> Acked-by: Pavel Emelyanov <xemul@openvz.org>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
>  Documentation/controllers/memory.txt |    3 --
>  mm/memcontrol.c                      |   48 ++++++++++++++++++++++++++++++++---
>  2 files changed, 45 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6.26-rc8-mm1/mm/memcontrol.c
> ===================================================================
> --- linux-2.6.26-rc8-mm1.orig/mm/memcontrol.c
> +++ linux-2.6.26-rc8-mm1/mm/memcontrol.c
> @@ -836,6 +836,30 @@ int mem_cgroup_shrink_usage(struct mm_st
>  	return 0;
>  }
>  
> +int mem_cgroup_resize_limit(struct mem_cgroup *memcg, unsigned long long val)
> +{
> +
> +	int retry_count = MEM_CGROUP_RECLAIM_RETRIES;
> +	int progress;
> +	int ret = 0;
> +
> +	while (res_counter_set_limit(&memcg->res, val)) {
> +		if (signal_pending(current)) {
> +			ret = -EINTR;
> +			break;
> +		}
> +		if (!retry_count) {
> +			ret = -EBUSY;
> +			break;
> +		}
> +		progress = try_to_free_mem_cgroup_pages(memcg, GFP_KERNEL);
> +		if (!progress)
> +			retry_count--;
> +	}
> +	return ret;
> +}

We could perhaps get away with a basically-unchanglogged patch if the
code was adequately commented.  But it is not.

What the heck does this function *do*?  Why does it exist?

Guys, this is core Linux kernel, not some weekend hack project.  Please
work to make it as comprehensible and as maintainable as we possibly
can.

Also, it is frequently a mistake for a callee to assume that the caller
can use GFP_KERNEL.  Often when we do this we end having to change the
interface so that the caller passes in the gfp_t.  As there's only one
caller I guess we can get away with it this time.  For now.

> +
>  /*
>   * This routine traverse page_cgroup in given list and drop them all.
>   * *And* this routine doesn't reclaim page itself, just removes page_cgroup.
> @@ -916,13 +940,29 @@ static u64 mem_cgroup_read(struct cgroup
>  	return res_counter_read_u64(&mem_cgroup_from_cont(cont)->res,
>  				    cft->private);
>  }
> -
> +/*
> + * The user of this function is...
> + * RES_LIMIT.
> + */
>  static int mem_cgroup_write(struct cgroup *cont, struct cftype *cft,
>  			    const char *buffer)
>  {
> -	return res_counter_write(&mem_cgroup_from_cont(cont)->res,
> -				 cft->private, buffer,
> -				 res_counter_memparse_write_strategy);
> +	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
> +	unsigned long long val;
> +	int ret;
> +
> +        switch (cft->private) {
> +	case RES_LIMIT:
> +		/* This function does all necessary parse...reuse it */
> +		ret = res_counter_memparse_write_strategy(buffer, &val);
> +		if (!ret)
> +			ret = mem_cgroup_resize_limit(memcg, val);
> +		break;
> +	default:
> +		ret = -EINVAL; /* should be BUG() ? */
> +		break;
> +	}
> +	return ret;
>  }

--
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: Re: [PATCH 2/2][-mm][resend] memcg limit change shrink usage.

[kamezawa.hiroyu]

----- Original Message -----

>On Mon, 14 Jul 2008 17:15:22 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fuji
tsu.com> wrote:
>
>> Shrinking memory usage at limit change.
>
>The above six words are all we really have as a changelog.  It is not
>adequate.
>
I'll add enough description (in this week), sorry,


>> +	while (res_counter_set_limit(&memcg->res, val)) {
>> +		if (signal_pending(current)) {
>> +			ret = -EINTR;
>> +			break;
>> +		}
>> +		if (!retry_count) {
>> +			ret = -EBUSY;
>> +			break;
>> +		}
>> +		progress = try_to_free_mem_cgroup_pages(memcg, GFP_KERNEL);
>> +		if (!progress)
>> +			retry_count--;
>> +	}
>> +	return ret;
>> +}
>
>We could perhaps get away with a basically-unchanglogged patch if the
>code was adequately commented.  But it is not.
>
>What the heck does this function *do*?  Why does it exist?
>
Sorry. I should do so.

>Guys, this is core Linux kernel, not some weekend hack project.  Please
>work to make it as comprehensible and as maintainable as we possibly
>can.
>
>Also, it is frequently a mistake for a callee to assume that the caller
>can use GFP_KERNEL.  Often when we do this we end having to change the
>interface so that the caller passes in the gfp_t.  As there's only one
>caller I guess we can get away with it this time.  For now.
>

Hmm, ok. will rework this and take gfp_t as an argument.

Thanks,
-Kame

--
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 2/2][-mm][resend] memcg limit change shrink usage.

[Andrew Morton]

On Mon, 28 Jul 2008 17:43:22 +0900 (JST) kamezawa.hiroyu@jp.fujitsu.com wrote:

> >Guys, this is core Linux kernel, not some weekend hack project.  Please
> >work to make it as comprehensible and as maintainable as we possibly
> >can.
> >
> >Also, it is frequently a mistake for a callee to assume that the caller
> >can use GFP_KERNEL.  Often when we do this we end having to change the
> >interface so that the caller passes in the gfp_t.  As there's only one
> >caller I guess we can get away with it this time.  For now.
> >
> 
> Hmm, ok. will rework this and take gfp_t as an argument.

I don't think it's necessary, really.  I was just talking to myself ;)

--
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 1/2][-mm][resend] res_counter limit change support ebusy

[Andrew Morton]

On Mon, 14 Jul 2008 17:11:54 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> +static inline int res_counter_set_limit(struct res_counter *cnt,
> +	unsigned long long limit)
> +{
> +	unsigned long flags;
> +	int ret = -EBUSY;
> +
> +	spin_lock_irqsave(&cnt->lock, flags);
> +	if (cnt->usage < limit) {
> +		cnt->limit = limit;
> +		ret = 0;
> +	}
> +	spin_unlock_irqrestore(&cnt->lock, flags);
> +	return ret;
> +}

Need I say it?  This function is waaaaaay too large to be inlined.

--
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: Re: [PATCH 1/2][-mm][resend] res_counter limit change support ebusy

[kamezawa.hiroyu]

----- Original Message -----
>On Mon, 14 Jul 2008 17:11:54 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fuji
tsu.com> wrote:
>
>> +static inline int res_counter_set_limit(struct res_counter *cnt,
>> +	unsigned long long limit)
>> +{
>> +	unsigned long flags;
>> +	int ret = -EBUSY;
>> +
>> +	spin_lock_irqsave(&cnt->lock, flags);
>> +	if (cnt->usage < limit) {
>> +		cnt->limit = limit;
>> +		ret = 0;
>> +	}
>> +	spin_unlock_irqrestore(&cnt->lock, flags);
>> +	return ret;
>> +}
>
>Need I say it?  This function is waaaaaay too large to be inlined.

Will rework and make this uninlined. (in this week)

Thanks,
-Kame

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