lkml.org 
[lkml]   [2018]   [Dec]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [patch v2 for-4.20] Revert "mm, thp: consolidate THP gfp handling into alloc_hugepage_direct_gfpmask"
From
Date
On 12/7/18 11:50 PM, David Rientjes wrote:
> This reverts commit 89c83fb539f95491be80cdd5158e6f0ce329e317.
>
> This should have been done as part of 2f0799a0ffc0 ("mm, thp: restore
> node-local hugepage allocations"). The movement of the thp allocation
> policy from alloc_pages_vma() to alloc_hugepage_direct_gfpmask() was
> intended to only set __GFP_THISNODE for mempolicies that are not
> MPOL_BIND whereas the revert could set this regardless of mempolicy.
>
> While the check for MPOL_BIND between alloc_hugepage_direct_gfpmask()
> and alloc_pages_vma() was racy, that has since been removed since the

I would have expected mmap_sem to prevent the race, as faults have it
locked for read and updating mempolicies for write, IIRC? But didn't
check in detail.

> revert. What is left is the possibility to use __GFP_THISNODE in
> policy_node() when it is unexpected because the special handling for
> hugepages in alloc_pages_vma() was removed as part of the consolidation.

Yeah that was a bug.

> Secondly, prior to 89c83fb539f9, alloc_pages_vma() implemented a somewhat
> different policy for hugepage allocations, which were allocated through
> alloc_hugepage_vma(). For hugepage allocations, if the allocating
> process's node is in the set of allowed nodes, allocate with
> __GFP_THISNODE for that node (for MPOL_PREFERRED, use that node with
> __GFP_THISNODE instead). This was changed for shmem_alloc_hugepage() to
> allow fallback to other nodes in 89c83fb539f9 as it did for new_page() in
> mm/mempolicy.c which is functionally different behavior and removes the
> requirement to only allocate hugepages locally.

TBH this slight difference was known and stated in the changelog of
89c83fb539f9 so you could have objected.

> So this commit does a full revert of 89c83fb539f9 instead of the partial
> revert that was done in 2f0799a0ffc0. The result is the same thp
> allocation policy for 4.20 that was in 4.19.
>
> Fixes: 89c83fb539f9 ("mm, thp: consolidate THP gfp handling into alloc_hugepage_direct_gfpmask")
> Fixes: 2f0799a0ffc0 ("mm, thp: restore node-local hugepage allocations")
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
> This indeed restores the thp allocation policy fully to what it was in
> 4.19 since there is obivously more discussion to be had about how the
> NUMA aspects of thp allocations should be addressed. We can do this
> with a stable 4.20 tree in the background that has the same allocation
> policy that was in 4.0.

I agree that this is probably the safest option for now so that the next
rc doesn't contain the warning introduced in 2f0799a0ffc0.

\
 
 \ /
  Last update: 2018-12-08 11:03    [W:0.026 / U:20.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site