From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 340E0C433EF for ; Fri, 27 May 2022 20:21:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BAD318D0003; Fri, 27 May 2022 16:21:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B5C788D0002; Fri, 27 May 2022 16:21:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A22E58D0003; Fri, 27 May 2022 16:21:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 954C28D0002 for ; Fri, 27 May 2022 16:21:46 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 6B0F735CF8 for ; Fri, 27 May 2022 20:21:46 +0000 (UTC) X-FDA: 79512643812.19.93A7C6C Received: from mail-pg1-f169.google.com (mail-pg1-f169.google.com [209.85.215.169]) by imf29.hostedemail.com (Postfix) with ESMTP id 4419112003E for ; Fri, 27 May 2022 20:21:34 +0000 (UTC) Received: by mail-pg1-f169.google.com with SMTP id j21so4784829pga.13 for ; Fri, 27 May 2022 13:21:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QhIbxKNU3kNqyxNYcZ0IFxXWlA6XuN2QWsdqs0Ijyh0=; b=Pj2AQ/1f2fkmKQvjwuVmWsQ1rtDjVD05aZmeIBBhPaHuY115oVH/bqzf+9KJG33zJS G4DN3Lk0OWAw97yz5vwxjDKTey7It5Bf97CxNoRtgDeHGpxWJMbc1AFIWqI5NNN8kLaG om6L2tOvJ+g8AtWl7EYc0ER9J6eiyiXCDNTiut3FHnFvk3Dm3J6Jxkf21COOVLjx+ACS 1o+xrEdjNjk9tGQfP/PXUFH9gmzDvhml6ZOJlpNPh94CcrSId0fTf6/rEstQO7HNPz9t 9yjmwslyqY/YNrO5I6uns7isSJ0CZMOlmnQmA+lvcdQbzLZ22NwN4e1dYDYFVGA0FOkB j3Pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QhIbxKNU3kNqyxNYcZ0IFxXWlA6XuN2QWsdqs0Ijyh0=; b=yvExB8wxmJcWUt3VDcOuhIh0DyW9eZyVT3bNBC1DRKJB4rudd/ihovpSg47eMscG7O EHaxcXWPjleeSdpr/PDj0i0ebeY+j3abJJoxr+vz4xkX+J6fTMLU01c1EuP8u+h9tTRB pBxw74ptIAh3cjuimBO8QExr4pexzQteWtvJeKXO3+i0ATR9Nw87LHnZSDFTv0L0l9Lr MeqNFdETPQTcOd8ccrn6/OBNywMRVC4QsOG6MCe8XdKGyrZEHX6YNFY5lJf5UhyyIvzT GsmnABr5U/atl2mk0TjDh/igc9gbsFIPWFmY87deCw2CRR6MxwOQQ1/xZ+S9xWY/kfBO LaVA== X-Gm-Message-State: AOAM5306HkveUpdra+1C21vOnGtoKcSQoqe8Cq7PuLBl02NkUiSIPdS/ 60R4pGzJj/vLDfTGV6LRHDYhFUzm3S2lt9LXZcI= X-Google-Smtp-Source: ABdhPJyjnh/O+oeckZWgq3nPDx00PrbO8GVPLHeKO45OY6WOZ6yvMgpDKPApY4Mmif9NRCV4qTUEuPjoPfUz1Pok4Nk= X-Received: by 2002:a63:384c:0:b0:3f5:cc47:8a40 with SMTP id h12-20020a63384c000000b003f5cc478a40mr37852556pgn.587.1653682904773; Fri, 27 May 2022 13:21:44 -0700 (PDT) MIME-Version: 1.0 References: <20220527190731.322722-1-jiaqiyan@google.com> <20220527190731.322722-2-jiaqiyan@google.com> In-Reply-To: <20220527190731.322722-2-jiaqiyan@google.com> From: Yang Shi Date: Fri, 27 May 2022 13:21:32 -0700 Message-ID: Subject: Re: [PATCH v4 1/2] mm: khugepaged: recover from poisoned anonymous memory To: Jiaqi Yan Cc: Tong Tiangen , Tony Luck , =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= , "Kirill A. Shutemov" , Miaohe Lin , Jue Wang , Linux MM Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 4419112003E X-Stat-Signature: 4jk9jb5tuf4nzs3ziczbg8zehiysysxq X-Rspam-User: Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="Pj2AQ/1f"; spf=pass (imf29.hostedemail.com: domain of shy828301@gmail.com designates 209.85.215.169 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1653682894-392476 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, May 27, 2022 at 12:07 PM Jiaqi Yan wrote: > > Make __collapse_huge_page_copy return whether > collapsing/copying anonymous pages succeeded, > and make collapse_huge_page handle the return status. > > Break existing PTE scan loop into two for-loops. > The first loop copies source pages into target huge page, > and can fail gracefully when running into memory errors in > source pages. If copying all pages succeeds, the second loop > releases and clears up these normal pages. > Otherwise, the second loop does the following to > roll back the page table and page states: > 1) re-establish the original PTEs-to-PMD connection. > 2) release source pages back to their LRU list. > > Signed-off-by: Jiaqi Yan Thanks for your patience. Reviewed-by: Yang Shi > --- > include/linux/highmem.h | 19 +++++ > include/trace/events/huge_memory.h | 3 +- > mm/khugepaged.c | 130 ++++++++++++++++++++++------- > 3 files changed, 121 insertions(+), 31 deletions(-) > > diff --git a/include/linux/highmem.h b/include/linux/highmem.h > index 39bb9b47fa9cd..0ccb1e92c4b06 100644 > --- a/include/linux/highmem.h > +++ b/include/linux/highmem.h > @@ -298,6 +298,25 @@ static inline void copy_highpage(struct page *to, struct page *from) > > #endif > > +/* > + * Machine check exception handled version of copy_highpage. > + * Return true if copying page content failed; otherwise false. > + * Note handling #MC requires arch opt-in. > + */ > +static inline bool copy_highpage_mc(struct page *to, struct page *from) > +{ > + char *vfrom, *vto; > + unsigned long ret; > + > + vfrom = kmap_local_page(from); > + vto = kmap_local_page(to); > + ret = copy_mc_to_kernel(vto, vfrom, PAGE_SIZE); > + kunmap_local(vto); > + kunmap_local(vfrom); > + > + return ret > 0; > +} > + > static inline void memcpy_page(struct page *dst_page, size_t dst_off, > struct page *src_page, size_t src_off, > size_t len) > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h > index 4fdb14a81108b..f08687046ce41 100644 > --- a/include/trace/events/huge_memory.h > +++ b/include/trace/events/huge_memory.h > @@ -34,7 +34,8 @@ > EM( SCAN_ALLOC_HUGE_PAGE_FAIL, "alloc_huge_page_failed") \ > EM( SCAN_CGROUP_CHARGE_FAIL, "ccgroup_charge_failed") \ > EM( SCAN_TRUNCATED, "truncated") \ > - EMe(SCAN_PAGE_HAS_PRIVATE, "page_has_private") \ > + EM( SCAN_PAGE_HAS_PRIVATE, "page_has_private") \ > + EMe(SCAN_COPY_MC, "copy_poisoned_page") \ > > #undef EM > #undef EMe > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 131492fd1148b..0dd28ecc915d1 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -52,6 +52,7 @@ enum scan_result { > SCAN_CGROUP_CHARGE_FAIL, > SCAN_TRUNCATED, > SCAN_PAGE_HAS_PRIVATE, > + SCAN_COPY_MC, > }; > > #define CREATE_TRACE_POINTS > @@ -739,44 +740,99 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma, > return 0; > } > > -static void __collapse_huge_page_copy(pte_t *pte, struct page *page, > - struct vm_area_struct *vma, > - unsigned long address, > - spinlock_t *ptl, > - struct list_head *compound_pagelist) > +/* > + * __collapse_huge_page_copy - attempts to copy memory contents from normal > + * pages to a hugepage. Cleanup the normal pages if copying succeeds; > + * otherwise restore the original page table and release isolated normal pages. > + * Returns true if copying succeeds, otherwise returns false. > + * > + * @pte: starting of the PTEs to copy from > + * @page: the new hugepage to copy contents to > + * @pmd: pointer to the new hugepage's PMD > + * @rollback: the original normal pages' PMD > + * @address: starting address to copy > + * @pte_ptl: lock on normal pages' PTEs > + * @compound_pagelist: list that stores compound pages > + */ > +static bool __collapse_huge_page_copy(pte_t *pte, > + struct page *page, > + pmd_t *pmd, > + pmd_t rollback, > + struct vm_area_struct *vma, > + unsigned long address, > + spinlock_t *pte_ptl, > + struct list_head *compound_pagelist) > { > struct page *src_page, *tmp; > pte_t *_pte; > - for (_pte = pte; _pte < pte + HPAGE_PMD_NR; > - _pte++, page++, address += PAGE_SIZE) { > - pte_t pteval = *_pte; > + pte_t pteval; > + unsigned long _address; > + spinlock_t *pmd_ptl; > + bool copy_succeeded = true; > > - if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) { > + /* > + * Copying pages' contents is subject to memory poison at any iteration. > + */ > + for (_pte = pte, _address = address; > + _pte < pte + HPAGE_PMD_NR; > + _pte++, page++, _address += PAGE_SIZE) { > + pteval = *_pte; > + > + if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) > clear_user_highpage(page, address); > - add_mm_counter(vma->vm_mm, MM_ANONPAGES, 1); > - if (is_zero_pfn(pte_pfn(pteval))) { > - /* > - * ptl mostly unnecessary. > - */ > - spin_lock(ptl); > - ptep_clear(vma->vm_mm, address, _pte); > - spin_unlock(ptl); > + else { > + src_page = pte_page(pteval); > + if (copy_highpage_mc(page, src_page)) { > + copy_succeeded = false; > + break; > + } > + } > + } > + > + if (!copy_succeeded) { > + /* > + * Copying failed, re-establish the regular PMD that points to > + * the regular page table. Restoring PMD needs to be done prior > + * to releasing pages. Since pages are still isolated and locked > + * here, acquiring anon_vma_lock_write is unnecessary. > + */ > + pmd_ptl = pmd_lock(vma->vm_mm, pmd); > + pmd_populate(vma->vm_mm, pmd, pmd_pgtable(rollback)); > + spin_unlock(pmd_ptl); > + } > + > + for (_pte = pte, _address = address; _pte < pte + HPAGE_PMD_NR; > + _pte++, _address += PAGE_SIZE) { > + pteval = *_pte; > + if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) { > + if (copy_succeeded) { > + add_mm_counter(vma->vm_mm, MM_ANONPAGES, 1); > + if (is_zero_pfn(pte_pfn(pteval))) { > + /* > + * ptl mostly unnecessary. > + */ > + spin_lock(pte_ptl); > + pte_clear(vma->vm_mm, _address, _pte); > + spin_unlock(pte_ptl); > + } > } > } else { > src_page = pte_page(pteval); > - copy_user_highpage(page, src_page, address, vma); > if (!PageCompound(src_page)) > release_pte_page(src_page); > - /* > - * ptl mostly unnecessary, but preempt has to > - * be disabled to update the per-cpu stats > - * inside page_remove_rmap(). > - */ > - spin_lock(ptl); > - ptep_clear(vma->vm_mm, address, _pte); > - page_remove_rmap(src_page, false); > - spin_unlock(ptl); > - free_page_and_swap_cache(src_page); > + > + if (copy_succeeded) { > + /* > + * ptl mostly unnecessary, but preempt has to > + * be disabled to update the per-cpu stats > + * inside page_remove_rmap(). > + */ > + spin_lock(pte_ptl); > + pte_clear(vma->vm_mm, _address, _pte); > + page_remove_rmap(src_page, false); > + spin_unlock(pte_ptl); > + free_page_and_swap_cache(src_page); > + } > } > } > > @@ -784,6 +840,8 @@ static void __collapse_huge_page_copy(pte_t *pte, struct page *page, > list_del(&src_page->lru); > release_pte_page(src_page); > } > + > + return copy_succeeded; > } > > static void khugepaged_alloc_sleep(void) > @@ -1066,6 +1124,7 @@ static void collapse_huge_page(struct mm_struct *mm, > struct vm_area_struct *vma; > struct mmu_notifier_range range; > gfp_t gfp; > + bool copied = false; > > VM_BUG_ON(address & ~HPAGE_PMD_MASK); > > @@ -1177,9 +1236,13 @@ static void collapse_huge_page(struct mm_struct *mm, > */ > anon_vma_unlock_write(vma->anon_vma); > > - __collapse_huge_page_copy(pte, new_page, vma, address, pte_ptl, > - &compound_pagelist); > + copied = __collapse_huge_page_copy(pte, new_page, pmd, _pmd, > + vma, address, pte_ptl, &compound_pagelist); > pte_unmap(pte); > + if (!copied) { > + result = SCAN_COPY_MC; > + goto out_up_write; > + } > /* > * spin_lock() below is not the equivalent of smp_wmb(), but > * the smp_wmb() inside __SetPageUptodate() can be reused to > @@ -2168,6 +2231,13 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, > khugepaged_scan_file(mm, file, pgoff, hpage); > fput(file); > } else { > + /* > + * mmap_read_lock is > + * 1) still held if scan failed; > + * 2) released if scan succeeded. > + * It is not affected by collapsing or copying > + * operations. > + */ > ret = khugepaged_scan_pmd(mm, vma, > khugepaged_scan.address, > hpage); > -- > 2.36.1.124.g0e6072fb45-goog >