From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sha Zhengju <handai.szj@gmail.com>
Subject: Re: [PATCH] memcg: make threshold index in the right position
Date: Thu, 02 Feb 2012 19:08:44 +0800
Message-ID: <4F2A6EBC.5090207@gmail.com>
References: <1328175919-11209-1-git-send-email-handai.szj@taobao.com> <20120202101410.GA12291@shutemov.name> <4F2A6B5F.3090303@gmail.com>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Return-path: <owner-linux-mm@kvack.org>
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=gamma;
        h=message-id:date:from:user-agent:mime-version:to:cc:subject
         :references:in-reply-to:content-type:content-transfer-encoding;
        bh=ALuwZ/vmPur3+wSKFxFbxG9rrc8AgOzKCnOKPjBNgMw=;
        b=IIYKIGicnhgnZ6Spl8A/3qXbkEKVNS9TltHjc23EtLLXZoI7VSyQrvOV5L7zyNYXzn
         Rne5GyOJuD4F4njcUz69rFJT+wpUg9qjMnfsnaVK6Fl8hXFEBUMcLNno8hngLZC5rU4v
         YDrV+ovMZDRwiBBoZGZRZ3SRq+z4tCipQSx8E=
In-Reply-To: <4F2A6B5F.3090303@gmail.com>
Sender: owner-linux-mm@kvack.org
List-ID: <cgroups.vger.kernel.org>
Content-Type: text/plain; charset="us-ascii"; format="flowed"
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: linux-mm@kvack.org, cgroups@vger.kernel.org, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

On 02/02/2012 06:54 PM, Sha Zhengju wrote:
> On 02/02/2012 06:14 PM, Kirill A. Shutemov wrote:
>> On Thu, Feb 02, 2012 at 05:45:19PM +0800, Sha Zhengju wrote:
>>> From: Sha Zhengju<handai.szj@taobao.com>
>>>
>>> Index current_threshold may point to threshold that just equal to
>>> usage after __mem_cgroup_threshold is triggerd.
>> I don't see it. Could you describe conditions?
>>
> It is because of the following code path in __mem_cgroup_threshold:
> {
>     ...
>         i = t->current_threshold;
>
>         for (; i >= 0 && unlikely(t->entries[i].threshold > usage); i--)
>                 eventfd_signal(t->entries[i].eventfd, 1);
>         i++;
>
>         for (; i < t->size && unlikely(t->entries[i].threshold <= 
> usage); i++)
>                 eventfd_signal(t->entries[i].eventfd, 1);
>
>         t->current_threshold = i - 1;
>     ...
> }
>
> For example:
> now:
>     threshold array:  3  5  7  9   (usage = 6)
>                                    ^
>                                 index
>
> next turn:
>     threshold array:  3  5  7  9   (usage = 7)
>                                        ^
>                                     index
>
> after registering a new event(threshold = 10):
>     threshold array:  3  5  7  9  10 (usage = 7)
>                                    ^
>                                 index
Err.. Sorry for showing inaccurate index position... (may because of the 
mail format)

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

next turn:
     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)

>>> But after registering
>>> a new event, it will change (pointing to threshold just below usage).
>>> So make it consistent here.
>>>
>>> Cc: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>>> Cc: Kirill A. Shutemov<kirill@shutemov.name>
>>> Signed-off-by: Sha Zhengju<handai.szj@taobao.com>
>>> ---
>>>   mm/memcontrol.c |    7 ++++---
>>>   1 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 22d94f5..79f4a58 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;
>>> @@ -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 */
>>> -- 
>>> 1.7.4.1
>>>
>

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