Re: [PATCH 1/4] mm: migrate: use a folio in add_page_for_migration()

[Mike Kravetz]

On 08/10/23 09:49, Kefeng Wang wrote:
> 
> 
> On 2023/8/10 6:44, Mike Kravetz wrote:
> > On 08/09/23 13:53, Mike Kravetz wrote:
> > > On 08/09/23 20:37, Kefeng Wang wrote:
> > > > > 
> > > > > Cc Mike to help us clarify the expected behavior of hugetlb.
> > > > > 
> > > > > Hi Mike, what is the expected behavior, if a user tries to use move_pages()
> > > > > to migrate a non head page of a hugetlb page?
> > > > 
> > > > Could you give some advise, thanks
> > > > 
> > > 
> > > Sorry, I was away for a while.
> > > 
> > > It seems unfortunate that move_pages says the passed user addresses
> > > should be aligned to page boundaries.  However, IIUC this is not checked
> > > or enforced.  Otherwise, passing a hugetlb page should return the same
> > > error.
> > > 
> > > One thought would be that hugetlb mappings should behave the same
> > > non-hugetlb mappings.  If passed the address of a hugetlb tail page, align
> > > the address to a hugetlb boundary and migrate the page.  This changes the
> > > existing behavior.  However, it would be hard to imagine anyone depending
> > > on this.
> > > 
> > > After taking a closer look at the add_page_for_migration(), it seems to
> > > just ignore passed tail pages and do nothing for such passed addresses.
> > > Correct?  Or, am I missing something?  Perhaps that is behavior we want/
> > > need to preserve?
> > 
> > My mistake, status -EACCES is returned when passing a tail page of a
> > hugetlb page.
> > 
> 
> As mentioned in previous mail， before e66f17ff7177 ("mm/hugetlb: take
> page table lock in follow_huge_pmd()") in v4.0, follow_page() will
> return NULL on tail page for Huagetlb page, so move_pages() will return
> -ENOENT errno, but after that commit, -EACCES is returned.
> 
> Meanwhile, the behavior of THP/HUGETLB is different, the whole THP will be
> migrated on a tail page, but HUGETLB will return -EACCES(after v4.0)
> or -ENOENT(before v4.0) on tail page.
> 
> > Back to the question of 'What is the expected behavior if a tail page is
> > passed?'.  I do not think we have defined an expected behavior.  If
> > anything is 'expected' I would say it is -EACCES as returned today.
> > 
> 
> My question is,
> 
> Should we keep seem behavior between HUGETLB and THP, or only change the
> errno from -EACCES to -ENOENT/-EBUSY.

Just to be clear.  When you say "keep seem behavior between HUGETLB and THP",
are you saying that you would like hugetlb to perform migration of the entire
hugetlb page if a tail page is passed?

IMO, this would be ideal as it would mean that hugetlb and THP behave the same
when passed the address of a tail page.  The fewer places where hugetlb
behavior diverges, the better.  However, this does change behavior.

As mentioned above, I have a hard time imagining someone depending on the
behavior that passing the address of a hugetlb tail page returns error.  But,
this is almost impossible to predict.

Thoughts from others?  
-- 
Mike Kravetz

> 
> I would like to drop PageHead() check for Hugetlb to keep seem behavior,
> which will keep seem error code if isolate fail or success on head/tail
> page.
> 
> Thanks.