Re: [PATCH 2/4] mm/gup: restrict CMA region by using allocation scope API

[Michal Hocko]

On Fri 17-07-20 18:28:16, Joonsoo Kim wrote:
> 2020ë 7ì 17ì (ê) ìí 5:26, Michal Hocko <mhocko@xxxxxxxxxx>ëì ìì:
> >
> > On Fri 17-07-20 16:46:38, Joonsoo Kim wrote:
> > > 2020ë 7ì 15ì (ì) ìí 5:24, Michal Hocko <mhocko@xxxxxxxxxx>ëì ìì:
> > > >
> > > > On Wed 15-07-20 14:05:27, Joonsoo Kim wrote:
> > > > > From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> > > > >
> > > > > We have well defined scope API to exclude CMA region.
> > > > > Use it rather than manipulating gfp_mask manually. With this change,
> > > > > we can now use __GFP_MOVABLE for gfp_mask and the ZONE_MOVABLE is also
> > > > > searched by page allocator. For hugetlb, gfp_mask is redefined since
> > > > > it has a regular allocation mask filter for migration target.
> > > > >
> > > > > Note that this can be considered as a fix for the commit 9a4e9f3b2d73
> > > > > ("mm: update get_user_pages_longterm to migrate pages allocated from
> > > > > CMA region"). However, "Fixes" tag isn't added here since it is just
> > > > > suboptimal but it doesn't cause any problem.
> > > >
> > > > But it is breaking the contract that the longterm pins never end up in a
> > > > cma managed memory. So I think Fixes tag is really due. I am not sure
> > > > about stable backport. If the patch was the trivial move of
> > >
> > > Previous implementation is correct since longterm pins never end up in a CMA
> > > managed memory with that implementation. It's just a different and suboptimal
> > > implementation to exclude the CMA area. This is why I don't add the "Fixes"A
> > > tag on the patch.
> >
> > But the current implementation calls memalloc_nocma_restore too early so
> > __gu_longterm_locked will migrate pages possibly to CMA ranges as there
> > is no GFP_MOVABLE restriction in place. Or am I missing something?
> 
> IIUC, calling memalloc_nocma_restore() too early doesn't cause the
> actual problem.
> 
> Final check is done by check_and_migrate_cma_pages() which is outside of
> scope API. If we find a CMA page in between the gup range here, the page is
> migrated to the migration target page and this target page is allocated by
> new_non_cma_page().
> 
> new_non_cma_page() try to allocate the page without __GFP_MOVABLE so
> returned page will not be CMA area memory. Therefore, even if
> memalloc_nocma_restore() is called early, there is no actual problem.

Right you are! I have misread gfp_t gfp_mask = GFP_USER | __GFP_NOWARN
and didn't realize that it doesn't include MOVABLE flag.

Sorry about the noise! The issue is only formal coding style so Fixes
tag could indeed cause more confusion than it would help.

-- 
Michal Hocko
SUSE Labs