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 X-Spam-Level: X-Spam-Status: No, score=-9.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F40BBC433E0 for ; Fri, 5 Mar 2021 01:30:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8D42B65001 for ; Fri, 5 Mar 2021 01:30:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8D42B65001 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kingsoft.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C209B6B0005; Thu, 4 Mar 2021 20:30:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BAA946B0007; Thu, 4 Mar 2021 20:30:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9D49C6B0008; Thu, 4 Mar 2021 20:30:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0076.hostedemail.com [216.40.44.76]) by kanga.kvack.org (Postfix) with ESMTP id 774DE6B0005 for ; Thu, 4 Mar 2021 20:30:29 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 28AE6180ACC20 for ; Fri, 5 Mar 2021 01:30:29 +0000 (UTC) X-FDA: 77884090578.03.F8D9523 Received: from mail.kingsoft.com (unknown [114.255.44.146]) by imf16.hostedemail.com (Postfix) with ESMTP id C696A80192C0 for ; Fri, 5 Mar 2021 01:30:26 +0000 (UTC) X-AuditID: 0a580155-6fbff700000550c6-7f-6041825a337f Received: from mail.kingsoft.com (localhost [10.88.1.32]) (using TLS with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.kingsoft.com (SMG-2-NODE-85) with SMTP id 58.35.20678.A5281406; Fri, 5 Mar 2021 08:59:06 +0800 (HKT) Received: from alex-virtual-machine (172.16.253.254) by KSBJMAIL2.kingsoft.cn (10.88.1.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1979.3; Fri, 5 Mar 2021 09:30:21 +0800 Date: Fri, 5 Mar 2021 09:30:16 +0800 From: Aili Yao To: "Luck, Tony" CC: "HORIGUCHI =?UTF-8?B?TkFPWUE=?=(=?UTF-8?B?5aCA5Y+j44CA55u05Lmf?=)" , Oscar Salvador , "david@redhat.com" , "akpm@linux-foundation.org" , "bp@alien8.de" , "tglx@linutronix.de" , "mingo@redhat.com" , "hpa@zytor.com" , "x86@kernel.org" , "linux-edac@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "yangfeng1@kingsoft.com" , Subject: Re: [PATCH] mm,hwpoison: return -EBUSY when page already poisoned Message-ID: <20210305093016.40c87375@alex-virtual-machine> In-Reply-To: <20210304235720.GA215567@agluck-desk2.amr.corp.intel.com> References: <20210225181542.GA178925@agluck-desk2.amr.corp.intel.com> <20210226021907.GA27861@hori.linux.bs1.fc.nec.co.jp> <20210226105915.6cf7d2b8@alex-virtual-machine> <20210303033953.GA205389@agluck-desk2.amr.corp.intel.com> <20210303115710.2e9f8e23@alex-virtual-machine> <20210303163912.3d508e0f@alex-virtual-machine> <1a78e9abdc134e35a5efcbf6b2fd2263@intel.com> <20210304101653.546a9da1@alex-virtual-machine> <20210304121941.667047c3@alex-virtual-machine> <20210304144524.795872d7@alex-virtual-machine> <20210304235720.GA215567@agluck-desk2.amr.corp.intel.com> Organization: kingsoft X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [172.16.253.254] X-ClientProxiedBy: KSBJMAIL1.kingsoft.cn (10.88.1.31) To KSBJMAIL2.kingsoft.cn (10.88.1.32) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprNIsWRmVeSWpSXmKPExsXCFcGooBvV5Jhg8G6mucWc9WvYLD5v+Mdm 8XX9L2aLaRvFLS6camCyuLxrDpvFvTX/WS0uHVjAZHGx8QCjxZlpRRabN01ltnhz4R6LxY8N j1kdeD2+t/axeCze85LJY9OqTjaPTZ8msXu8O3eO3ePEjN8sHi+ubmTxeL/vKpvH5tPVHp83 yXmcaPnCGsAdxWWTkpqTWZZapG+XwJXx4eQq1oITsRWtXXdYGxg/enQxcnJICJhIXFuwn72L kYtDSGA6k8SGg53MEM5LRokPezeyglSxCKhIbH66jRHEZhNQldh1bxZYXERATeLS4gdgDcwC s1klTk0+ywySEBbwkvhyfy1QAwcHr4CVxONNDiBhTgE3icc3nrCB2EICJ1kk5u6vBLH5BcQk eq/8Z4K4yF6ibcsisF28AoISJ2c+YQGxmQV0JE6sOsYMYWtLLFv4mhlijqLE4SW/2CF6lSSO dM9gg7BjJZbNe8U6gVF4FpJRs5CMmoVk1AJG5lWMLMW56UabGCHxF7qDcUbTR71DjEwcjIcY JTiYlUR4xV/aJgjxpiRWVqUW5ccXleakFh9ilOZgURLnnbrVJEFIID2xJDU7NbUgtQgmy8TB KdXAtPhx/6kliiHpUTysGzY6zJbK8W9/xsF6padC/0u24fr3yefeVNrocfBynj0/7//meTX3 JdaX+P+/6n5TSG1nq6Yx78U6w+/BLdttNdR9o+/89fANELcv3tp+pe3uo7XfV9/53mlxSKvg 4ImF6qp3vwbzabJNT2Is7cpdp3D7wvOFk06ElK/Y5Nv+4v0Pi37nP3XMHXJ7Yxf/uFa2xqHd T4WHf3tF8oHdP21Wr414rbNzkrXwZx+PWZN6LB+nOtxcMFdIcZOufWMrK8uRTbVN7LMsHC7Z vXA7VrSG1byx1qGtOT/o7Wke9Z2RAZ/n8M0WUe1zSy9heVe/QaSe8Z/lw50qG+43Pt+62v6L bCWXEktxRqKhFnNRcSIAUreChy4DAAA= X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: C696A80192C0 X-Stat-Signature: zhpmjbgjpon9q6psez35px3fbhn5sjqt Received-SPF: none (kingsoft.com>: No applicable sender policy available) receiver=imf16; identity=mailfrom; envelope-from=""; helo=mail.kingsoft.com; client-ip=114.255.44.146 X-HE-DKIM-Result: none/none X-HE-Tag: 1614907826-736959 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 Thu, 4 Mar 2021 15:57:20 -0800 "Luck, Tony" wrote: > On Thu, Mar 04, 2021 at 02:45:24PM +0800, Aili Yao wrote: > > > > if your methods works, should it be like this? > > > >=20 > > > > 1582 pteval =3D swp_entry_to_pte(make_hwpoi= son_entry(subpage)); > > > > 1583 if (PageHuge(page)) { > > > > 1584 hugetlb_count_sub(compound_nr(= page), mm); > > > > 1585 set_huge_swap_pte_at(mm, addre= ss, > > > > 1586 pvmw.pte,= pteval, > > > > 1587 vma_mmu_p= agesize(vma)); > > > > 1588 } else { > > > > 1589 dec_mm_counter(mm, mm_counter(= page)); > > > > 1590 set_pte_at(mm, address, pvmw.p= te, pteval); > > > > 1591 } > > > >=20 > > > > the page fault check if it's a poison page using is_hwpoison_entry(= ), > > > > =20 > > >=20 > > > And if it works, does we need some locking mechanism before we call w= alk_page_range(); > > > if we lock, does we need to process the blocking interrupted error as= other places will do? > > > =20 > >=20 > > And another thing: > > Do we need a call to flush_tlb_page(vma, address) to make the pte chang= es into effect? =20 >=20 > Thanks for all the pointers. I added them to the patch (see below). > [The pmd/pud cases may need some tweaking ... but just trying to get > the 4K page case working first] >=20 > I tried testing by skipping the call to memory_failure() and just > using this new code to search the page tables for current page and > marking it hwpoison (to simulate the case where 2nd process gets the > early return from memory_failure(). Something is still missing because I = get: >=20 > [ 481.911298] mce: pte_entry: matched pfn - mark poison & zap pte > [ 481.917935] MCE: Killing einj_mem_uc:5555 due to hardware memory corru= ption fault at 7fe64b33b400 > [ 482.933775] BUG: Bad page cache in process einj_mem_uc pfn:408b6d6 > [ 482.940777] page:0000000013ea6e96 refcount:3 mapcount:1 mapping:000000= 00e3a069d9 index:0x0 pfn:0x408b6d6 > [ 482.951355] memcg:ffff94a809834000 > [ 482.955153] aops:shmem_aops ino:3c04 > [ 482.959142] flags: 0x97ffffc0880015(locked|uptodate|lru|swapbacked|hwp= oison) > [ 482.967018] raw: 0097ffffc0880015 ffff94c80e93ec00 ffff94c80e93ec00 ff= ff94c80a9b25a8 > [ 482.975666] raw: 0000000000000000 0000000000000000 0000000300000000 ff= ff94a809834000 > [ 482.984310] page dumped because: still mapped when deleted =46rom the walk, it seems we have got the virtual address, can we just send a= SIGBUS with it? > commit e5de44560b33e2d407704243566253a70f858a59 > Author: Tony Luck > Date: Tue Mar 2 15:06:33 2021 -0800 >=20 > x86/mce: Handle races between machine checks > =20 > When multiple CPUs hit the same poison memory there is a race. The > first CPU into memory_failure() atomically marks the page as poison > and continues processing to hunt down all the tasks that map this page > so that the virtual addresses can be marked not-present and SIGBUS > sent to the task that did the access. > =20 > Later CPUs get an early return from memory_failure() and may return > to user mode and access the poison again. > =20 > Add a new argument to memory_failure() so that it can indicate when > the race has been lost. Fix kill_me_maybe() to scan page tables in > this case to unmap pages. >=20 > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/cor= e.c > index 7962355436da..a52c6a772de2 100644 > --- a/arch/x86/kernel/cpu/mce/core.c > +++ b/arch/x86/kernel/cpu/mce/core.c > @@ -28,8 +28,12 @@ > #include > #include > #include > +#include > +#include > +#include > #include > #include > +#include > #include > #include > #include > @@ -637,6 +641,7 @@ static int uc_decode_notifier(struct notifier_block *= nb, unsigned long val, > { > struct mce *mce =3D (struct mce *)data; > unsigned long pfn; > + int already =3D 0; > =20 > if (!mce || !mce_usable_address(mce)) > return NOTIFY_DONE; > @@ -646,8 +651,9 @@ static int uc_decode_notifier(struct notifier_block *= nb, unsigned long val, > return NOTIFY_DONE; > =20 > pfn =3D mce->addr >> PAGE_SHIFT; > - if (!memory_failure(pfn, 0)) { > - set_mce_nospec(pfn, whole_page(mce)); > + if (!memory_failure(pfn, 0, &already)) { > + if (!already) > + set_mce_nospec(pfn, whole_page(mce)); > mce->kflags |=3D MCE_HANDLED_UC; > } > =20 > @@ -1248,6 +1254,79 @@ static void __mc_scan_banks(struct mce *m, struct = pt_regs *regs, struct mce *fin > *m =3D *final; > } > =20 > +static int pte_entry(pte_t *pte, unsigned long addr, unsigned long next,= struct mm_walk *walk) > +{ > + u64 pfn =3D (u64)walk->private; > + struct page *page; > + pte_t pteval; > + > + if (pte_pfn(*pte) =3D=3D pfn) { > +pr_info("pte_entry: matched pfn - mark poison & zap pte\n"); > + page =3D pfn_to_page(pfn); > + lock_page(page); > +SetPageHWPoison(page); > + pteval =3D swp_entry_to_pte(make_hwpoison_entry(page)); > + dec_mm_counter(walk->mm, mm_counter(page)); > + set_pte_at(current->mm, addr, pte, pteval); > + unlock_page(page); > + flush_tlb_page(walk->vma, addr); > + } > + > + return 0; > +} > + > +static int pmd_entry(pmd_t *pmd, unsigned long addr, unsigned long next,= struct mm_walk *walk) > +{ > + int shift =3D PMD_SHIFT - PAGE_SHIFT; > + u64 pfn =3D (u64)walk->private; > + struct page *page; > + pte_t pteval; > + > + if (!pmd_large(*pmd)) > + return 0; > + > + if (pmd_pfn(*pmd) >> shift =3D=3D pfn >> shift) { > + page =3D pfn_to_page(pfn); > + lock_page(page); > + pteval =3D swp_entry_to_pte(make_hwpoison_entry(page)); > + hugetlb_count_sub(compound_nr(page), walk->mm); > + set_huge_swap_pte_at(walk->mm, addr, (pte_t *)pmd, pteval, vma_mmu_pag= esize(walk->vma)); > + unlock_page(page); > + flush_tlb_page(walk->vma, addr); > + } > + > + return 0; > +} > + > +static int pud_entry(pud_t *pud, unsigned long addr, unsigned long next,= struct mm_walk *walk) > +{ > + int shift =3D PUD_SHIFT - PAGE_SHIFT; > + u64 pfn =3D (u64)walk->private; > + struct page *page; > + pte_t pteval; > + > + if (!pud_large(*pud)) > + return 0; > + > + if (pud_pfn(*pud) >> shift =3D=3D pfn >> shift) { > + page =3D pfn_to_page(pfn); > + lock_page(page); > + pteval =3D swp_entry_to_pte(make_hwpoison_entry(page)); > + hugetlb_count_sub(compound_nr(page), walk->mm); > + set_huge_swap_pte_at(walk->mm, addr, (pte_t *)pud, pteval, vma_mmu_pag= esize(walk->vma)); > + unlock_page(page); > + flush_tlb_page(walk->vma, addr); > + } > + > + return 0; > +} > + > +static struct mm_walk_ops walk =3D { > + .pte_entry =3D pte_entry, > + .pmd_entry =3D pmd_entry, > + .pud_entry =3D pud_entry > +}; > + > static void kill_me_now(struct callback_head *ch) > { > force_sig(SIGBUS); > @@ -1257,15 +1336,22 @@ static void kill_me_maybe(struct callback_head *c= b) > { > struct task_struct *p =3D container_of(cb, struct task_struct, mce_kill= _me); > int flags =3D MF_ACTION_REQUIRED; > + int already =3D 0; > =20 > pr_err("Uncorrected hardware memory error in user-access at %llx", p->m= ce_addr); > =20 > if (!p->mce_ripv) > flags |=3D MF_MUST_KILL; > =20 > - if (!memory_failure(p->mce_addr >> PAGE_SHIFT, flags) && > + if (!memory_failure(p->mce_addr >> PAGE_SHIFT, flags, &already) && > !(p->mce_kflags & MCE_IN_KERNEL_COPYIN)) { > - set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page); > + if (already) { > + mmap_read_lock(current->mm); > + walk_page_range(current->mm, 0, TASK_SIZE_MAX, &walk, (void *)(p->mce= _addr >> PAGE_SHIFT)); > + mmap_read_unlock(current->mm); > + } else { > + set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page); > + } > sync_core(); > return; > } > @@ -1452,7 +1538,7 @@ noinstr void do_machine_check(struct pt_regs *regs) > EXPORT_SYMBOL_GPL(do_machine_check); > =20 > #ifndef CONFIG_MEMORY_FAILURE > -int memory_failure(unsigned long pfn, int flags) > +int memory_failure(unsigned long pfn, int flags, int *already) > { > /* mce_severity() should not hand us an ACTION_REQUIRED error */ > BUG_ON(flags & MF_ACTION_REQUIRED); > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > index f35298425575..144500983656 100644 > --- a/drivers/base/memory.c > +++ b/drivers/base/memory.c > @@ -480,7 +480,7 @@ static ssize_t hard_offline_page_store(struct device = *dev, > if (kstrtoull(buf, 0, &pfn) < 0) > return -EINVAL; > pfn >>=3D PAGE_SHIFT; > - ret =3D memory_failure(pfn, 0); > + ret =3D memory_failure(pfn, 0, NULL); > return ret ? ret : count; > } > =20 > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 77e64e3eac80..beaa6e871cbe 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -3003,7 +3003,7 @@ enum mf_flags { > MF_MUST_KILL =3D 1 << 2, > MF_SOFT_OFFLINE =3D 1 << 3, > }; > -extern int memory_failure(unsigned long pfn, int flags); > +extern int memory_failure(unsigned long pfn, int flags, int *already); > extern void memory_failure_queue(unsigned long pfn, int flags); > extern void memory_failure_queue_kick(int cpu); > extern int unpoison_memory(unsigned long pfn); > diff --git a/mm/hwpoison-inject.c b/mm/hwpoison-inject.c > index 1ae1ebc2b9b1..bfd5151dcd3f 100644 > --- a/mm/hwpoison-inject.c > +++ b/mm/hwpoison-inject.c > @@ -48,7 +48,7 @@ static int hwpoison_inject(void *data, u64 val) > =20 > inject: > pr_info("Injecting memory failure at pfn %#lx\n", pfn); > - return memory_failure(pfn, 0); > + return memory_failure(pfn, 0, NULL); > } > =20 > static int hwpoison_unpoison(void *data, u64 val) > diff --git a/mm/madvise.c b/mm/madvise.c > index df692d2e35d4..09f569fed68d 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -908,7 +908,7 @@ static int madvise_inject_error(int behavior, > } else { > pr_info("Injecting memory failure for pfn %#lx at process virtual add= ress %#lx\n", > pfn, start); > - ret =3D memory_failure(pfn, MF_COUNT_INCREASED); > + ret =3D memory_failure(pfn, MF_COUNT_INCREASED, NULL); > } > =20 > if (ret) > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 24210c9bd843..9a8911aa5fc9 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1398,7 +1398,7 @@ static int memory_failure_dev_pagemap(unsigned long= pfn, int flags, > * Must run in process context (e.g. a work queue) with interrupts > * enabled and no spinlocks hold. > */ > -int memory_failure(unsigned long pfn, int flags) > +int memory_failure(unsigned long pfn, int flags, int *already) > { > struct page *p; > struct page *hpage; > @@ -1428,6 +1428,8 @@ int memory_failure(unsigned long pfn, int flags) > if (PageHuge(p)) > return memory_failure_hugetlb(pfn, flags); > if (TestSetPageHWPoison(p)) { > + if (already) > + *already =3D 1; > pr_err("Memory failure: %#lx: already hardware poisoned\n", > pfn); > return 0; > @@ -1634,7 +1636,7 @@ static void memory_failure_work_func(struct work_st= ruct *work) > if (entry.flags & MF_SOFT_OFFLINE) > soft_offline_page(entry.pfn, entry.flags); > else > - memory_failure(entry.pfn, entry.flags); > + memory_failure(entry.pfn, entry.flags, NULL); > } > } > =20 --=20 Thanks! Aili Yao