When a block is partially outside the zone of the cursor page, the
function cuts the range to the pivot page instead of the zone
start. This can leave large parts of the block behind, which
encourages incompatible page mixing down the line (ask for one type,
get another), and thus long-term fragmentation.

This triggers reliably on the first block in the DMA zone, whose
start_pfn is 1. The block is stolen, but everything before the pivot
page (which was often hundreds of pages) is left on the old list.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 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 f6f658c3d394..5bbe5f3be5ad 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1652,7 +1652,7 @@ int move_freepages_block(struct zone *zone, struct page *page,
 
 	/* Do not cross zone boundaries */
 	if (!zone_spans_pfn(zone, start_pfn))
-		start_pfn = pfn;
+		start_pfn = zone->zone_start_pfn;
 	if (!zone_spans_pfn(zone, end_pfn))
 		return 0;
 
-- 
2.42.0

On Mon, Sep 11, 2023 at 03:41:45PM -0400, Johannes Weiner wrote:
> When a block is partially outside the zone of the cursor page, the
> function cuts the range to the pivot page instead of the zone
> start. This can leave large parts of the block behind, which
> encourages incompatible page mixing down the line (ask for one type,
> get another), and thus long-term fragmentation.
> 
> This triggers reliably on the first block in the DMA zone, whose
> start_pfn is 1. The block is stolen, but everything before the pivot
> page (which was often hundreds of pages) is left on the old list.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Oops

Acked-by: Mel Gorman <mgorman@techsingularity.net>

-- 
Mel Gorman
SUSE Labs

On 9/11/23 21:41, Johannes Weiner wrote:
> When a block is partially outside the zone of the cursor page, the
> function cuts the range to the pivot page instead of the zone
> start. This can leave large parts of the block behind, which
> encourages incompatible page mixing down the line (ask for one type,
> get another), and thus long-term fragmentation.
> 
> This triggers reliably on the first block in the DMA zone, whose
> start_pfn is 1. The block is stolen, but everything before the pivot
> page (which was often hundreds of pages) is left on the old list.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Note below:

> ---
>  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 f6f658c3d394..5bbe5f3be5ad 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1652,7 +1652,7 @@ int move_freepages_block(struct zone *zone, struct page *page,
>  
>  	/* Do not cross zone boundaries */
>  	if (!zone_spans_pfn(zone, start_pfn))
> -		start_pfn = pfn;
> +		start_pfn = zone->zone_start_pfn;
>  	if (!zone_spans_pfn(zone, end_pfn))
>  		return 0;

Culdn't we also adjust end_pfn to zone_end_pfn() so we don't just ignore the
last half-pageblock for no good reason? (or am I missing any?)
Also would stop treating end_pfn as inclusive here and in move_freepages(),
it's rather uncommon.

>

On Wed, Sep 13, 2023 at 04:40:48PM +0200, Vlastimil Babka wrote:
> On 9/11/23 21:41, Johannes Weiner wrote:
> > When a block is partially outside the zone of the cursor page, the
> > function cuts the range to the pivot page instead of the zone
> > start. This can leave large parts of the block behind, which
> > encourages incompatible page mixing down the line (ask for one type,
> > get another), and thus long-term fragmentation.
> > 
> > This triggers reliably on the first block in the DMA zone, whose
> > start_pfn is 1. The block is stolen, but everything before the pivot
> > page (which was often hundreds of pages) is left on the old list.
> > 
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

> > @@ -1652,7 +1652,7 @@ int move_freepages_block(struct zone *zone, struct page *page,
> >  
> >  	/* Do not cross zone boundaries */
> >  	if (!zone_spans_pfn(zone, start_pfn))
> > -		start_pfn = pfn;
> > +		start_pfn = zone->zone_start_pfn;
> >  	if (!zone_spans_pfn(zone, end_pfn))
> >  		return 0;
> 
> Culdn't we also adjust end_pfn to zone_end_pfn() so we don't just ignore the
> last half-pageblock for no good reason? (or am I missing any?)
> Also would stop treating end_pfn as inclusive here and in move_freepages(),
> it's rather uncommon.

You raise a good point here and in the reply to 5/6. Let me reply to
the other email.

On 11 Sep 2023, at 15:41, Johannes Weiner wrote:

> When a block is partially outside the zone of the cursor page, the
> function cuts the range to the pivot page instead of the zone
> start. This can leave large parts of the block behind, which
> encourages incompatible page mixing down the line (ask for one type,
> get another), and thus long-term fragmentation.
>
> This triggers reliably on the first block in the DMA zone, whose
> start_pfn is 1. The block is stolen, but everything before the pivot
> page (which was often hundreds of pages) is left on the old list.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>


--
Best Regards,
Yan, Zi