Re: [PATCH] mm: fix is_pinnable_page against on cma page

[Minchan Kim]

On Thu, May 05, 2022 at 10:00:07AM -0700, Mike Kravetz wrote:
> On 5/4/22 23:48, Minchan Kim wrote:
> > On Wed, May 04, 2022 at 03:48:54PM -0700, Minchan Kim wrote:
> >> On Tue, May 03, 2022 at 06:02:33PM +0200, David Hildenbrand wrote:
> >>> On 03.05.22 17:26, Minchan Kim wrote:
> >>>> On Tue, May 03, 2022 at 03:15:24AM +0200, David Hildenbrand wrote:
> >>>>>
> >>>>>>>>> However, I assume we have the same issue right now already with
> >>>>>>> ZONE_MOVABLE and MIGRATE_CMA when trying to pin a page residing on these
> >>>>>>
> >>>>>> ZONE_MOVALBE is also changed dynamically?
> >>>>>>
> >>>>>
> >>>>> Sorry, with "same issue" I meant failing to pin if having to migrate and
> >>>>> the page is temporarily unmovable.
> >>>>>
> >>>>>>> there are temporarily unmovable and we fail to migrate. But it would now
> >>>>>>> apply even without ZONE_MOVABLE or MIGRATE_CMA. Hm...
> >>>>>>
> >>>>>> Didn't parse your last mention.
> >>>>>
> >>>>> On a system that neither uses ZONE_MOVABLE nor MIGRATE_CMA we might have
> >>>>> to migrate now when pinning.
> >>>>
> >>>> I don't understand your point. My problem is pin_user_pages with
> >>>> FOLL_LONGTERM. It shouldn't pin a page from ZONE_MOVABLE and cma area
> >>>> without migrating page out of movable zone or CMA area.
> >>>> That's why try_grab_folio checks whether target page stays in those
> >>>> movable areas. However, to check CMA area, is_migrate_cma_page is
> >>>> racy so the FOLL_LONGTERM flag semantic is broken right now.
> >>>>
> >>>> Do you see any problem of the fix?
> >>>
> >>> My point is that you might decide to migrate a page because you stumble
> >>> over MIGRATE_ISOLATE, although there is no need to reject long-term
> >>> pinning and to trigger page migration.
> >>>
> >>> Assume a system without ZONE_MOVABLE and without MIGRATE_CMA. Assume
> >>> someone reserves gigantic pages (alloc_contig_range()) and you have
> >>> concurrent long-term pinning on a page that is no MIGRATE_ISOLATE.
> >>>
> >>> GUP would see MIGRATE_ISOLATE and would reject pinning. The page has to
> >>> be migrated, which can fail if the page is temporarily unmovable.
> >>
> >> A dump question since I'm not familiar with hugetlb.
> >>
> >> Is above reasonable scenario?
> >>
> >> The gigantic page is about to be created using alloc_contig_range so
> >> they has MIGRATE_ISOLATE as temporal state. It means no one uses the
> >> page yet so I guess the page is not mapped at userspace but other is
> >> trying to access the page using pin_user_pages?
> >>
> > 
> > Too dump question. Never mind.
> > Posted v2 - https://lore.kernel.org/all/20220505064429.2818496-1-minchan@xxxxxxxxxx/T/#u
> > 
> 
> Well your question mentioned hugetlb so my mail filters caught it :)
> 
> Your question caused me to think of the following.  No need for any immediate
> change: I think.  Just wanted to share.
> 
> Suppose someone has reserved CMA for gigantic hugetlb allocations.  And,
> suppose FOLL_LONGTERM is attempted on such a page (it would be in use).  The
> desired action would be to migrate the page out of CMA.  Correct?
> 
> Gigantic pages can only be migrated IF there is another (already allocated)
> gigantic page available.  The routine to try and allocate a page 'on the fly'
> for migration will fail if passed a gigantic size.  There 'might' be a free
> pre-allocated gigantic page.  However, if the user set up CMA reserves for
> gigantic page allocations it is likely the free gigantic page is also in CMA.
> Therefore, it can not be used for this migration.  So, unless my reasoning
> is wrong, FOLL_LONGTERM would almost always fail for gigantic pages in CMA.

FOLL_LONGTERM on CMA-backed gigantic page would already fail, Thanks for sharing!

Anyway, David's concern was non-CMA-backed gigantic page. The alloc_contig_range 
with MIGRATE_ISOLATE runs with concurrent FOLL_LONGTERM pinning, which could
trigger page migration we didn't have before so it might increase FOLL_LONGTERM
GUP failure rate.