[RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[Yisheng Xie]

We had considered all of the non-lru pages as unmovable before
commit bda807d44454 ("mm: migrate: support non-lru movable page
migration"). But now some of non-lru pages like zsmalloc,
virtio-balloon pages also become movable. So we can offline such
blocks by using non-lru page migration.

This patch straightforwardly add non-lru migration code, which
means adding non-lru related code to the functions which scan
over pfn and collect pages to be migrated and isolate them before
migration.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2
 make a minor change about lock_page logic in function scan_movable_pages.

 mm/memory_hotplug.c | 36 +++++++++++++++++++++++++-----------
 mm/page_alloc.c     |  8 ++++++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e43142c1..5559175 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1510,10 +1510,10 @@ int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn)
 }
 
 /*
- * Scan pfn range [start,end) to find movable/migratable pages (LRU pages
- * and hugepages). We scan pfn because it's much easier than scanning over
- * linked list. This function returns the pfn of the first found movable
- * page if it's found, otherwise 0.
+ * Scan pfn range [start,end) to find movable/migratable pages (LRU pages,
+ * non-lru movable pages and hugepages). We scan pfn because it's much
+ * easier than scanning over linked list. This function returns the pfn
+ * of the first found movable page if it's found, otherwise 0.
  */
 static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 {
@@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 					pfn = round_up(pfn + 1,
 						1 << compound_order(page)) - 1;
 			}
+			/*
+			 * check __PageMovable in lock_page to avoid miss some
+			 * non-lru movable pages at race condition.
+			 */
+			lock_page(page);
+			if (__PageMovable(page)) {
+				unlock_page(page);
+				return pfn;
+			}
+			unlock_page(page);
 		}
 	}
 	return 0;
@@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private,
 		if (!get_page_unless_zero(page))
 			continue;
 		/*
-		 * We can skip free pages. And we can only deal with pages on
-		 * LRU.
+		 * We can skip free pages. And we can deal with pages on
+		 * LRU and non-lru movable pages.
 		 */
-		ret = isolate_lru_page(page);
+		if (PageLRU(page))
+			ret = isolate_lru_page(page);
+		else
+			ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);
 		if (!ret) { /* Success */
 			put_page(page);
 			list_add_tail(&page->lru, &source);
 			move_pages--;
-			inc_node_page_state(page, NR_ISOLATED_ANON +
-					    page_is_file_cache(page));
+			if (!__PageMovable(page))
+				inc_node_page_state(page, NR_ISOLATED_ANON +
+						    page_is_file_cache(page));
 
 		} else {
 #ifdef CONFIG_DEBUG_VM
-			pr_alert("removing pfn %lx from LRU failed\n", pfn);
-			dump_page(page, "failed to remove from LRU");
+			pr_alert("failed to isolate pfn %lx\n", pfn);
+			dump_page(page, "isolation failed");
 #endif
 			put_page(page);
 			/* Because we don't have big zone->lock. we should
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d604d25..52d3067 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7055,8 +7055,9 @@ void *__init alloc_large_system_hash(const char *tablename,
  * If @count is not zero, it is okay to include less @count unmovable pages
  *
  * PageLRU check without isolation or lru_lock could race so that
- * MIGRATE_MOVABLE block might include unmovable pages. It means you can't
- * expect this function should be exact.
+ * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
+ * check without lock_page also may miss some movable non-lru pages at
+ * race condition. So you can't expect this function should be exact.
  */
 bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 			 bool skip_hwpoisoned_pages)
@@ -7112,6 +7113,9 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 		if (skip_hwpoisoned_pages && PageHWPoison(page))
 			continue;
 
