On 2018/08/20 19:41, Michal Hocko wrote:
> On Sat 18-08-18 22:02:14, Tetsuo Handa wrote:
>> On 2018/08/16 19:03, Michal Hocko wrote:
>>> The code is quite subtle and we have a bad history of copying stuff
>>> without rethinking whether the code still is needed. Which is sad and a
>>> clear sign that the code is too complex. I cannot say this change
>>> doesn't have any subtle side effects but it makes the intention clear at
>>> least so I _think_ it is good to go. If we find some unintended side
>>> effects we should simply rethink the whole reset zonelist thing.
>>
>> Does this change affect
>>
>> /*
>> * This is not a __GFP_THISNODE allocation, so a truncated nodemask in
>> * the page allocator means a mempolicy is in effect. Cpuset policy
>> * is enforced in get_page_from_freelist().
>> */
>> if (oc->nodemask &&
>> !nodes_subset(node_states[N_MEMORY], *oc->nodemask)) {
>> oc->totalpages = total_swap_pages;
>> for_each_node_mask(nid, *oc->nodemask)
>> oc->totalpages += node_spanned_pages(nid);
>> return CONSTRAINT_MEMORY_POLICY;
>> }
>>
>> in constrained_alloc() called from
>>
>> /*
>> * Check if there were limitations on the allocation (only relevant for
>> * NUMA and memcg) that may require different handling.
>> */
>> constraint = constrained_alloc(oc);
>> if (constraint != CONSTRAINT_MEMORY_POLICY)
>> oc->nodemask = NULL;
>>
>> in out_of_memory() ?
>
> No practical difference AFAICS. We are losing the nodemask for oom
> victims but their mere existance should make oom decisions void
> and so the constrain shouldn't really matter.
>
If my guess that whether oc->nodemask is NULL affects whether oom_unkillable_task()
(effectively has_intersects_mems_allowed()) returns true is correct, I worried that
OOM victims select next OOM victim from wider targets if MMF_OOM_SKIP was already set
on the OOM victim's mm. That might be an unexpected behavior...
