All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yisheng Xie <ysxie@foxmail.com>
To: Shakeel Butt <shakeelb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Mel Gorman <mgorman@suse.de>, Vlastimil Babka <vbabka@suse.cz>,
	Michal Hocko <mhocko@suse.com>,
	riel@redhat.com, Linux MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	xieyisheng1@huawei.com, guohanjun@huawei.com,
	Xishi Qiu <qiuxishi@huawei.com>
Subject: Re: [PATCH v2 RFC] mm/vmscan: more restrictive condition for retry in do_try_to_free_pages
Date: Sun, 12 Mar 2017 18:15:05 +0800	[thread overview]
Message-ID: <58C51FA9.4000705@foxmail.com> (raw)
In-Reply-To: <CALvZod6dptidW33mpvSkQfMBM=xsfSPEEJzB+3u4ekr8m3bSOA@mail.gmail.com>

hi, Shakeel,

On 03/12/2017 01:52 AM, Shakeel Butt wrote:
> On Sat, Mar 11, 2017 at 5:51 AM, Yisheng Xie <ysxie@foxmail.com> wrote:
>> From: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> When we enter do_try_to_free_pages, the may_thrash is always clear, and
>> it will retry shrink zones to tap cgroup's reserves memory by setting
>> may_thrash when the former shrink_zones reclaim nothing.
>>
>> However, when memcg is disabled or on legacy hierarchy, it should not do
>> this useless retry at all, for we do not have any cgroup's reserves
>> memory to tap, and we have already done hard work but made no progress.
>>
>> To avoid this time costly and useless retrying, add a stub function
>> may_thrash and return true when memcg is disabled or on legacy
>> hierarchy.
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> Suggested-by: Shakeel Butt <shakeelb@google.com>
>> ---
>>
>>                 return 1;
>>
>>         /* Untapped cgroup reserves?  Don't OOM, retry. */
>> -       if (!sc->may_thrash) {
>> +       if (!may_thrash(sc)) {
> Thanks Yisheng. The name of the function may_thrash() is confusing in
> the sense that it is returning exactly the opposite of what its name
> implies. 
Right.

> How about reversing the condition of may_thrash() function
> and change the scan_control's field may_thrash to thrashed?
hmm, maybe I can change the may_thrash() function to mem_cgroup_thrashed().
For, if change the scan_control's may_thrash to thrashed, it may also looks
confusing in shrink_node, and it will be like:
                         if (mem_cgroup_low(root, memcg)) {
                                 if (!sc->thrashed) -----> looks confuse here?
                                         continue;
                                 mem_cgroup_events(memcg, MEMCG_LOW, 1);
                        }

Thanks
Yisheng Xie
	@

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

WARNING: multiple messages have this Message-ID (diff)
From: Yisheng Xie <ysxie@foxmail.com>
To: Shakeel Butt <shakeelb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Mel Gorman <mgorman@suse.de>, Vlastimil Babka <vbabka@suse.cz>,
	Michal Hocko <mhocko@suse.com>,
	riel@redhat.com, Linux MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	xieyisheng1@huawei.com, guohanjun@huawei.com,
	Xishi Qiu <qiuxishi@huawei.com>
Subject: Re: [PATCH v2 RFC] mm/vmscan: more restrictive condition for retry in do_try_to_free_pages
Date: Sun, 12 Mar 2017 18:15:05 +0800	[thread overview]
Message-ID: <58C51FA9.4000705@foxmail.com> (raw)
In-Reply-To: <CALvZod6dptidW33mpvSkQfMBM=xsfSPEEJzB+3u4ekr8m3bSOA@mail.gmail.com>

hi, Shakeel,

On 03/12/2017 01:52 AM, Shakeel Butt wrote:
> On Sat, Mar 11, 2017 at 5:51 AM, Yisheng Xie <ysxie@foxmail.com> wrote:
>> From: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> When we enter do_try_to_free_pages, the may_thrash is always clear, and
>> it will retry shrink zones to tap cgroup's reserves memory by setting
>> may_thrash when the former shrink_zones reclaim nothing.
>>
>> However, when memcg is disabled or on legacy hierarchy, it should not do
>> this useless retry at all, for we do not have any cgroup's reserves
>> memory to tap, and we have already done hard work but made no progress.
>>
>> To avoid this time costly and useless retrying, add a stub function
>> may_thrash and return true when memcg is disabled or on legacy
>> hierarchy.
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> Suggested-by: Shakeel Butt <shakeelb@google.com>
>> ---
>>
>>                 return 1;
>>
>>         /* Untapped cgroup reserves?  Don't OOM, retry. */
>> -       if (!sc->may_thrash) {
>> +       if (!may_thrash(sc)) {
> Thanks Yisheng. The name of the function may_thrash() is confusing in
> the sense that it is returning exactly the opposite of what its name
> implies. 
Right.

> How about reversing the condition of may_thrash() function
> and change the scan_control's field may_thrash to thrashed?
hmm, maybe I can change the may_thrash() function to mem_cgroup_thrashed().
For, if change the scan_control's may_thrash to thrashed, it may also looks
confusing in shrink_node, and it will be like:
                         if (mem_cgroup_low(root, memcg)) {
                                 if (!sc->thrashed) -----> looks confuse here?
                                         continue;
                                 mem_cgroup_events(memcg, MEMCG_LOW, 1);
                        }

Thanks
Yisheng Xie
	@

  reply	other threads:[~2017-03-12 10:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-11 13:51 [PATCH v2 RFC] mm/vmscan: more restrictive condition for retry in do_try_to_free_pages Yisheng Xie
2017-03-11 13:51 ` Yisheng Xie
2017-03-11 17:52 ` Shakeel Butt
2017-03-11 17:52   ` Shakeel Butt
2017-03-12 10:15   ` Yisheng Xie [this message]
2017-03-12 10:15     ` Yisheng Xie
2017-03-16 19:57   ` Johannes Weiner
2017-03-16 19:57     ` Johannes Weiner
2017-03-16 20:26     ` Shakeel Butt
2017-03-16 20:26       ` Shakeel Butt
  -- strict thread matches above, loose matches on Subject: below --
2017-03-11 13:36 Yisheng Xie
2017-03-11 13:36 ` Yisheng Xie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=58C51FA9.4000705@foxmail.com \
    --to=ysxie@foxmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=guohanjun@huawei.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.com \
    --cc=qiuxishi@huawei.com \
    --cc=riel@redhat.com \
    --cc=shakeelb@google.com \
    --cc=vbabka@suse.cz \
    --cc=xieyisheng1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.