hi, Shakeel,
On 03/12/2017 01:52 AM, Shakeel Butt wrote:
> On Sat, Mar 11, 2017 at 5:51 AM, Yisheng Xie <ys...@foxmail.com> wrote:
>> From: Yisheng Xie <xieyishe...@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 <xieyishe...@huawei.com>
>> Suggested-by: Shakeel Butt <shake...@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
@
