diff mbox series

[05/10] mm: page_alloc: fix move_freepages_block() range error

Message ID 20240320180429.678181-6-hannes@cmpxchg.org (mailing list archive)
State New
Headers show
Series mm: page_alloc: freelist migratetype hygiene | expand

Commit Message

Johannes Weiner March 20, 2024, 6:02 p.m. UTC
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 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Vlastimil Babka March 25, 2024, 4:22 p.m. UTC | #1
On 3/20/24 7:02 PM, 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>

> ---
>  mm/page_alloc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a1376a6fe7e4..7373329763e6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1645,9 +1645,15 @@ int move_freepages_block(struct zone *zone, struct page *page,
>  	start_pfn = pageblock_start_pfn(pfn);
>  	end_pfn = pageblock_end_pfn(pfn) - 1;
>  
> -	/* Do not cross zone boundaries */
> +	/*
> +	 * The caller only has the lock for @zone, don't touch ranges
> +	 * that straddle into other zones. While we could move part of
> +	 * the range that's inside the zone, this call is usually
> +	 * accompanied by other operations such as migratetype updates
> +	 * which also should be locked.
> +	 */
>  	if (!zone_spans_pfn(zone, start_pfn))
> -		start_pfn = pfn;
> +		return 0;
>  	if (!zone_spans_pfn(zone, end_pfn))
>  		return 0;
>
diff mbox series

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a1376a6fe7e4..7373329763e6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1645,9 +1645,15 @@  int move_freepages_block(struct zone *zone, struct page *page,
 	start_pfn = pageblock_start_pfn(pfn);
 	end_pfn = pageblock_end_pfn(pfn) - 1;
 
-	/* Do not cross zone boundaries */
+	/*
+	 * The caller only has the lock for @zone, don't touch ranges
+	 * that straddle into other zones. While we could move part of
+	 * the range that's inside the zone, this call is usually
+	 * accompanied by other operations such as migratetype updates
+	 * which also should be locked.
+	 */
 	if (!zone_spans_pfn(zone, start_pfn))
-		start_pfn = pfn;
+		return 0;
 	if (!zone_spans_pfn(zone, end_pfn))
 		return 0;