+		if (__PageMovable(page))
+			continue;
+
 		if (!PageLRU(page))
 			found++;
 		/*
-- 
1.7.12.4

[RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[Yisheng Xie]

We had considered all of the non-lru pages as unmovable before
commit bda807d44454 ("mm: migrate: support non-lru movable page
migration"). But now some of non-lru pages like zsmalloc,
virtio-balloon pages also become movable. So we can offline such
blocks by using non-lru page migration.

This patch straightforwardly add non-lru migration code, which
means adding non-lru related code to the functions which scan
over pfn and collect pages to be migrated and isolate them before
migration.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2
 make a minor change about lock_page logic in function scan_movable_pages.

 mm/memory_hotplug.c | 36 +++++++++++++++++++++++++-----------
 mm/page_alloc.c     |  8 ++++++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e43142c1..5559175 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1510,10 +1510,10 @@ int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn)
 }
 
 /*
- * Scan pfn range [start,end) to find movable/migratable pages (LRU pages
- * and hugepages). We scan pfn because it's much easier than scanning over
- * linked list. This function returns the pfn of the first found movable
- * page if it's found, otherwise 0.
+ * Scan pfn range [start,end) to find movable/migratable pages (LRU pages,
+ * non-lru movable pages and hugepages). We scan pfn because it's much
+ * easier than scanning over linked list. This function returns the pfn
+ * of the first found movable page if it's found, otherwise 0.
  */
 static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 {
@@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 					pfn = round_up(pfn + 1,
 						1 << compound_order(page)) - 1;
 			}
+			/*
+			 * check __PageMovable in lock_page to avoid miss some
+			 * non-lru movable pages at race condition.
+			 */
+			lock_page(page);
+			if (__PageMovable(page)) {
+				unlock_page(page);
+				return pfn;
+			}
+			unlock_page(page);
 		}
 	}
 	return 0;
@@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private,
 		if (!get_page_unless_zero(page))
 			continue;
 		/*
-		 * We can skip free pages. And we can only deal with pages on
-		 * LRU.
+		 * We can skip free pages. And we can deal with pages on
+		 * LRU and non-lru movable pages.
 		 */
-		ret = isolate_lru_page(page);
+		if (PageLRU(page))
+			ret = isolate_lru_page(page);
+		else
+			ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);
 		if (!ret) { /* Success */
 			put_page(page);
 			list_add_tail(&page->lru, &source);
 			move_pages--;
-			inc_node_page_state(page, NR_ISOLATED_ANON +
-					    page_is_file_cache(page));
+			if (!__PageMovable(page))
+				inc_node_page_state(page, NR_ISOLATED_ANON +
+						    page_is_file_cache(page));
 
 		} else {
 #ifdef CONFIG_DEBUG_VM
-			pr_alert("removing pfn %lx from LRU failed\n", pfn);
-			dump_page(page, "failed to remove from LRU");
+			pr_alert("failed to isolate pfn %lx\n", pfn);
+			dump_page(page, "isolation failed");
 #endif
 			put_page(page);
 			/* Because we don't have big zone->lock. we should
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d604d25..52d3067 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7055,8 +7055,9 @@ void *__init alloc_large_system_hash(const char *tablename,
  * If @count is not zero, it is okay to include less @count unmovable pages
  *
  * PageLRU check without isolation or lru_lock could race so that
- * MIGRATE_MOVABLE block might include unmovable pages. It means you can't
- * expect this function should be exact.
+ * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
+ * check without lock_page also may miss some movable non-lru pages at
+ * race condition. So you can't expect this function should be exact.
  */
 bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 			 bool skip_hwpoisoned_pages)
@@ -7112,6 +7113,9 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 		if (skip_hwpoisoned_pages && PageHWPoison(page))
 			continue;
 
