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 B7B56C5478C for ; Wed, 28 Feb 2024 01:55:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4D1816B0277; Tue, 27 Feb 2024 20:55:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 433E36B0279; Tue, 27 Feb 2024 20:55:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2AD3F6B027A; Tue, 27 Feb 2024 20:55:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 161856B0277 for ; Tue, 27 Feb 2024 20:55:34 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E8F8BC0E73 for ; Wed, 28 Feb 2024 01:55:33 +0000 (UTC) X-FDA: 81839545746.27.6F2B28C Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by imf15.hostedemail.com (Postfix) with ESMTP id 87702A000C for ; Wed, 28 Feb 2024 01:55:29 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf15.hostedemail.com: domain of mawupeng1@huawei.com designates 45.249.212.191 as permitted sender) smtp.mailfrom=mawupeng1@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709085331; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MvGCnPDDb0DujszQbO+GO7fd2nKg2QW0Sjqd41oT4rA=; b=OdP2sXkAOn/X35V859kk8gRm18bcgKTOeAzo4G4mjvMhNExPGTLTEZaBAtoZDnKOOD83Px wPL8IdAvkLd3s3RRbeozQjRx/hfVPImve6kT+tTyrV6CU2RqiUw7fGq0O/anY2juLYPNoe Dt1yumoMnGLdFH6ueF49zfiHNR7QTi4= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf15.hostedemail.com: domain of mawupeng1@huawei.com designates 45.249.212.191 as permitted sender) smtp.mailfrom=mawupeng1@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709085331; a=rsa-sha256; cv=none; b=6RXE4VKBTdV8vgE0pLyCYHkqLmzr1j/zOTbuu/aiFbHr2h+v8LaRxibfE2/0RS9fflb42h Z4JOtBVOi8Akxbl0FTnz3XYtqF7yh7it584XsCI/JKrZVHB5gRLbb3NjKEj7NMP/NT91BD fYr09+A5xUaVKu/fDfPJUfdi+JtZQQA= Received: from mail.maildlp.com (unknown [172.19.88.214]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4Tky6v1lrtz1h0vs; Wed, 28 Feb 2024 09:53:11 +0800 (CST) Received: from dggpemd200001.china.huawei.com (unknown [7.185.36.224]) by mail.maildlp.com (Postfix) with ESMTPS id B7CB81A016C; Wed, 28 Feb 2024 09:55:25 +0800 (CST) Received: from [10.174.178.120] (10.174.178.120) by dggpemd200001.china.huawei.com (7.185.36.224) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.28; Wed, 28 Feb 2024 09:55:24 +0800 Message-ID: Date: Wed, 28 Feb 2024 09:55:24 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird CC: , , , , , , , , , , Subject: Re: [Question] CoW on VM_PFNMAP vma during write fault To: , , , , , References: <20240227122814.3781907-1-mawupeng1@huawei.com> Content-Language: en-US From: mawupeng In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.178.120] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpemd200001.china.huawei.com (7.185.36.224) X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 87702A000C X-Stat-Signature: 33cqc31cusd4kq3nup1mgdmgk3k1fntz X-Rspam-User: X-HE-Tag: 1709085329-511667 X-HE-Meta: U2FsdGVkX1/rPX2FSuTQkiD0X3zKDVhTtH9REfZvd6U54W4khmH7q186aGqO+6S9QgDykNwXKP9K6vAX3HgOMlWMp6+wAiH9Xoo4QdwxQxlvTi0RofoFbxz3yL0/bMJNrBeEHDSp417JQsqMiy6alLNcZJXvvFiPlVNMv9MgHoMZXHgCgDSrex9LZKJfy3u3ekTSwOZSNEuJBW1RPHIf3t3mCEzXCQ1pgT7YfzXDMtnsxGHdqkTO0BMLKfus0i3xp7xirk3OXK88rfg4KzAtoivkOJzyKCeG8kSMaQo7pZBSUFsM2MrfajQqtZnBGIl0nmiYbV/JlyrF73P9oWrfCT8d2YXJfaBySCOl1Mw9u6V2NHQkvN8SvHm0sbc+sHVupn9DpBraLiY+GipUt0vLM+IfGNpzLpUy90dpBg+Vkaajy8c48f3LtcyOkHQfa4RTJf/7tT/+iYX8NGyqi96SbdR9lM9UtqQbIK+HdhVJuiNuimhiWtTeoFz/ikbMpkTSHSJSRxDZRzFN+SVCC5c5CPlbR0xY+42WkyvY3SWxJu2JtQYLJWQKvVQ+BGKJSqAm+h01lp5JF1hdaOksZ4c0Wz2o5vTunTEZKV7MU/2Zder5FjOPr+Du7BpVqIH+yKq5tgjIxU1j2vU4FQrmZ48KNtmWylNHucBWJpd1cCbr7UhaSG7LvkAAaQnAix6ni4sYLsYjLjKIkiNKTIQRwF1jx70ch9HLyFGNyRrPAQ1vxA36jqkvhnKg1ucuv0dmdXoFtH/wdJsjjdbj1blFGK8+4uKPq1kQwU24kT2MwpUb7cMCIjF287R5y7ciuh9gZTOn8g2TluYYsQfpTnyI4fREQEBZq743h4xa4Lu4bXXpUaFUy1Ca//VJ9+UslrykxDqJZKzNRHlxMukMrmw7tFqmpHZifi5qc4kOZkxG26flK/Fk9Q92fgFucL5weC5zJ2AElY0l5ouCGZYbNINKxKg 1q2EbFif 220OQetSDBTetjscO9c1lLG+eOALXAZPhaEaxKHSMi3ouZy4VOG/DTk1zP+iJNW4xSTL4+A6IExbhJumFSqmY5XTXXbCKroJkvraim04YHXiVGLA+vHjYIh2th1tKy+ztHjII0pMsz49dhqQHPRBpML2n4ECS+AEYJg9FcHPJavKKfXpppJ2Q6kkjcRJiXVbjkpS+Yg7lZpv4miW05vfuzvUr/g== 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: List-Subscribe: List-Unsubscribe: On 2024/2/27 21:15, David Hildenbrand wrote: > On 27.02.24 14:00, David Hildenbrand wrote: >> On 27.02.24 13:28, Wupeng Ma wrote: >>> We find that a warn will be produced during our test, the detail log is >>> shown in the end. >>> >>> The core problem of this warn is that the first pfn of this pfnmap vma is >>> cleared during memory-failure. Digging into the source we find that this >>> problem can be triggered as following: >>> >>> // mmap with MAP_PRIVATE and specific fd which hook mmap >>> mmap(MAP_PRIVATE, fd) >>>     __mmap_region >>>       remap_pfn_range >>>       // set vma with pfnmap and the prot of pte is read only >>>      >> >> Okay, so we get a MAP_PRIVATE VM_PFNMAP I assume. >> >> What fd is that exactly? Often, we disallow private mappings in the >> mmap() callback (for a good reason). just a device fd with device-specify mmap which use remap_pfn_range to assign memory. >> >>> // memset this memory with trigger fault >>> handle_mm_fault >>>     __handle_mm_fault >>>       handle_pte_fault >>>         // write fault and !pte_write(entry) >>>         do_wp_page >>>           wp_page_copy // this will alloc a new page with valid page struct >>>                        // for this pfnmap vma >> >> Here we replace the mapped PFNMAP thingy by a proper anon folio. My problem is can wen replace a pfn with fully functioned page for pfnmap vma? This is not MIXEDMAP vma. >> >>> >>> // inject a hwpoison to the first page of this vma >> >> I assume this is an anon folio? Yes. >> >>> madvise_inject_error >>>     memory_failure >>>       hwpoison_user_mappings >>>         try_to_unmap_one >>>           // mark this pte as invalid (hwpoison) >>>           mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, vma->vm_mm, >>>                   address, range.end); If we can replace the mapped PFNMAP thingy by a proper anon folio, we need to make memory_failure to handle pfnmap vma properly since pfnmap vma shoule not touch its struct page? Current this page have a valid mapping and can be unmap. Maybe there is something wrong with my understanding of CoW on a private pfnmap vma. >>> >>> // during unmap vma, the first pfn of this pfnmap vma is invalid >>> vm_mmap_pgoff >>>     do_mmap >>>       __do_mmap_mm >>>         __mmap_region >>>           __do_munmap >>>             unmap_region >>>               unmap_vmas >>>                 unmap_single_vma >>>                   untrack_pfn >>>                     follow_phys // pte is already invalidate, WARN_ON here >> >> unmap_single_vma()->...->zap_pte_range() should do the right thing when >> calling vm_normal_page(). >> >> untrack_pfn() is the problematic part. For pfnmap vma, it don't have a valid page for all pfns, so unmap is not expected. In this case, it just check wheather the first address have a valid pte or not which seems reasonable to me. >> >>> >>> CoW with a valid page for pfnmap vma is weird to us. Can we use >>> remap_pfn_range for private vma(read only)? Once CoW happens on a pfnmap >>> vma during write fault, this page is normal(page flag is valid) for most mm >>> subsystems, such as memory failure in thais case and extra should be done to >>> handle this special page. >>> >>> During unmap, if this vma is pfnmap, unmap shouldn't be done since page >>> should not be touched for pfnmap vma. >>> >>> But the root problem is that can we insert a valid page for pfnmap vma? >>> >>> Any thoughts to solve this warn? >> >> vm_normal_page() documentation explains how that magic is supposed to >> work. vm_normal_page() should be able to correctly identify whether we >> want to look at the struct page for an anon folio that was COWed. vm_normal_page() can find out a CoW mapping but >> >> >> untrack_pfn() indeed does not seem to be well prepared for handling >> MAP_PRIVATE mappings where we end up having anon folios. >> >> I think it will already *completely mess up* simply when unmapping the >> range without the memory failure involved. >> >> See, follow_phys() would get the PFN of the anon folio and then >> untrack_pfn() would do some nonesense with that. Completely broken. >> >> The WARN is just a side-effect of the brokenness. >> >> In follow_phys(), we'd likely have to call vm_normal_page(). If we get a >> page back, we'd likely have to fail follow_phys() instead of returning a >> PFN of an anon folio. >> >> Now, how do we fix untrack_pfn() ? I really don't know. In theory, we >> might no longer have *any* PFNMAP PFN in there after COW'ing everything. >> >> Sounds like MAP_PRIVATE VM_PFNMAP + __HAVE_PFNMAP_TRACKING is some >> broken garbage (sorry). Can we disallow it? > > Staring at track_pfn_copy(), it's maybe similarly broken? > > I think we want to do: > > diff --git a/mm/memory.c b/mm/memory.c > index 098356b8805ae..da5d1e37c5534 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -6050,6 +6050,10 @@ int follow_phys(struct vm_area_struct *vma, >                 goto out; >         pte = ptep_get(ptep); >   > +       /* Never return addresses of COW'ed anon folios. */ > +       if (vm_normal_page(vma, address, pte)) > +               goto unlock; > + >         if ((flags & FOLL_WRITE) && !pte_write(pte)) >                 goto unlock; >   > > And then, just disallow it with PAT involved: > > diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c > index 0904d7e8e1260..e4d2b2e8c0281 100644 > --- a/arch/x86/mm/pat/memtype.c > +++ b/arch/x86/mm/pat/memtype.c > @@ -997,6 +997,15 @@ int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot, >                                 && size == (vma->vm_end - vma->vm_start))) { >                 int ret; >   > +               /* > +                * untrack_pfn() and friends cannot handl regions that suddenly > +                * contain anon folios after COW. In particular, follow_phys() > +                * will fail when we have an anon folio at the beginning og the > +                * VMA. > +                */ > +               if (vma && is_cow_mapping(vma->vm_flags)) > +                       return -EINVAL; In this case, anyone use remap_pfn_range can not be cow_maaping which means if VM_MAYWRITE exists, VM_SHARED is needed for this vma. This can solve this CoW on private vma problem. > + >                 ret = reserve_pfn_range(paddr, size, prot, 0); >                 if (ret == 0 && vma) >                         vm_flags_set(vma, VM_PAT); > > > I'm afraid that will break something. But well, it's already semi-broken. > > As long as VM_PAT is not involved, it should work as expected. > > In an ideal world, we'd get rid of follow_phys() completely and just > derive that information from the VMA? >