[RFC,v2,09/16] mm,hwpoison: Unify THP handling for hard and soft offline
diff mbox series

Message ID 20191017142123.24245-10-osalvador@suse.de
State New
Headers show
Series
  • Hwpoison rework {hard,soft}-offline
Related show

Commit Message

Oscar Salvador Oct. 17, 2019, 2:21 p.m. UTC
Place the THP's page handling in a helper and use it
from both hard and soft-offline machinery, so we get rid
of some duplicated code.

Signed-off-by: Oscar Salvador <osalvador@suse.de>
---
 mm/memory-failure.c | 48 ++++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

Comments

Naoya Horiguchi Oct. 21, 2019, 7:04 a.m. UTC | #1
On Thu, Oct 17, 2019 at 04:21:16PM +0200, Oscar Salvador wrote:
> Place the THP's page handling in a helper and use it
> from both hard and soft-offline machinery, so we get rid
> of some duplicated code.
> 
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> ---
>  mm/memory-failure.c | 48 ++++++++++++++++++++++--------------------------
>  1 file changed, 22 insertions(+), 26 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 836d671fb74f..37b230b8cfe7 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1066,6 +1066,25 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>  	return page_action(ps, p, pfn);
>  }
>  
> +static int try_to_split_thp_page(struct page *page, const char *msg)
> +{
> +	lock_page(page);
> +	if (!PageAnon(page) || unlikely(split_huge_page(page))) {
> +		unsigned long pfn = page_to_pfn(page);
> +
> +		unlock_page(page);
> +		if (!PageAnon(page))
> +			pr_info("%s: %#lx: non anonymous thp\n", msg, pfn);
> +		else
> +			pr_info("%s: %#lx: thp split failed\n", msg, pfn);
> +		put_page(page);
> +		return -EBUSY;
> +	}
> +	unlock_page(page);
> +
> +	return 0;
> +}
> +
>  static int memory_failure_hugetlb(unsigned long pfn, int flags)
>  {
>  	struct page *p = pfn_to_page(pfn);
> @@ -1288,21 +1307,8 @@ int memory_failure(unsigned long pfn, int flags)
>  	}
>  
>  	if (PageTransHuge(hpage)) {
> -		lock_page(p);
> -		if (!PageAnon(p) || unlikely(split_huge_page(p))) {
> -			unlock_page(p);
> -			if (!PageAnon(p))
> -				pr_err("Memory failure: %#lx: non anonymous thp\n",
> -					pfn);
> -			else
> -				pr_err("Memory failure: %#lx: thp split failed\n",
> -					pfn);
> -			if (TestClearPageHWPoison(p))
> -				num_poisoned_pages_dec();
> -			put_page(p);
> +		if (try_to_split_thp_page(p, "Memory Failure") < 0)
>  			return -EBUSY;

Although this is not a cleanup thing, this failure path means that
hwpoison is handled (PG_hwpoison is marked), so action_result() should
be called.  I'll add a patch for this later.

Anyway, this cleanup patch looks fine to me.

Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

> -		}
> -		unlock_page(p);
>  		VM_BUG_ON_PAGE(!page_count(p), p);
>  		hpage = compound_head(p);
>  	}
> @@ -1801,19 +1807,9 @@ static int soft_offline_in_use_page(struct page *page)
>  	int mt;
>  	struct page *hpage = compound_head(page);
>  
> -	if (!PageHuge(page) && PageTransHuge(hpage)) {
> -		lock_page(page);
> -		if (!PageAnon(page) || unlikely(split_huge_page(page))) {
> -			unlock_page(page);
> -			if (!PageAnon(page))
> -				pr_info("soft offline: %#lx: non anonymous thp\n", page_to_pfn(page));
> -			else
> -				pr_info("soft offline: %#lx: thp split failed\n", page_to_pfn(page));
> -			put_page(page);
> +	if (!PageHuge(page) && PageTransHuge(hpage))
> +		if (try_to_split_thp_page(page, "soft offline") < 0)
>  			return -EBUSY;
> -		}
> -		unlock_page(page);
> -	}
>  
>  	/*
>  	 * Setting MIGRATE_ISOLATE here ensures that the page will be linked
> -- 
> 2.12.3
> 
>

Patch
diff mbox series

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 836d671fb74f..37b230b8cfe7 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1066,6 +1066,25 @@  static int identify_page_state(unsigned long pfn, struct page *p,
 	return page_action(ps, p, pfn);
 }
 
+static int try_to_split_thp_page(struct page *page, const char *msg)
+{
+	lock_page(page);
+	if (!PageAnon(page) || unlikely(split_huge_page(page))) {
+		unsigned long pfn = page_to_pfn(page);
+
+		unlock_page(page);
+		if (!PageAnon(page))
+			pr_info("%s: %#lx: non anonymous thp\n", msg, pfn);
+		else
+			pr_info("%s: %#lx: thp split failed\n", msg, pfn);
+		put_page(page);
+		return -EBUSY;
+	}
+	unlock_page(page);
+
+	return 0;
+}
+
 static int memory_failure_hugetlb(unsigned long pfn, int flags)
 {
 	struct page *p = pfn_to_page(pfn);
@@ -1288,21 +1307,8 @@  int memory_failure(unsigned long pfn, int flags)
 	}
 
 	if (PageTransHuge(hpage)) {
-		lock_page(p);
-		if (!PageAnon(p) || unlikely(split_huge_page(p))) {
-			unlock_page(p);
-			if (!PageAnon(p))
-				pr_err("Memory failure: %#lx: non anonymous thp\n",
-					pfn);
-			else
-				pr_err("Memory failure: %#lx: thp split failed\n",
-					pfn);
-			if (TestClearPageHWPoison(p))
-				num_poisoned_pages_dec();
-			put_page(p);
+		if (try_to_split_thp_page(p, "Memory Failure") < 0)
 			return -EBUSY;
-		}
-		unlock_page(p);
 		VM_BUG_ON_PAGE(!page_count(p), p);
 		hpage = compound_head(p);
 	}
@@ -1801,19 +1807,9 @@  static int soft_offline_in_use_page(struct page *page)
 	int mt;
 	struct page *hpage = compound_head(page);
 
-	if (!PageHuge(page) && PageTransHuge(hpage)) {
-		lock_page(page);
-		if (!PageAnon(page) || unlikely(split_huge_page(page))) {
-			unlock_page(page);
-			if (!PageAnon(page))
-				pr_info("soft offline: %#lx: non anonymous thp\n", page_to_pfn(page));
-			else
-				pr_info("soft offline: %#lx: thp split failed\n", page_to_pfn(page));
-			put_page(page);
+	if (!PageHuge(page) && PageTransHuge(hpage))
+		if (try_to_split_thp_page(page, "soft offline") < 0)
 			return -EBUSY;
-		}
-		unlock_page(page);
-	}
 
 	/*
 	 * Setting MIGRATE_ISOLATE here ensures that the page will be linked