+		if (__PageMovable(page))
+			continue;
+
 		if (!PageLRU(page))
 			found++;
 		/*
-- 
1.7.12.4

--
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: [RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[Michal Hocko]

On Wed 25-01-17 14:59:45, Yisheng Xie wrote:
> We had considered all of the non-lru pages as unmovable before
> commit bda807d44454 ("mm: migrate: support non-lru movable page
> migration"). But now some of non-lru pages like zsmalloc,
> virtio-balloon pages also become movable. So we can offline such
> blocks by using non-lru page migration.
> 
> This patch straightforwardly add non-lru migration code, which
> means adding non-lru related code to the functions which scan
> over pfn and collect pages to be migrated and isolate them before
> migration.
> 
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2
>  make a minor change about lock_page logic in function scan_movable_pages.
> 
>  mm/memory_hotplug.c | 36 +++++++++++++++++++++++++-----------
>  mm/page_alloc.c     |  8 ++++++--
>  2 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e43142c1..5559175 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1510,10 +1510,10 @@ int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn)
>  }
>  
>  /*
> - * Scan pfn range [start,end) to find movable/migratable pages (LRU pages
> - * and hugepages). We scan pfn because it's much easier than scanning over
> - * linked list. This function returns the pfn of the first found movable
> - * page if it's found, otherwise 0.
> + * Scan pfn range [start,end) to find movable/migratable pages (LRU pages,
> + * non-lru movable pages and hugepages). We scan pfn because it's much
> + * easier than scanning over linked list. This function returns the pfn
> + * of the first found movable page if it's found, otherwise 0.
>   */
>  static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  {
> @@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  					pfn = round_up(pfn + 1,
>  						1 << compound_order(page)) - 1;
>  			}
> +			/*
> +			 * check __PageMovable in lock_page to avoid miss some
> +			 * non-lru movable pages at race condition.
> +			 */
> +			lock_page(page);
> +			if (__PageMovable(page)) {
> +				unlock_page(page);
> +				return pfn;
> +			}
> +			unlock_page(page);

This doesn't make any sense to me. __PageMovable can change right after
you drop the lock so why the race matters? If we cannot tolerate races
then the above doesn't work and if we can then taking the lock is
pointless.

>  		}
>  	}
>  	return 0;
> @@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private,
>  		if (!get_page_unless_zero(page))
>  			continue;
>  		/*
> -		 * We can skip free pages. And we can only deal with pages on
> -		 * LRU.
> +		 * We can skip free pages. And we can deal with pages on
> +		 * LRU and non-lru movable pages.
>  		 */
> -		ret = isolate_lru_page(page);
> +		if (PageLRU(page))
> +			ret = isolate_lru_page(page);
> +		else
> +			ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);

we really want to propagate the proper error code to the caller.

-- 
Michal Hocko
SUSE Labs

Re: [RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[Michal Hocko]

On Wed 25-01-17 14:59:45, Yisheng Xie wrote:
> We had considered all of the non-lru pages as unmovable before
> commit bda807d44454 ("mm: migrate: support non-lru movable page
> migration"). But now some of non-lru pages like zsmalloc,
> virtio-balloon pages also become movable. So we can offline such
> blocks by using non-lru page migration.
> 
> This patch straightforwardly add non-lru migration code, which
> means adding non-lru related code to the functions which scan
> over pfn and collect pages to be migrated and isolate them before
> migration.
> 
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2
>  make a minor change about lock_page logic in function scan_movable_pages.
> 
>  mm/memory_hotplug.c | 36 +++++++++++++++++++++++++-----------
>  mm/page_alloc.c     |  8 ++++++--
>  2 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e43142c1..5559175 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1510,10 +1510,10 @@ int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn)
>  }
>  
>  /*
> - * Scan pfn range [start,end) to find movable/migratable pages (LRU pages
> - * and hugepages). We scan pfn because it's much easier than scanning over
> - * linked list. This function returns the pfn of the first found movable
> - * page if it's found, otherwise 0.
> + * Scan pfn range [start,end) to find movable/migratable pages (LRU pages,
> + * non-lru movable pages and hugepages). We scan pfn because it's much
> + * easier than scanning over linked list. This function returns the pfn
> + * of the first found movable page if it's found, otherwise 0.
>   */
>  static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  {
> @@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  					pfn = round_up(pfn + 1,
>  						1 << compound_order(page)) - 1;
>  			}
> +			/*
> +			 * check __PageMovable in lock_page to avoid miss some
> +			 * non-lru movable pages at race condition.
> +			 */
> +			lock_page(page);
> +			if (__PageMovable(page)) {
> +				unlock_page(page);
> +				return pfn;
> +			}
> +			unlock_page(page);

This doesn't make any sense to me. __PageMovable can change right after
you drop the lock so why the race matters? If we cannot tolerate races
then the above doesn't work and if we can then taking the lock is
pointless.

