[PATCH v4 0/2] HWPOISON: soft offlining for non-lru movable page

[PATCH v4 0/2] HWPOISON: soft offlining for non-lru movable page

[ysxie]

From: Yisheng Xie <xieyisheng1@huawei.com>

Hi Andrew,
Could you please help to abandon the v3 of this patch for it will compile
error with CONFIG_MIGRATION=n, and it also has error path handling problem.

Hi Michal, Minchan and all,
Could you please help to review it? 

Any suggestion is more than welcome.

The aim of this patchset is to support soft offlining of movable no-lru pages,
which already support migration after Minchan's commit bda807d44454 ("mm: migrate:
support non-lru movable page migration"). That means this patch heavily depend
on non-lru movable page migration.

So when memory corrected errors occur on a non-lru movable page, we can stop
to use it by migrating data onto another page and disable the original (maybe
half-broken) one.

--------
v4:
 * make isolate_movable_page always defined to avoid compile error with
   CONFIG_MIGRATION = n
 * return -EBUSY when isolate_movable_page return false which means failed
   to isolate movable page.

v3:
  * delete some unneed limitation and use !__PageMovable instead of PageLRU
    after isolate page to avoid isolated count mismatch, as Minchan Kim's suggestion.

v2:
 * delete function soft_offline_movable_page() and hanle non-lru movable
   page in __soft_offline_page() as Michal Hocko suggested.

Yisheng Xie (2):
  mm/migration: make isolate_movable_page always defined
  HWPOISON: soft offlining for non-lru movable page

 include/linux/migrate.h |  2 ++
 mm/memory-failure.c     | 26 ++++++++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH v4 0/2] HWPOISON: soft offlining for non-lru movable page

[ysxie]

From: Yisheng Xie <xieyisheng1@huawei.com>

Hi Andrew,
Could you please help to abandon the v3 of this patch for it will compile
error with CONFIG_MIGRATION=n, and it also has error path handling problem.

Hi Michal, Minchan and all,
Could you please help to review it? 

Any suggestion is more than welcome.

The aim of this patchset is to support soft offlining of movable no-lru pages,
which already support migration after Minchan's commit bda807d44454 ("mm: migrate:
support non-lru movable page migration"). That means this patch heavily depend
on non-lru movable page migration.

So when memory corrected errors occur on a non-lru movable page, we can stop
to use it by migrating data onto another page and disable the original (maybe
half-broken) one.

--------
v4:
 * make isolate_movable_page always defined to avoid compile error with
   CONFIG_MIGRATION = n
 * return -EBUSY when isolate_movable_page return false which means failed
   to isolate movable page.

v3:
  * delete some unneed limitation and use !__PageMovable instead of PageLRU
    after isolate page to avoid isolated count mismatch, as Minchan Kim's suggestion.

v2:
 * delete function soft_offline_movable_page() and hanle non-lru movable
   page in __soft_offline_page() as Michal Hocko suggested.

Yisheng Xie (2):
  mm/migration: make isolate_movable_page always defined
  HWPOISON: soft offlining for non-lru movable page

 include/linux/migrate.h |  2 ++
 mm/memory-failure.c     | 26 ++++++++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

-- 
1.9.1



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

[PATCH v4 1/2] mm/migration: make isolate_movable_page always defined

[ysxie]

From: Yisheng Xie <xieyisheng1@huawei.com>

Define isolate_movable_page as a static inline function when
CONFIG_MIGRATION is not enable. It should return false
here which means failed to isolate movable pages.

This patch do not have any functional change but prepare for
later patch.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
CC: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/migrate.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index ae8d475..631a8c8 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -56,6 +56,8 @@ static inline int migrate_pages(struct list_head *l, new_page_t new,
 		free_page_t free, unsigned long private, enum migrate_mode mode,
 		int reason)
 	{ return -ENOSYS; }
+static inline bool isolate_movable_page(struct page *page, isolate_mode_t mode)
+	{ return false; }
 
 static inline int migrate_prep(void) { return -ENOSYS; }
 static inline int migrate_prep_local(void) { return -ENOSYS; }
-- 
1.9.1


.

[PATCH v4 1/2] mm/migration: make isolate_movable_page always defined

[ysxie]

From: Yisheng Xie <xieyisheng1@huawei.com>

Define isolate_movable_page as a static inline function when
CONFIG_MIGRATION is not enable. It should return false
here which means failed to isolate movable pages.

