Re: [PATCH] mm, page_alloc: do not break __GFP_THISNODE by zonelist reset

[Mel Gorman]

On Fri, May 25, 2018 at 03:08:53PM +0200, Vlastimil Babka wrote:
> In __alloc_pages_slowpath() we reset zonelist and preferred_zoneref for
> allocations that can ignore memory policies. The zonelist is obtained from
> current CPU's node. This is a problem for __GFP_THISNODE allocations that want
> to allocate on a different node, e.g. because the allocating thread has been
> migrated to a different CPU.
> 
> This has been observed to break SLAB in our 4.4-based kernel, because there it
> relies on __GFP_THISNODE working as intended. If a slab page is put on wrong
> node's list, then further list manipulations may corrupt the list because
> page_to_nid() is used to determine which node's list_lock should be locked and
> thus we may take a wrong lock and race.
> 
> Current SLAB implementation seems to be immune by luck thanks to commit
> 511e3a058812 ("mm/slab: make cache_grow() handle the page allocated on
> arbitrary node") but there may be others assuming that __GFP_THISNODE works as
> promised.
> 
> We can fix it by simply removing the zonelist reset completely. There is
> actually no reason to reset it, because memory policies and cpusets don't
> affect the zonelist choice in the first place. This was different when commit
> 183f6371aac2 ("mm: ignore mempolicies when using ALLOC_NO_WATERMARK")
> introduced the code, as mempolicies provided their own restricted zonelists.
> 
> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
> Fixes: 183f6371aac2 ("mm: ignore mempolicies when using ALLOC_NO_WATERMARK")

Acked-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>

Thanks.

-- 
Mel Gorman
SUSE Labs