>  		}
>  	}
>  	return 0;
> @@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private,
>  		if (!get_page_unless_zero(page))
>  			continue;
>  		/*
> -		 * We can skip free pages. And we can only deal with pages on
> -		 * LRU.
> +		 * We can skip free pages. And we can deal with pages on
> +		 * LRU and non-lru movable pages.
>  		 */
> -		ret = isolate_lru_page(page);
> +		if (PageLRU(page))
> +			ret = isolate_lru_page(page);
> +		else
> +			ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);

we really want to propagate the proper error code to the caller.

-- 
Michal Hocko
SUSE Labs

--
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: [RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[Yisheng Xie]

hi Michal,
Thank you for reviewing and sorry for late reply.

On 01/26/2017 05:43 PM, Michal Hocko wrote:
> On Wed 25-01-17 14:59:45, Yisheng Xie wrote:
>
>  static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  {
> @@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  					pfn = round_up(pfn + 1,
>  						1 << compound_order(page)) - 1;
>  			}
> +			/*
> +			 * check __PageMovable in lock_page to avoid miss some
> +			 * non-lru movable pages at race condition.
> +			 */
> +			lock_page(page);
> +			if (__PageMovable(page)) {
> +				unlock_page(page);
> +				return pfn;
> +			}
> +			unlock_page(page);
> This doesn't make any sense to me. __PageMovable can change right after
> you drop the lock so why the race matters? If we cannot tolerate races
> then the above doesn't work and if we can then taking the lock is
> pointless.
hmm, for PageLRU check may also race without lru-lock，
I think it is ok to check __PageMovable without lock_page, here.

>>  		}
>>  	}
>>  	return 0;
>> @@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private,
>>  		if (!get_page_unless_zero(page))
>>  			continue;
>>  		/*
>> -		 * We can skip free pages. And we can only deal with pages on
>> -		 * LRU.
>> +		 * We can skip free pages. And we can deal with pages on
>> +		 * LRU and non-lru movable pages.
>>  		 */
>> -		ret = isolate_lru_page(page);
>> +		if (PageLRU(page))
>> +			ret = isolate_lru_page(page);
>> +		else
>> +			ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);
> we really want to propagate the proper error code to the caller.
Yes , I make the same mistake again. Really sorry about that.

Maybe I can rewrite the isolate_movable_page to let it return int as isolate_lru_page
do in this patchset :)

Thanks
Yisheng Xie

Re: [RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages

[Yisheng Xie]

hi Michal,
Thank you for reviewing and sorry for late reply.

On 01/26/2017 05:43 PM, Michal Hocko wrote:
> On Wed 25-01-17 14:59:45, Yisheng Xie wrote:
>
>  static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  {
> @@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  					pfn = round_up(pfn + 1,
>  						1 << compound_order(page)) - 1;
>  			}
> +			/*
> +			 * check __PageMovable in lock_page to avoid miss some
> +			 * non-lru movable pages at race condition.
> +			 */
> +			lock_page(page);
> +			if (__PageMovable(page)) {
> +				unlock_page(page);
> +				return pfn;
> +			}
> +			unlock_page(page);
> This doesn't make any sense to me. __PageMovable can change right after
> you drop the lock so why the race matters? If we cannot tolerate races
> then the above doesn't work and if we can then taking the lock is
> pointless.
hmm, for PageLRU check may also race without lru-locki 1/4 ?
I think it is ok to check __PageMovable without lock_page, here.

>>  		}
>>  	}
>>  	return 0;
>> @@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private,
>>  		if (!get_page_unless_zero(page))
>>  			continue;
>>  		/*
>> -		 * We can skip free pages. And we can only deal with pages on
>> -		 * LRU.
>> +		 * We can skip free pages. And we can deal with pages on
>> +		 * LRU and non-lru movable pages.
>>  		 */
>> -		ret = isolate_lru_page(page);
>> +		if (PageLRU(page))
>> +			ret = isolate_lru_page(page);
>> +		else
>> +			ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);
> we really want to propagate the proper error code to the caller.
Yes , I make the same mistake again. Really sorry about that.

Maybe I can rewrite the isolate_movable_page to let it return int as isolate_lru_page
do in this patchset :)

Thanks
Yisheng Xie

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