From: Fan Ni <fan.ni@samsung.com>

Sending out this patch per Matthew Wilcox's suggestion 
that we need to convert is_migrate_isolate_page() to use folio
https://lore.kernel.org/linux-mm/Z_XmUrbxKtYmzmJ6@casper.infradead.org/

However, when looking into the code, I have noticed that among the uers
of is_migrate_isolate_page(), in most cases the page passed in is from a 
a pageblock. 
I am not sure how we should proceed with these cases.
Should we deal with pageblock or just leave it as it is and only do the page
to folio conversion for the pages within?

So This RFC is mainly sent for collecting input about how to move forward.



Fan Ni (1):
  mm: Convert is_migrate_isolate_page() to is_migrate_isolate_folio()

 include/linux/page-isolation.h |  6 +++---
 mm/hugetlb.c                   |  2 +-
 mm/page_isolation.c            | 10 +++++-----
 3 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.47.2

On Tue, May 06, 2025 at 11:38:28AM -0700, nifan.cxl@gmail.com wrote:
> From: Fan Ni <fan.ni@samsung.com>
> 
> Sending out this patch per Matthew Wilcox's suggestion 
> that we need to convert is_migrate_isolate_page() to use folio
> https://lore.kernel.org/linux-mm/Z_XmUrbxKtYmzmJ6@casper.infradead.org/

That's not what I said!

This is what I said:
> >  
> > -		if (is_migrate_isolate_page(&folio->page))
> > +		if (is_migrate_isolate_page(folio_page(folio, 0)))
> >  			continue;
>
> I think we need an is_migrate_isolate_folio() instead of this.

> However, when looking into the code, I have noticed that among the uers
> of is_migrate_isolate_page(), in most cases the page passed in is from a 
> a pageblock. 
> I am not sure how we should proceed with these cases.
> Should we deal with pageblock or just leave it as it is and only do the page
> to folio conversion for the pages within?

Neither.  Add a folio_test_migrate_isolate() in addition to
is_migrate_isolate_page().  Don't force a conversion as it's a
legitimate question to ask of pages as well as of folios.
And some of the pages you want to ask it of may well not be part of
folios (they may be part of a slab or some other memdesc).

On Tue, May 06, 2025 at 08:08:55PM +0100, Matthew Wilcox wrote:
> On Tue, May 06, 2025 at 11:38:28AM -0700, nifan.cxl@gmail.com wrote:
> > From: Fan Ni <fan.ni@samsung.com>
> > 
> > Sending out this patch per Matthew Wilcox's suggestion 
> > that we need to convert is_migrate_isolate_page() to use folio
> > https://lore.kernel.org/linux-mm/Z_XmUrbxKtYmzmJ6@casper.infradead.org/
> 
> That's not what I said!
> 
> This is what I said:
> > >  
> > > -		if (is_migrate_isolate_page(&folio->page))
> > > +		if (is_migrate_isolate_page(folio_page(folio, 0)))
> > >  			continue;
> >
> > I think we need an is_migrate_isolate_folio() instead of this.
> 
> > However, when looking into the code, I have noticed that among the uers
> > of is_migrate_isolate_page(), in most cases the page passed in is from a 
> > a pageblock. 
> > I am not sure how we should proceed with these cases.
> > Should we deal with pageblock or just leave it as it is and only do the page
> > to folio conversion for the pages within?
> 
> Neither.  Add a folio_test_migrate_isolate() in addition to
> is_migrate_isolate_page().  Don't force a conversion as it's a
> legitimate question to ask of pages as well as of folios.
> And some of the pages you want to ask it of may well not be part of
> folios (they may be part of a slab or some other memdesc).

Oh. I misunderstood "we need ... instead of .." :-(.
Thanks for the the clarification.

Another separate question.

We have a free_frozen_pages(page, order), which have two types of users
1) head page and order directly from a struct folio; or
2) page pointer that does not neccesarily be the head page of a
folio and order that may not be directly related to a folio;

Does it make sense to introduce a dedicate function like
free_frozen_folio(struct folio *folio) to handle case 1)?

Fan



-- 
Fan Ni

On Tue, May 06, 2025 at 12:23:15PM -0700, Fan Ni wrote:
> We have a free_frozen_pages(page, order), which have two types of users
> 1) head page and order directly from a struct folio; or
> 2) page pointer that does not neccesarily be the head page of a
> folio and order that may not be directly related to a folio;
> 
> Does it make sense to introduce a dedicate function like
> free_frozen_folio(struct folio *folio) to handle case 1)?

No.  free_frozen_pages() will eventually be just free_pages()
when struct page has lost its refcount (as the refcount will have moved
into the memdescs which need it).  It's premature to do anything to it
now, we have many steps to go.