Skip to contentSkip to conversation

mm: reset migratetype if the range spans two pageblocks

PATCH In review Xishi Qiu 3 participants 4 messages Oct 16, 2015

Subsystems (2)
lore
Share & tools
lore

Xishi Qiu

author
#0 lore raw

PATCH 1/1 mm: reset migratetype if the range spans two pageblocks

__rmqueue_fallback() will change the migratetype of pageblock, so it is possible that two continuous pageblocks have different migratetypes.

When freeing all pages of the two blocks, they will be merged to 4M, and added to the buddy list which the migratetype is the first pageblock's.

If later alloc some pages and split the 4M, the second pageblock will be added to the buddy list, and the migratetype is the first pageblock's, so it is different from the its pageblock's.

That means the page in buddy list's migratetype is different from the page in pageblock's migratetype. This will make confusion.

However,if we change the hotpath, it will be performance degradation, so any better ideas?

---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)
mm/page_alloc.c +3 -0
index 48aaf7b..5c91348 100644
@@ -726,6 +726,9 @@ static inline void __free_one_page(struct page *page,
 	list_add(&page->lru, &zone->free_area[order].free_list[migratetype]);
 out:
 	zone->free_area[order].nr_free++;
+	/* If the range spans two pageblocks, reset the migratetype. */
+	if (order > pageblock_order)
+		change_pageblock_range(page, order, migratetype);
 }
 
 static inline int free_pages_check(struct page *page)
Signature · 1 line
2.0.0
author
#1 lore raw
Quoted 12 lines · Xishi Qiu

__rmqueue_fallback() will change the migratetype of pageblock, so it is possible that two continuous pageblocks have different migratetypes.

When freeing all pages of the two blocks, they will be merged to 4M, and added to the buddy list which the migratetype is the first pageblock's.

If later alloc some pages and split the 4M, the second pageblock will be added to the buddy list, and the migratetype is the first pageblock's, so it is different from the its pageblock's.

Quoted 6 lines

That means the page in buddy list's migratetype is different from the page in pageblock's migratetype. This will make confusion.

However,if we change the hotpath, it will be performance degradation, so any better ideas?

How about using get_pfnblock_migratetype() to get the pageblock's migratetype first, and compare them?

Thanks,
Xishi Qiu

Quoted 19 lines
---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)
mm/page_alloc.c +3 -0
index 48aaf7b..5c91348 100644
@@ -726,6 +726,9 @@ static inline void __free_one_page(struct page *page,
 	list_add(&page->lru, &zone->free_area[order].free_list[migratetype]);
 out:
 	zone->free_area[order].nr_free++;
+	/* If the range spans two pageblocks, reset the migratetype. */
+	if (order > pageblock_order)
+		change_pageblock_range(page, order, migratetype);
 }
 
 static inline int free_pages_check(struct page *page)
#2 lore raw
In reply to Xishi Qiu
reproduce:

Hi Xishi,

[auto build test ERROR on v4.3-rc5 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Xishi-Qiu/mm-reset-migratetype-if-the-range-spans-two-pageblocks/20151016-180949
config: i386-randconfig-s1-201541 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

mm/page_alloc.c: In function '__free_one_page':

>> mm/page_alloc.c:731:3: error: implicit declaration of function 'change_pageblock_range' [-Werror=implicit-function-declaration]
      change_pageblock_range(page, order, migratetype);
      ^
   mm/page_alloc.c: At top level:
>> mm/page_alloc.c:1515:13: warning: conflicting types for 'change_pageblock_range'
    static void change_pageblock_range(struct page *pageblock_page,
                ^
>> mm/page_alloc.c:1515:13: error: static declaration of 'change_pageblock_range' follows non-static declaration
   mm/page_alloc.c:731:3: note: previous implicit declaration of 'change_pageblock_range' was here
      change_pageblock_range(page, order, migratetype);
      ^

cc1: some warnings being treated as errors

vim +/change_pageblock_range +731 mm/page_alloc.c

725
726 list_add(&page->lru, &zone->free_area[order].free_list[migratetype]);
727 out:
728 zone->free_area[order].nr_free++;
729 /* If the range spans two pageblocks, reset the migratetype. */
730 if (order > pageblock_order)

 > 731			change_pageblock_range(page, order, migratetype);

732 }
733
734 static inline int free_pages_check(struct page *page)

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#3 lore raw
In reply to Xishi Qiu
Quoted 8 lines · Xishi Qiu

__rmqueue_fallback() will change the migratetype of pageblock, so it is possible that two continuous pageblocks have different migratetypes.

When freeing all pages of the two blocks, they will be merged to 4M, and added to the buddy list which the migratetype is the first pageblock's.

Quoted 6 lines

If later alloc some pages and split the 4M, the second pageblock will be added to the buddy list, and the migratetype is the first pageblock's, so it is different from the its pageblock's.

That means the page in buddy list's migratetype is different from the page in pageblock's migratetype. This will make confusion.

So what will be the bad effects of this confusion? There are many situations where a free page (of any size) will be on different list than the pageblock's migratetype.

In case of full free pageblock, it IIRC doesn't really matter on which freelist it is, as the fallback scenarios for all migratetypes are trivial and non-fragmenting (just grab the whole pageblock, which means the migratetype is updated).

Maybe compaction will get it wrong when deciding which pageblocks are suitable for scanning, but that's not critical.

Quoted 2 lines

However,if we change the hotpath, it will be performance degradation, so any better ideas?

I don't see immediately a way to fix that outside of hotpath, and don't see the reasons being strong enough to fix it in the hotpath.

Quoted 12 lines
---
  mm/page_alloc.c | 3 +++
  1 file changed, 3 insertions(+)
mm/page_alloc.c +0 -0
index 48aaf7b..5c91348 100644

mm/page_alloc.c static inline void __free_one_page(struct page *page, · line 734

Quoted 9 lines
mm/page_alloc.c +3 -0
@@ -726,6 +726,9 @@ static inline void __free_one_page(struct page *page,
  	list_add(&page->lru, &zone->free_area[order].free_list[migratetype]);
  out:
  	zone->free_area[order].nr_free++;
+	/* If the range spans two pageblocks, reset the migratetype. */
+	if (order > pageblock_order)
+		change_pageblock_range(page, order, migratetype);
  }
 
  static inline int free_pages_check(struct page *page)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>