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 <osalva...@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-horigu...@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
>
>
