Re: [PATCH] mem-hotplug: Don't clear the only node in new_node_page()

[Michal Hocko <mhocko@suse.cz>]

On Mon 05-09-16 16:18:29, Vlastimil Babka wrote:
> On 09/05/2016 04:59 AM, Li Zhong wrote:
> > Commit 394e31d2c introduced new_node_page() for memory hotplug.
> > 
> > In new_node_page(), the nid is cleared before calling __alloc_pages_nodemask().
> > But if it is the only node of the system,
> 
> So the use case is that we are partially offlining the only online node?
> 
> > and the first round allocation fails,
> > it will not be able to get memory from an empty nodemask, and trigger oom.
> 
> Hmm triggering OOM due to empty nodemask sounds like a wrong thing to do.
> CCing some OOM experts for insight.

Hmm, to be honest I think that using an empty nodemask is just a bug in
the code. I do not see any reasonable scenario when this would make a
sense. I agree that triggering an OOM killer for that is bad as well but
do we actually want to allow for such a case at all? How can this
happen?

> Also OOM is skipped for __GFP_THISNODE
> allocations, so we might also consider the same for nodemask-constrained
> allocations?
> 
> > The patch checks whether it is the last node on the system, and if it is, then
> > don't clear the nid in the nodemask.
> 
> I'd rather see the allocation not OOM, and rely on the fallback in
> new_node_page() that doesn't have nodemask. But I suspect it might also make
> sense to treat empty nodemask as something unexpected and put some WARN_ON
> (instead of OOM) in the allocator.

To be honest I am really not all that happy about 394e31d2ceb4
("mem-hotplug: alloc new page from a nearest neighbor node when
mem-offline") and find it a bit fishy. I would rather re-iterate that
patch rather than build new hacks on top.
-- 
Michal Hocko
SUSE Labs

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