The first parameter passed to set_pageblock_migratetype should be the
buddy, not the page. Let's change it back correctly.

Fixes: fd919a85cd55 ("mm: page_isolation: prepare for hygienic freelists")
Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cd584aace6bf..5422f6f8975d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1728,7 +1728,7 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
 
 		del_page_from_free_list(buddy, zone, order,
 					get_pfnblock_migratetype(buddy, pfn));
-		set_pageblock_migratetype(page, migratetype);
+		set_pageblock_migratetype(buddy, migratetype);
 		split_large_buddy(zone, buddy, pfn, order);
 		return true;
 	}
-- 
2.25.1

On Tue, May 14, 2024 at 02:59:33PM +0800, Yajun Deng wrote:
> The first parameter passed to set_pageblock_migratetype should be the
> buddy, not the page. Let's change it back correctly.

No it shouldn't be.

> Fixes: fd919a85cd55 ("mm: page_isolation: prepare for hygienic freelists")
> Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index cd584aace6bf..5422f6f8975d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1728,7 +1728,7 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>  
>  		del_page_from_free_list(buddy, zone, order,
>  					get_pfnblock_migratetype(buddy, pfn));
> -		set_pageblock_migratetype(page, migratetype);
> +		set_pageblock_migratetype(buddy, migratetype);
>  		split_large_buddy(zone, buddy, pfn, order);

[ pageblock | pageblock ]
^             ^
buddy         page

We're trying to isolate the second pageblock, but it's part of a
bigger buddy. Remove buddy from the free list, update the migratetype
of page, then split and free the chunks back. This puts the first
pageblock back on the original freelist and the second block onto the
isolate list.

In the else branch, it's the first block we're interested in:

[ pageblock | pageblock ]
^
page

So again, take the whole thing off, but now mark the first block
isolate, then split and free.

May 14, 2024 at 8:40 PM, "Johannes Weiner" <hannes@cmpxchg.org> wrote:



> 
> On Tue, May 14, 2024 at 02:59:33PM +0800, Yajun Deng wrote:
> 
> > 
> > The first parameter passed to set_pageblock_migratetype should be the
> > 
> >  buddy, not the page. Let's change it back correctly.
> > 
> 
> No it shouldn't be.
> 
> > 
> > Fixes: fd919a85cd55 ("mm: page_isolation: prepare for hygienic freelists")
> > 
> >  Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
> > 
> >  ---
> > 
> >  mm/page_alloc.c | 2 +-
> > 
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> >  
> > 
> >  diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > 
> >  index cd584aace6bf..5422f6f8975d 100644
> > 
> >  --- a/mm/page_alloc.c
> > 
> >  +++ b/mm/page_alloc.c
> > 
> >  @@ -1728,7 +1728,7 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
> > 
> >  
> > 
> >  del_page_from_free_list(buddy, zone, order,
> > 
> >  get_pfnblock_migratetype(buddy, pfn));
> > 
> >  - set_pageblock_migratetype(page, migratetype);
> > 
> >  + set_pageblock_migratetype(buddy, migratetype);
> > 
> >  split_large_buddy(zone, buddy, pfn, order);
> > 
> 
> [ pageblock | pageblock ]
> 
> ^ ^
> 
> buddy page
> 
> We're trying to isolate the second pageblock, but it's part of a
> 
> bigger buddy. Remove buddy from the free list, update the migratetype
> 
> of page, then split and free the chunks back. This puts the first
> 
> pageblock back on the original freelist and the second block onto the
> 
> isolate list.
> 
> In the else branch, it's the first block we're interested in:
> 
> [ pageblock | pageblock ]
> 
> ^
> 
> page
> 
> So again, take the whole thing off, but now mark the first block
> 
> isolate, then split and free.
>

Got it, thank you for your detailed explanation.