This patch do not have any functional change but prepare for
later patch.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
CC: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/migrate.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index ae8d475..631a8c8 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -56,6 +56,8 @@ static inline int migrate_pages(struct list_head *l, new_page_t new,
 		free_page_t free, unsigned long private, enum migrate_mode mode,
 		int reason)
 	{ return -ENOSYS; }
+static inline bool isolate_movable_page(struct page *page, isolate_mode_t mode)
+	{ return false; }
 
 static inline int migrate_prep(void) { return -ENOSYS; }
 static inline int migrate_prep_local(void) { return -ENOSYS; }
-- 
1.9.1



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

[PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[ysxie]

From: Yisheng Xie <xieyisheng1@huawei.com>

This patch is to extends soft offlining framework to support
non-lru page, which already support migration after
commit bda807d44454 ("mm: migrate: support non-lru movable page
migration")

When memory corrected errors occur on a non-lru movable page,
we can choose to stop using it by migrating data onto another
page and disable the original (maybe half-broken) one.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Suggested-by: Michal Hocko <mhocko@kernel.org>
Suggested-by: Minchan Kim <minchan@kernel.org>
Reviewed-by: Minchan Kim <minchan@kernel.org>
Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
CC: Vlastimil Babka <vbabka@suse.cz>
---
 mm/memory-failure.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index f283c7e..56e39f8 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
 {
 	int ret = __get_any_page(page, pfn, flags);
 
-	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
+	if (ret == 1 && !PageHuge(page) &&
+	    !PageLRU(page) && !__PageMovable(page)) {
 		/*
 		 * Try to free it.
 		 */
@@ -1649,7 +1650,10 @@ static int __soft_offline_page(struct page *page, int flags)
 	 * Try to migrate to a new page instead. migrate.c
 	 * handles a large number of cases for us.
 	 */
-	ret = isolate_lru_page(page);
+	if (PageLRU(page))
+		ret = isolate_lru_page(page);
+	else if (!isolate_movable_page(page, ISOLATE_UNEVICTABLE))
+		ret = -EBUSY;
 	/*
 	 * Drop page reference which is came from get_any_page()
 	 * successful isolate_lru_page() already took another one.
@@ -1657,18 +1661,20 @@ static int __soft_offline_page(struct page *page, int flags)
 	put_hwpoison_page(page);
 	if (!ret) {
 		LIST_HEAD(pagelist);
-		inc_node_page_state(page, NR_ISOLATED_ANON +
-					page_is_file_cache(page));
+		/*
+		 * After isolated lru page, the PageLRU will be cleared,
+		 * so use !__PageMovable instead for LRU page's mapping
+		 * cannot have PAGE_MAPPING_MOVABLE.
+		 */
+		if (!__PageMovable(page))
+			inc_node_page_state(page, NR_ISOLATED_ANON +
+						page_is_file_cache(page));
 		list_add(&page->lru, &pagelist);
 		ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
 					MIGRATE_SYNC, MR_MEMORY_FAILURE);
 		if (ret) {
-			if (!list_empty(&pagelist)) {
-				list_del(&page->lru);
-				dec_node_page_state(page, NR_ISOLATED_ANON +
-						page_is_file_cache(page));
-				putback_lru_page(page);
-			}
+			if (!list_empty(&pagelist))
+				putback_movable_pages(&pagelist);
 
 			pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
 				pfn, ret, page->flags);
-- 
1.9.1

[PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[ysxie]

From: Yisheng Xie <xieyisheng1@huawei.com>

This patch is to extends soft offlining framework to support
non-lru page, which already support migration after
commit bda807d44454 ("mm: migrate: support non-lru movable page
migration")

When memory corrected errors occur on a non-lru movable page,
we can choose to stop using it by migrating data onto another
page and disable the original (maybe half-broken) one.

Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Suggested-by: Michal Hocko <mhocko@kernel.org>
Suggested-by: Minchan Kim <minchan@kernel.org>
Reviewed-by: Minchan Kim <minchan@kernel.org>
Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
CC: Vlastimil Babka <vbabka@suse.cz>
---
 mm/memory-failure.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index f283c7e..56e39f8 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
 {
 	int ret = __get_any_page(page, pfn, flags);
 
-	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
+	if (ret == 1 && !PageHuge(page) &&
+	    !PageLRU(page) && !__PageMovable(page)) {
 		/*
 		 * Try to free it.
 		 */
@@ -1649,7 +1650,10 @@ static int __soft_offline_page(struct page *page, int flags)
 	 * Try to migrate to a new page instead. migrate.c
 	 * handles a large number of cases for us.
 	 */
-	ret = isolate_lru_page(page);
+	if (PageLRU(page))
+		ret = isolate_lru_page(page);
+	else if (!isolate_movable_page(page, ISOLATE_UNEVICTABLE))
+		ret = -EBUSY;
 	/*
 	 * Drop page reference which is came from get_any_page()
 	 * successful isolate_lru_page() already took another one.
@@ -1657,18 +1661,20 @@ static int __soft_offline_page(struct page *page, int flags)
 	put_hwpoison_page(page);
 	if (!ret) {
 		LIST_HEAD(pagelist);
-		inc_node_page_state(page, NR_ISOLATED_ANON +
-					page_is_file_cache(page));
+		/*
+		 * After isolated lru page, the PageLRU will be cleared,
+		 * so use !__PageMovable instead for LRU page's mapping
+		 * cannot have PAGE_MAPPING_MOVABLE.
+		 */
+		if (!__PageMovable(page))
+			inc_node_page_state(page, NR_ISOLATED_ANON +
+						page_is_file_cache(page));
 		list_add(&page->lru, &pagelist);
 		ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
 					MIGRATE_SYNC, MR_MEMORY_FAILURE);
 		if (ret) {
-			if (!list_empty(&pagelist)) {
-				list_del(&page->lru);
-				dec_node_page_state(page, NR_ISOLATED_ANON +
-						page_is_file_cache(page));
-				putback_lru_page(page);
-			}
+			if (!list_empty(&pagelist))
+				putback_movable_pages(&pagelist);
 
 			pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
 				pfn, ret, page->flags);
-- 
1.9.1



--
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 v4 1/2] mm/migration: make isolate_movable_page always defined

[Michal Hocko]

On Wed 25-01-17 23:05:37, ysxie@foxmail.com wrote:
> From: Yisheng Xie <xieyisheng1@huawei.com>
> 
> Define isolate_movable_page as a static inline function when
> CONFIG_MIGRATION is not enable. It should return false
> here which means failed to isolate movable pages.
> 
> This patch do not have any functional change but prepare for
> later patch.

I think it would make more sense to make isolate_movable_page return int
and have the same semantic as __isolate_lru_page. This would be a better
preparatory patch for the later work.

> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> CC: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/migrate.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index ae8d475..631a8c8 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -56,6 +56,8 @@ static inline int migrate_pages(struct list_head *l, new_page_t new,
>  		free_page_t free, unsigned long private, enum migrate_mode mode,
>  		int reason)
>  	{ return -ENOSYS; }
> +static inline bool isolate_movable_page(struct page *page, isolate_mode_t mode)
> +	{ return false; }
>  
>  static inline int migrate_prep(void) { return -ENOSYS; }
>  static inline int migrate_prep_local(void) { return -ENOSYS; }
> -- 
> 1.9.1
> 
> 
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v4 1/2] mm/migration: make isolate_movable_page always defined

[Michal Hocko]

On Wed 25-01-17 23:05:37, ysxie@foxmail.com wrote:
> From: Yisheng Xie <xieyisheng1@huawei.com>
> 
> Define isolate_movable_page as a static inline function when
> CONFIG_MIGRATION is not enable. It should return false
> here which means failed to isolate movable pages.
> 
> This patch do not have any functional change but prepare for
> later patch.

I think it would make more sense to make isolate_movable_page return int
and have the same semantic as __isolate_lru_page. This would be a better
preparatory patch for the later work.

> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> CC: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/migrate.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index ae8d475..631a8c8 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -56,6 +56,8 @@ static inline int migrate_pages(struct list_head *l, new_page_t new,
>  		free_page_t free, unsigned long private, enum migrate_mode mode,
>  		int reason)
>  	{ return -ENOSYS; }
> +static inline bool isolate_movable_page(struct page *page, isolate_mode_t mode)
> +	{ return false; }
>  
>  static inline int migrate_prep(void) { return -ENOSYS; }
>  static inline int migrate_prep_local(void) { return -ENOSYS; }
> -- 
> 1.9.1
> 
> 
> 

-- 
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: [PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[Michal Hocko]

On Wed 25-01-17 23:05:38, ysxie@foxmail.com wrote:
> From: Yisheng Xie <xieyisheng1@huawei.com>
> 
> This patch is to extends soft offlining framework to support
> non-lru page, which already support migration after
> commit bda807d44454 ("mm: migrate: support non-lru movable page
> migration")
> 
> When memory corrected errors occur on a non-lru movable page,
> we can choose to stop using it by migrating data onto another
> page and disable the original (maybe half-broken) one.
> 
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> Suggested-by: Michal Hocko <mhocko@kernel.org>
> Suggested-by: Minchan Kim <minchan@kernel.org>
> Reviewed-by: Minchan Kim <minchan@kernel.org>
> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> CC: Vlastimil Babka <vbabka@suse.cz>
> ---
>  mm/memory-failure.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index f283c7e..56e39f8 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
>  {
>  	int ret = __get_any_page(page, pfn, flags);
>  
> -	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
> +	if (ret == 1 && !PageHuge(page) &&
> +	    !PageLRU(page) && !__PageMovable(page)) {
>  		/*
>  		 * Try to free it.
>  		 */

Is this sufficient? Not that I am familiar with get_any_page() but
__get_any_page doesn't seem to be aware of movable pages and neither
shake_page is.

> @@ -1649,7 +1650,10 @@ static int __soft_offline_page(struct page *page, int flags)
>  	 * Try to migrate to a new page instead. migrate.c
>  	 * handles a large number of cases for us.
>  	 */
> -	ret = isolate_lru_page(page);
> +	if (PageLRU(page))
> +		ret = isolate_lru_page(page);
> +	else if (!isolate_movable_page(page, ISOLATE_UNEVICTABLE))
> +		ret = -EBUSY;

As pointed out in the previous response isolate_movable_page should
really have the same return value contract as [__]isolate_lru_page

>  	/*
>  	 * Drop page reference which is came from get_any_page()
>  	 * successful isolate_lru_page() already took another one.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[Michal Hocko]

On Wed 25-01-17 23:05:38, ysxie@foxmail.com wrote:
> From: Yisheng Xie <xieyisheng1@huawei.com>
> 
> This patch is to extends soft offlining framework to support
> non-lru page, which already support migration after
> commit bda807d44454 ("mm: migrate: support non-lru movable page
> migration")
> 
> When memory corrected errors occur on a non-lru movable page,
> we can choose to stop using it by migrating data onto another
> page and disable the original (maybe half-broken) one.
> 
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> Suggested-by: Michal Hocko <mhocko@kernel.org>
> Suggested-by: Minchan Kim <minchan@kernel.org>
> Reviewed-by: Minchan Kim <minchan@kernel.org>
> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> CC: Vlastimil Babka <vbabka@suse.cz>
> ---
>  mm/memory-failure.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index f283c7e..56e39f8 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
>  {
>  	int ret = __get_any_page(page, pfn, flags);
>  
> -	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
> +	if (ret == 1 && !PageHuge(page) &&
> +	    !PageLRU(page) && !__PageMovable(page)) {
>  		/*
>  		 * Try to free it.
>  		 */

Is this sufficient? Not that I am familiar with get_any_page() but
__get_any_page doesn't seem to be aware of movable pages and neither
shake_page is.

> @@ -1649,7 +1650,10 @@ static int __soft_offline_page(struct page *page, int flags)
>  	 * Try to migrate to a new page instead. migrate.c
>  	 * handles a large number of cases for us.
>  	 */
> -	ret = isolate_lru_page(page);
> +	if (PageLRU(page))
> +		ret = isolate_lru_page(page);
> +	else if (!isolate_movable_page(page, ISOLATE_UNEVICTABLE))
> +		ret = -EBUSY;

As pointed out in the previous response isolate_movable_page should
really have the same return value contract as [__]isolate_lru_page

>  	/*
>  	 * Drop page reference which is came from get_any_page()
>  	 * successful isolate_lru_page() already took another one.
-- 
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: [PATCH v4 1/2] mm/migration: make isolate_movable_page always defined

[Yisheng Xie]

Hi， Michal，
Sorry for late reply.

On 01/26/2017 05:18 PM, Michal Hocko wrote:
> On Wed 25-01-17 23:05:37, ysxie@foxmail.com wrote:
>> From: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> Define isolate_movable_page as a static inline function when
>> CONFIG_MIGRATION is not enable. It should return false
>> here which means failed to isolate movable pages.
>>
>> This patch do not have any functional change but prepare for
>> later patch.
> I think it would make more sense to make isolate_movable_page return int
> and have the same semantic as __isolate_lru_page. This would be a better
> preparatory patch for the later work.
Yes, I think you are right, it is better to make isolate_movable_page return int
just as what isolate_lru_page do, to make a better code style.

It seems Andrew had already merged the fixed patch from Arnd Bergmann,
Maybe I can rewrite it in a later patch if it is suitable :)

Re: [PATCH v4 1/2] mm/migration: make isolate_movable_page always defined

[Yisheng Xie]

Hii 1/4 ? Michali 1/4 ?
Sorry for late reply.

On 01/26/2017 05:18 PM, Michal Hocko wrote:
> On Wed 25-01-17 23:05:37, ysxie@foxmail.com wrote:
>> From: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> Define isolate_movable_page as a static inline function when
>> CONFIG_MIGRATION is not enable. It should return false
>> here which means failed to isolate movable pages.
>>
>> This patch do not have any functional change but prepare for
>> later patch.
> I think it would make more sense to make isolate_movable_page return int
> and have the same semantic as __isolate_lru_page. This would be a better
> preparatory patch for the later work.
Yes, I think you are right, it is better to make isolate_movable_page return int
just as what isolate_lru_page do, to make a better code style.

It seems Andrew had already merged the fixed patch from Arnd Bergmann,
Maybe I can rewrite it in a later patch if it is suitable :)


--
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 v4 2/2] HWPOISON: soft offlining for non-lru movable page

[Yisheng Xie]

Hi, Michal,
Sorry for late reply.

On 01/26/2017 05:27 PM, Michal Hocko wrote:
> On Wed 25-01-17 23:05:38, ysxie@foxmail.com wrote:
>> From: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> This patch is to extends soft offlining framework to support
>> non-lru page, which already support migration after
>> commit bda807d44454 ("mm: migrate: support non-lru movable page
>> migration")
>>
>> When memory corrected errors occur on a non-lru movable page,
>> we can choose to stop using it by migrating data onto another
>> page and disable the original (maybe half-broken) one.
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> Suggested-by: Michal Hocko <mhocko@kernel.org>
>> Suggested-by: Minchan Kim <minchan@kernel.org>
>> Reviewed-by: Minchan Kim <minchan@kernel.org>
>> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>> CC: Vlastimil Babka <vbabka@suse.cz>
>> ---
>>  mm/memory-failure.c | 26 ++++++++++++++++----------
>>  1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index f283c7e..56e39f8 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
>>  {
>>  	int ret = __get_any_page(page, pfn, flags);
>>  
>> -	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
>> +	if (ret == 1 && !PageHuge(page) &&
>> +	    !PageLRU(page) && !__PageMovable(page)) {
>>  		/*
>>  		 * Try to free it.
>>  		 */
> Is this sufficient? Not that I am familiar with get_any_page() but
> __get_any_page doesn't seem to be aware of movable pages and neither
> shake_page is.
Sorry,maybe I do not quite get what you mean.
 If the page can be migrated, it can skip "shake_page and __get_any_page once more" here,
though it is not a free page. right ?
Please let me know if I miss anything.

>> @@ -1649,7 +1650,10 @@ static int __soft_offline_page(struct page *page, int flags)
>>  	 * Try to migrate to a new page instead. migrate.c
>>  	 * handles a large number of cases for us.
>>  	 */
>> -	ret = isolate_lru_page(page);
>> +	if (PageLRU(page))
>> +		ret = isolate_lru_page(page);
>> +	else if (!isolate_movable_page(page, ISOLATE_UNEVICTABLE))
>> +		ret = -EBUSY;
> As pointed out in the previous response isolate_movable_page should
> really have the same return value contract as [__]isolate_lru_page
Yes, I agree with your suggestion. I will rewrite it in later patch if it is suitable.
as I mention before.

Thanks again for your reviewing.

Yisheng Xie
>>  	/*
>>  	 * Drop page reference which is came from get_any_page()
>>  	 * successful isolate_lru_page() already took another one.

Re: [PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[Yisheng Xie]

Hi, Michal,
Sorry for late reply.

On 01/26/2017 05:27 PM, Michal Hocko wrote:
> On Wed 25-01-17 23:05:38, ysxie@foxmail.com wrote:
>> From: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> This patch is to extends soft offlining framework to support
>> non-lru page, which already support migration after
>> commit bda807d44454 ("mm: migrate: support non-lru movable page
>> migration")
>>
>> When memory corrected errors occur on a non-lru movable page,
>> we can choose to stop using it by migrating data onto another
>> page and disable the original (maybe half-broken) one.
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> Suggested-by: Michal Hocko <mhocko@kernel.org>
>> Suggested-by: Minchan Kim <minchan@kernel.org>
>> Reviewed-by: Minchan Kim <minchan@kernel.org>
>> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>> CC: Vlastimil Babka <vbabka@suse.cz>
>> ---
>>  mm/memory-failure.c | 26 ++++++++++++++++----------
>>  1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index f283c7e..56e39f8 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
>>  {
>>  	int ret = __get_any_page(page, pfn, flags);
>>  
>> -	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
>> +	if (ret == 1 && !PageHuge(page) &&
>> +	    !PageLRU(page) && !__PageMovable(page)) {
>>  		/*
>>  		 * Try to free it.
>>  		 */
> Is this sufficient? Not that I am familiar with get_any_page() but
> __get_any_page doesn't seem to be aware of movable pages and neither
> shake_page is.
Sorry,maybe I do not quite get what you mean.
 If the page can be migrated, it can skip "shake_page and __get_any_page once more" here,
though it is not a free page. right ?
Please let me know if I miss anything.

>> @@ -1649,7 +1650,10 @@ static int __soft_offline_page(struct page *page, int flags)
>>  	 * Try to migrate to a new page instead. migrate.c
>>  	 * handles a large number of cases for us.
>>  	 */
>> -	ret = isolate_lru_page(page);
>> +	if (PageLRU(page))
>> +		ret = isolate_lru_page(page);
>> +	else if (!isolate_movable_page(page, ISOLATE_UNEVICTABLE))
>> +		ret = -EBUSY;
> As pointed out in the previous response isolate_movable_page should
> really have the same return value contract as [__]isolate_lru_page
Yes, I agree with your suggestion. I will rewrite it in later patch if it is suitable.
as I mention before.

Thanks again for your reviewing.

Yisheng Xie
>>  	/*
>>  	 * Drop page reference which is came from get_any_page()
>>  	 * successful isolate_lru_page() already took another one.



--
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 v4 1/2] mm/migration: make isolate_movable_page always defined

[Michal Hocko]

On Mon 30-01-17 22:59:52, Yisheng Xie wrote:
> Hi， Michal，
> Sorry for late reply.
> 
> On 01/26/2017 05:18 PM, Michal Hocko wrote:
> > On Wed 25-01-17 23:05:37, ysxie@foxmail.com wrote:
> >> From: Yisheng Xie <xieyisheng1@huawei.com>
> >>
> >> Define isolate_movable_page as a static inline function when
> >> CONFIG_MIGRATION is not enable. It should return false
> >> here which means failed to isolate movable pages.
> >>
> >> This patch do not have any functional change but prepare for
> >> later patch.
> > I think it would make more sense to make isolate_movable_page return int
> > and have the same semantic as __isolate_lru_page. This would be a better
> > preparatory patch for the later work.
> Yes, I think you are right, it is better to make isolate_movable_page return int
> just as what isolate_lru_page do, to make a better code style.
> 
> It seems Andrew had already merged the fixed patch from Arnd Bergmann,
> Maybe I can rewrite it in a later patch if it is suitable :)

I guess Andrew can just drop the current series with the folow up fixes
and wait for your newer version.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v4 1/2] mm/migration: make isolate_movable_page always defined

[Michal Hocko]

On Mon 30-01-17 22:59:52, Yisheng Xie wrote:
> Hii 1/4 ? Michali 1/4 ?
> Sorry for late reply.
> 
> On 01/26/2017 05:18 PM, Michal Hocko wrote:
> > On Wed 25-01-17 23:05:37, ysxie@foxmail.com wrote:
> >> From: Yisheng Xie <xieyisheng1@huawei.com>
> >>
> >> Define isolate_movable_page as a static inline function when
> >> CONFIG_MIGRATION is not enable. It should return false
> >> here which means failed to isolate movable pages.
> >>
> >> This patch do not have any functional change but prepare for
> >> later patch.
> > I think it would make more sense to make isolate_movable_page return int
> > and have the same semantic as __isolate_lru_page. This would be a better
> > preparatory patch for the later work.
> Yes, I think you are right, it is better to make isolate_movable_page return int
> just as what isolate_lru_page do, to make a better code style.
> 
> It seems Andrew had already merged the fixed patch from Arnd Bergmann,
> Maybe I can rewrite it in a later patch if it is suitable :)

I guess Andrew can just drop the current series with the folow up fixes
and wait for your newer version.
-- 
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: [PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[Michal Hocko]

On Mon 30-01-17 23:04:13, Yisheng Xie wrote:
> Hi, Michal,
> Sorry for late reply.
> 
> On 01/26/2017 05:27 PM, Michal Hocko wrote:
> > On Wed 25-01-17 23:05:38, ysxie@foxmail.com wrote:
> >> From: Yisheng Xie <xieyisheng1@huawei.com>
> >>
> >> This patch is to extends soft offlining framework to support
> >> non-lru page, which already support migration after
> >> commit bda807d44454 ("mm: migrate: support non-lru movable page
> >> migration")
> >>
> >> When memory corrected errors occur on a non-lru movable page,
> >> we can choose to stop using it by migrating data onto another
> >> page and disable the original (maybe half-broken) one.
> >>
> >> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> >> Suggested-by: Michal Hocko <mhocko@kernel.org>
> >> Suggested-by: Minchan Kim <minchan@kernel.org>
> >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> >> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> >> CC: Vlastimil Babka <vbabka@suse.cz>
> >> ---
> >>  mm/memory-failure.c | 26 ++++++++++++++++----------
> >>  1 file changed, 16 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >> index f283c7e..56e39f8 100644
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
> >>  {
> >>  	int ret = __get_any_page(page, pfn, flags);
> >>  
> >> -	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
> >> +	if (ret == 1 && !PageHuge(page) &&
> >> +	    !PageLRU(page) && !__PageMovable(page)) {
> >>  		/*
> >>  		 * Try to free it.
> >>  		 */
> > Is this sufficient? Not that I am familiar with get_any_page() but
> > __get_any_page doesn't seem to be aware of movable pages and neither
> > shake_page is.
> Sorry,maybe I do not quite get what you mean.
>  If the page can be migrated, it can skip "shake_page and __get_any_page once more" here,
> though it is not a free page. right ?
> Please let me know if I miss anything.

No, you are right, it is me who read the code incorrectly. Sorry about
the confusion.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v4 2/2] HWPOISON: soft offlining for non-lru movable page

[Michal Hocko]

On Mon 30-01-17 23:04:13, Yisheng Xie wrote:
> Hi, Michal,
> Sorry for late reply.
> 
> On 01/26/2017 05:27 PM, Michal Hocko wrote:
> > On Wed 25-01-17 23:05:38, ysxie@foxmail.com wrote:
> >> From: Yisheng Xie <xieyisheng1@huawei.com>
> >>
> >> This patch is to extends soft offlining framework to support
> >> non-lru page, which already support migration after
> >> commit bda807d44454 ("mm: migrate: support non-lru movable page
> >> migration")
> >>
> >> When memory corrected errors occur on a non-lru movable page,
> >> we can choose to stop using it by migrating data onto another
> >> page and disable the original (maybe half-broken) one.
> >>
> >> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> >> Suggested-by: Michal Hocko <mhocko@kernel.org>
> >> Suggested-by: Minchan Kim <minchan@kernel.org>
> >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> >> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> >> CC: Vlastimil Babka <vbabka@suse.cz>
> >> ---
> >>  mm/memory-failure.c | 26 ++++++++++++++++----------
> >>  1 file changed, 16 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >> index f283c7e..56e39f8 100644
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -1527,7 +1527,8 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
> >>  {
> >>  	int ret = __get_any_page(page, pfn, flags);
> >>  
> >> -	if (ret == 1 && !PageHuge(page) && !PageLRU(page)) {
> >> +	if (ret == 1 && !PageHuge(page) &&
> >> +	    !PageLRU(page) && !__PageMovable(page)) {
> >>  		/*
> >>  		 * Try to free it.
> >>  		 */
> > Is this sufficient? Not that I am familiar with get_any_page() but
> > __get_any_page doesn't seem to be aware of movable pages and neither
> > shake_page is.
> Sorry,maybe I do not quite get what you mean.
>  If the page can be migrated, it can skip "shake_page and __get_any_page once more" here,
> though it is not a free page. right ?
> Please let me know if I miss anything.

No, you are right, it is me who read the code incorrectly. Sorry about
the confusion.
-- 
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>