On Thu, 2020-10-22 at 17:50 +0200, Michal Hocko wrote: > On Thu 22-10-20 09:25:21, Rik van Riel wrote: > > On Thu, 2020-10-22 at 10:15 +0200, Michal Hocko wrote: > > > On Wed 21-10-20 23:48:46, Rik van Riel wrote: > > > > > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > > > index 537c137698f8..d1290eb508e5 100644 > > > > --- a/mm/shmem.c > > > > +++ b/mm/shmem.c > > > > @@ -1545,8 +1545,11 @@ static struct page > > > > *shmem_alloc_hugepage(gfp_t gfp, > > > > return NULL; > > > > > > > > shmem_pseudo_vma_init(&pvma, info, hindex); > > > > - page = alloc_pages_vma(gfp | __GFP_COMP | __GFP_NORETRY > > > > | > > > > __GFP_NOWARN, > > > > - HPAGE_PMD_ORDER, &pvma, 0, > > > > numa_node_id(), > > > > true); > > > > + /* Limit the gfp mask according to THP configuration. > > > > */ > > > > + gfp |= __GFP_COMP | __GFP_NORETRY | __GFP_NOWARN; > > > > > > What is the reason for these when alloc_hugepage_direct_gfpmask > > > provides > > > the full mask? > > > > The mapping_gfp_mask for the shmem file might have additional > > restrictions above and beyond the gfp mask returned by > > alloc_hugepage_direct_gfpmask, and I am not sure we should just > > ignore the mapping_gfp_mask. > > No, we shouldn't. But I do not see why you should be adding the above > set of flags on top.
Because THP allocations are higher order and optimistic, and we want them to: 1) be annotated as compound allocations 2) fail (and fall back to 4kB allocations) when they cannot be easily satisfied, and 3) not create a spew of allocation failure backtraces on the (serial) console when these THP allocations fail > > That is why this patch takes the union of both gfp masks. > > > > However, digging into things more, it looks like shmem inodes > > always have the mapping gfp mask set to GFP_HIGHUSER_MOVABLE, > > and it is never changed, so simply using the output from > > alloc_hugepage_direct_gfpmask should be fine. > > > > I can do the patch either way. Just let me know what you prefer. > > I would just and the given gfp with alloc_hugepage_direct_gfpmask That would miss the three things we definitely want from above. -- All Rights Reversed.
signature.asc
Description: This is a digitally signed message part