[PATCH] mm: reset migratetype if the range spans two pageblocks

[PATCH] mm: reset migratetype if the range spans two pageblocks

[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.

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?

Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 48aaf7b..5c91348 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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.0.0

[PATCH] mm: reset migratetype if the range spans two pageblocks

[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.

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?

Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 48aaf7b..5c91348 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -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.0.0



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

Re: [PATCH] mm: reset migratetype if the range spans two pageblocks

[Xishi Qiu]

On 2015/10/16 18:06, Xishi Qiu wrote:

> __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?
> 

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

Thanks,
Xishi Qiu

> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
>  mm/page_alloc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 48aaf7b..5c91348 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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)

Re: [PATCH] mm: reset migratetype if the range spans two pageblocks

[Xishi Qiu]

On 2015/10/16 18:06, Xishi Qiu wrote:

> __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?
> 

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

Thanks,
Xishi Qiu

> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
>  mm/page_alloc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 48aaf7b..5c91348 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: reset migratetype if the range spans two pageblocks

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24803 bytes --]

Re: [PATCH] mm: reset migratetype if the range spans two pageblocks

[Vlastimil Babka]

On 10/16/2015 12:06 PM, Xishi Qiu wrote:
> __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.

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.

> 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.

>
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
>   mm/page_alloc.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 48aaf7b..5c91348 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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)
>

Re: [PATCH] mm: reset migratetype if the range spans two pageblocks

[Vlastimil Babka]

On 10/16/2015 12:06 PM, Xishi Qiu wrote:
> __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.

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.

> 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.

>
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
>   mm/page_alloc.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 48aaf7b..5c91348 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>