[PATCH] memcg: revise the position of threshold index while unregistering event

[PATCH] memcg: revise the position of threshold index while unregistering event

[Sha Zhengju]

From: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>

Index current_threshold should point to threshold just below or equal to usage.
See below:
http://www.spinics.net/lists/cgroups/msg00844.html


Signed-off-by: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>

---
 mm/memcontrol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 22d94f5..cd40d67 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4398,7 +4398,7 @@ static void mem_cgroup_usage_unregister_event(struct cgroup *cgrp,
 			continue;
 
 		new->entries[j] = thresholds->primary->entries[i];
-		if (new->entries[j].threshold < usage) {
+		if (new->entries[j].threshold <= usage) {
 			/*
 			 * new->current_threshold will not be used
 			 * until rcu_assign_pointer(), so it's safe to increment
-- 
1.7.4.1

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[Kirill A. Shutemov]

On Tue, Mar 06, 2012 at 08:12:23PM +0800, Sha Zhengju wrote:
> From: Sha Zhengju <handai.szj@taobao.com>
> 
> Index current_threshold should point to threshold just below or equal to usage.
> See below:
> http://www.spinics.net/lists/cgroups/msg00844.html
> 
> 
> Signed-off-by: Sha Zhengju <handai.szj@taobao.com>

Reviewved-by: Kirill A. Shutemov <kirill@shutemov.name>

> 
> ---
>  mm/memcontrol.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 22d94f5..cd40d67 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4398,7 +4398,7 @@ static void mem_cgroup_usage_unregister_event(struct cgroup *cgrp,
>  			continue;
>  
>  		new->entries[j] = thresholds->primary->entries[i];
> -		if (new->entries[j].threshold < usage) {
> +		if (new->entries[j].threshold <= usage) {
>  			/*
>  			 * new->current_threshold will not be used
>  			 * until rcu_assign_pointer(), so it's safe to increment
> -- 
> 1.7.4.1
> 

-- 
 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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[KAMEZAWA Hiroyuki]

On Tue,  6 Mar 2012 20:12:23 +0800
Sha Zhengju <handai.szj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> From: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
> 
> Index current_threshold should point to threshold just below or equal to usage.
> See below:
> http://www.spinics.net/lists/cgroups/msg00844.html
> 
> 
> Signed-off-by: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>

Thank you for resending.

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[Sha Zhengju]

On 03/08/2012 01:44 PM, KAMEZAWA Hiroyuki wrote:
> On Tue,  6 Mar 2012 20:12:23 +0800
> Sha Zhengju<handai.szj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>  wrote:
>
>> From: Sha Zhengju<handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
>>
>> Index current_threshold should point to threshold just below or equal to usage.
>> See below:
>> http://www.spinics.net/lists/cgroups/msg00844.html
>>
>>
>> Signed-off-by: Sha Zhengju<handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
> Thank you for resending.
>
> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
>
It's not a resending, though they are for the same reason.  May be I should
merge them together ...


Thanks,
Sha

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[KAMEZAWA Hiroyuki]

On Thu, 08 Mar 2012 15:02:50 +0800
Sha Zhengju <handai.szj@gmail.com> wrote:

> On 03/08/2012 01:44 PM, KAMEZAWA Hiroyuki wrote:
> > On Tue,  6 Mar 2012 20:12:23 +0800
> > Sha Zhengju<handai.szj@gmail.com>  wrote:
> >
> >> From: Sha Zhengju<handai.szj@taobao.com>
> >>
> >> Index current_threshold should point to threshold just below or equal to usage.
> >> See below:
> >> http://www.spinics.net/lists/cgroups/msg00844.html
> >>
> >>
> >> Signed-off-by: Sha Zhengju<handai.szj@taobao.com>
> > Thank you for resending.
> >
> > Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> >
> It's not a resending, though they are for the same reason.  May be I should
> merge them together ...
> 
Ah. Hmm..If your previous patch isn't picked up yet, could you send it again 
(or merge and post merged one ) ?

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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[Sha Zhengju]

On 03/08/2012 03:30 PM, KAMEZAWA Hiroyuki wrote:
> On Thu, 08 Mar 2012 15:02:50 +0800
> Sha Zhengju<handai.szj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>  wrote:
>
>> On 03/08/2012 01:44 PM, KAMEZAWA Hiroyuki wrote:
>>> On Tue,  6 Mar 2012 20:12:23 +0800
>>> Sha Zhengju<handai.szj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>   wrote:
>>>
>>>> From: Sha Zhengju<handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> Index current_threshold should point to threshold just below or equal to usage.
>>>> See below:
>>>> http://www.spinics.net/lists/cgroups/msg00844.html
>>>>
>>>>
>>>> Signed-off-by: Sha Zhengju<handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
>>> Thank you for resending.
>>>
>>> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
>>>
>> It's not a resending, though they are for the same reason.  May be I should
>> merge them together ...
>>
> Ah. Hmm..If your previous patch isn't picked up yet, could you send it again
> (or merge and post merged one ) ?
>
> Thanks,
> -Kame
>
Ok, I'll send a new version one later. :-)

Thanks,
Sha

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[Andrew Morton]

On Tue,  6 Mar 2012 20:12:23 +0800
Sha Zhengju <handai.szj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> From: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
> 
> Index current_threshold should point to threshold just below or equal to usage.
> See below:
> http://www.spinics.net/lists/cgroups/msg00844.html

I have a bad feeling that I looked at a version of this patch
yesterday, but I can't find it.

> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4398,7 +4398,7 @@ static void mem_cgroup_usage_unregister_event(struct cgroup *cgrp,
>  			continue;
>  
>  		new->entries[j] = thresholds->primary->entries[i];
> -		if (new->entries[j].threshold < usage) {
> +		if (new->entries[j].threshold <= usage) {
>  			/*
>  			 * new->current_threshold will not be used
>  			 * until rcu_assign_pointer(), so it's safe to increment

What were the runtime effects of this bug?

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[Andrew Morton]

On Thu, 5 Apr 2012 16:35:30 -0700
Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:

> On Tue,  6 Mar 2012 20:12:23 +0800
> Sha Zhengju <handai.szj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> > From: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
> > 
> > Index current_threshold should point to threshold just below or equal to usage.
> > See below:
> > http://www.spinics.net/lists/cgroups/msg00844.html
> 
> I have a bad feeling that I looked at a version of this patch
> yesterday, but I can't find it.

Found it!  Below.

I think we might as well fold "memcg: revise the position of threshold
index while unregistering event" into the below "memcg: make threshold
index in the right position" as a single patch?



From: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
Subject: memcg: make threshold index in the right position

Index current_threshold may point to threshold that just equal to usage
after last call of __mem_cgroup_threshold.  But after registering a new
event, it will change (pointing to threshold just below usage).  So make
it consistent here.

For example:
now:
	threshold array:  3  [5]  7  9   (usage = 6, [index] = 5)

next turn (after calling __mem_cgroup_threshold):
	threshold array:  3   5  [7]  9   (usage = 7, [index] = 7)

after registering a new event (threshold = 10):
	threshold array:  3  [5]  7  9  10 (usage = 7, [index] = 5)

Signed-off-by: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Reviewed-by: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
Cc: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
Signed-off-by: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
---

 mm/memcontrol.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff -puN mm/memcontrol.c~memcg-make-threshold-index-in-the-right-position mm/memcontrol.c
--- a/mm/memcontrol.c~memcg-make-threshold-index-in-the-right-position
+++ a/mm/memcontrol.c
@@ -181,7 +181,7 @@ struct mem_cgroup_threshold {
 
 /* For threshold */
 struct mem_cgroup_threshold_ary {
-	/* An array index points to threshold just below usage. */
+	/* An array index points to threshold just below or equal to usage. */
 	int current_threshold;
 	/* Size of entries[] */
 	unsigned int size;
@@ -4267,7 +4267,7 @@ static void __mem_cgroup_threshold(struc
 	usage = mem_cgroup_usage(memcg, swap);
 
 	/*
-	 * current_threshold points to threshold just below usage.
+	 * current_threshold points to threshold just below or equal to usage.
 	 * If it's not true, a threshold was crossed after last
 	 * call of __mem_cgroup_threshold().
 	 */
@@ -4393,14 +4393,15 @@ static int mem_cgroup_usage_register_eve
 	/* Find current threshold */
 	new->current_threshold = -1;
 	for (i = 0; i < size; i++) {
-		if (new->entries[i].threshold < usage) {
+		if (new->entries[i].threshold <= usage) {
 			/*
 			 * new->current_threshold will not be used until
 			 * rcu_assign_pointer(), so it's safe to increment
 			 * it here.
 			 */
 			++new->current_threshold;
-		}
+		} else
+			break;
 	}
 
 	/* Free old spare buffer and save old primary buffer as spare */
_

Re: [PATCH] memcg: revise the position of threshold index while unregistering event

[Sha Zhengju]

On 04/06/2012 07:37 AM, Andrew Morton wrote:
> On Thu, 5 Apr 2012 16:35:30 -0700
> Andrew Morton<akpm@linux-foundation.org>  wrote:
>
>> On Tue,  6 Mar 2012 20:12:23 +0800
>> Sha Zhengju<handai.szj@gmail.com>  wrote:
>>
>>> From: Sha Zhengju<handai.szj@taobao.com>
>>>
>>> Index current_threshold should point to threshold just below or equal to usage.
>>> See below:
>>> http://www.spinics.net/lists/cgroups/msg00844.html
>> I have a bad feeling that I looked at a version of this patch
>> yesterday, but I can't find it.
> Found it!  Below.
>
> I think we might as well fold "memcg: revise the position of threshold
> index while unregistering event" into the below "memcg: make threshold
> index in the right position" as a single patch?
>

Yeah, actually I've already sent a folded one before(maybe I should
mark it as V2):
http://www.spinics.net/lists/cgroups/msg01133.html

Thanks,
Sha


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[RESEND, PATCH] memcg: make threshold index in the right position

[Sha Zhengju]

From: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>

Index current_threshold may point to threshold that just equal to
usage after last call of __mem_cgroup_threshold. But after registering
or unregistering a new event, it will change (pointing to threshold 
just below usage). So make it consistent here.

For example:
now:
        threshold array:  3  [5]  7  9   (usage = 6, [index] = 5)

next turn (after calling __mem_cgroup_threshold):
        threshold array:  3   5  [7]  9   (usage = 7, [index] = 7)

after registering a new event (threshold = 10):
        threshold array:  3  [5]  7  9  10 (usage = 7, [index] = 5)

Signed-off-by: Sha Zhengju <handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Reviewed-by: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>

---
 mm/memcontrol.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 22d94f5..34737b7 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -183,7 +183,7 @@ struct mem_cgroup_threshold {
 
 /* For threshold */
 struct mem_cgroup_threshold_ary {
-	/* An array index points to threshold just below usage. */
+	/* An array index points to threshold just below or equal to usage. */
 	int current_threshold;
 	/* Size of entries[] */
 	unsigned int size;
@@ -4193,7 +4193,7 @@ static void __mem_cgroup_threshold(struct mem_cgroup *memcg, bool swap)
 	usage = mem_cgroup_usage(memcg, swap);
 
 	/*
-	 * current_threshold points to threshold just below usage.
+	 * current_threshold points to threshold just below or equal to usage.
 	 * If it's not true, a threshold was crossed after last
 	 * call of __mem_cgroup_threshold().
 	 */
@@ -4319,14 +4319,15 @@ static int mem_cgroup_usage_register_event(struct cgroup *cgrp,
 	/* Find current threshold */
 	new->current_threshold = -1;
 	for (i = 0; i < size; i++) {
-		if (new->entries[i].threshold < usage) {
+		if (new->entries[i].threshold <= usage) {
 			/*
 			 * new->current_threshold will not be used until
 			 * rcu_assign_pointer(), so it's safe to increment
 			 * it here.
 			 */
 			++new->current_threshold;
-		}
+		} else
+			break;
 	}
 
 	/* Free old spare buffer and save old primary buffer as spare */
@@ -4398,7 +4399,7 @@ static void mem_cgroup_usage_unregister_event(struct cgroup *cgrp,
 			continue;
 
 		new->entries[j] = thresholds->primary->entries[i];
-		if (new->entries[j].threshold < usage) {
+		if (new->entries[j].threshold <= usage) {
 			/*
 			 * new->current_threshold will not be used
 			 * until rcu_assign_pointer(), so it's safe to increment
-- 
1.7.4.1