On 29.09.20 11:18, Wei Yang wrote:

On Mon, Sep 28, 2020 at 08:21:08PM +0200, David Hildenbrand wrote:

quoted

Page isolation doesn't actually touch the pages, it simply isolates
pageblocks and moves all free pages to the MIGRATE_ISOLATE freelist.

We already place pages to the tail of the freelists when undoing
isolation via __putback_isolated_page(), let's do it in any case
(e.g., if order <= pageblock_order) and document the behavior.

Add a "to_tail" parameter to move_freepages_block() but introduce a
a new move_to_free_list_tail() - similar to add_to_free_list_tail().

s/a a/a/

quoted

This change results in all pages getting onlined via online_pages() to
be placed to the tail of the freelist.

Reviewed-by: Oscar Salvador <osalvador@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexander Duyck <redacted>
Cc: Mel Gorman <redacted>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Dave Hansen <redacted>
Cc: Vlastimil Babka <redacted>
Cc: Wei Yang <redacted>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Scott Cheloha <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: David Hildenbrand <redacted>
---
include/linux/page-isolation.h |  4 ++--
mm/page_alloc.c                | 35 +++++++++++++++++++++++-----------
mm/page_isolation.c            | 12 +++++++++---
3 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 572458016331..3eca9b3c5305 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h

@@ -36,8 +36,8 @@ static inline bool is_migrate_isolate(int migratetype)

struct page *has_unmovable_pages(struct zone *zone, struct page *page,
				 int migratetype, int flags);
void set_pageblock_migratetype(struct page *page, int migratetype);
-int move_freepages_block(struct zone *zone, struct page *page,
-				int migratetype, int *num_movable);
+int move_freepages_block(struct zone *zone, struct page *page, int migratetype,
+			 bool to_tail, int *num_movable);

/*
 * Changes migrate type in [start_pfn, end_pfn) to be MIGRATE_ISOLATE.

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9e3ed4a6f69a..d5a5f528b8ca 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c

@@ -905,6 +905,15 @@ static inline void move_to_free_list(struct page *page, struct zone *zone,

	list_move(&page->lru, &area->free_list[migratetype]);
}

+/* Used for pages which are on another list */
+static inline void move_to_free_list_tail(struct page *page, struct zone *zone,
+					  unsigned int order, int migratetype)
+{
+	struct free_area *area = &zone->free_area[order];
+
+	list_move_tail(&page->lru, &area->free_list[migratetype]);
+}
+

Would it be better to pass the *to_tail* to move_to_free_list(), so we won't
have a new function?

Hi,

thanks for the review!

See discussion in RFC + cover letter:

"Add a "to_tail" parameter to move_freepages_block() but introduce a new
move_to_free_list_tail() - similar to add_to_free_list_tail()."