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 AEF8AC433EF for ; Fri, 25 Feb 2022 05:15:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D597A8D0002; Fri, 25 Feb 2022 00:15:28 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D076D8D0001; Fri, 25 Feb 2022 00:15:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BF5C08D0002; Fri, 25 Feb 2022 00:15:28 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0126.hostedemail.com [216.40.44.126]) by kanga.kvack.org (Postfix) with ESMTP id ADC0C8D0001 for ; Fri, 25 Feb 2022 00:15:28 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 6037F9F5DE for ; Fri, 25 Feb 2022 05:15:28 +0000 (UTC) X-FDA: 79180139136.23.481FFE6 Received: from r3-24.sinamail.sina.com.cn (r3-24.sinamail.sina.com.cn [202.108.3.24]) by imf10.hostedemail.com (Postfix) with SMTP id 762D5C0003 for ; Fri, 25 Feb 2022 05:15:24 +0000 (UTC) Received: from unknown (HELO localhost.localdomain)([114.249.61.131]) by sina.com (172.16.97.23) with ESMTP id 621865C50000BDB0; Fri, 25 Feb 2022 13:14:47 +0800 (CST) X-Sender: hdanton@sina.com X-Auth-ID: hdanton@sina.com X-SMAIL-MID: 74271254919777 From: Hillf Danton To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Linus Torvalds , John Hubbard , Oded Gabbay Subject: Re: [PATCH RFC 03/13] mm/memory: slightly simplify copy_present_pte() Date: Fri, 25 Feb 2022 13:15:11 +0800 Message-Id: <20220225051511.2383-1-hdanton@sina.com> In-Reply-To: <20220224122614.94921-4-david@redhat.com> References: <20220224122614.94921-1-david@redhat.com> MIME-Version: 1.0 Authentication-Results: imf10.hostedemail.com; dkim=none; spf=pass (imf10.hostedemail.com: domain of hdanton@sina.com designates 202.108.3.24 as permitted sender) smtp.mailfrom=hdanton@sina.com; dmarc=none X-Rspam-User: X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 762D5C0003 X-Stat-Signature: acp8wxwhbqem5i5em75uhg9wwgxjhj11 X-HE-Tag: 1645766124-896387 Content-Transfer-Encoding: quoted-printable 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, 24 Feb 2022 13:26:04 +0100 David Hildenbrand wrote: > Let's move the pinning check into the caller, to simplify return code > logic and prepare for further changes: relocating the > page_needs_cow_for_dma() into rmap handling code. >=20 > While at it, remove the unused pte parameter and simplify the comments = a > bit. >=20 > No functional change intended. >=20 > Signed-off-by: David Hildenbrand > --- > mm/memory.c | 53 ++++++++++++++++------------------------------------- > 1 file changed, 16 insertions(+), 37 deletions(-) >=20 > diff --git a/mm/memory.c b/mm/memory.c > index c6177d897964..accb72a3343d 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -865,19 +865,11 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, str= uct mm_struct *src_mm, > } > =20 > /* > - * Copy a present and normal page if necessary. > + * Copy a present and normal page. > * > - * NOTE! The usual case is that this doesn't need to do > - * anything, and can just return a positive value. That > - * will let the caller know that it can just increase > - * the page refcount and re-use the pte the traditional > - * way. > - * > - * But _if_ we need to copy it because it needs to be > - * pinned in the parent (and the child should get its own > - * copy rather than just a reference to the same page), > - * we'll do that here and return zero to let the caller > - * know we're done. > + * NOTE! The usual case is that this isn't required; > + * instead, the caller can just increase the page refcount > + * and re-use the pte the traditional way. > * > * And if we need a pre-allocated page but don't yet have > * one, return a negative error to let the preallocation > @@ -887,25 +879,10 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, str= uct mm_struct *src_mm, > static inline int > copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struc= t *src_vma, > pte_t *dst_pte, pte_t *src_pte, unsigned long addr, int *rss, > - struct page **prealloc, pte_t pte, struct page *page) > + struct page **prealloc, struct page *page) > { > struct page *new_page; > - > - /* > - * What we want to do is to check whether this page may > - * have been pinned by the parent process. If so, > - * instead of wrprotect the pte on both sides, we copy > - * the page immediately so that we'll always guarantee > - * the pinned page won't be randomly replaced in the > - * future. > - * > - * The page pinning checks are just "has this mm ever > - * seen pinning", along with the (inexact) check of > - * the page count. That might give false positives for > - * for pinning, but it will work correctly. > - */ > - if (likely(!page_needs_cow_for_dma(src_vma, page))) > - return 1; > + pte_t pte; > =20 > new_page =3D *prealloc; > if (!new_page) > @@ -947,14 +924,16 @@ copy_present_pte(struct vm_area_struct *dst_vma, = struct vm_area_struct *src_vma, > struct page *page; > =20 > page =3D vm_normal_page(src_vma, addr, pte); > - if (page) { > - int retval; > - > - retval =3D copy_present_page(dst_vma, src_vma, dst_pte, src_pte, > - addr, rss, prealloc, pte, page); > - if (retval <=3D 0) > - return retval; > - > + if (page && unlikely(page_needs_cow_for_dma(src_vma, page))) { > + /* > + * If this page may have been pinned by the parent process, > + * copy the page immediately for the child so that we'll always > + * guarantee the pinned page won't be randomly replaced in the > + * future. > + */ > + return copy_present_page(dst_vma, src_vma, dst_pte, src_pte, > + addr, rss, prealloc, page); Off topic question, is it likely that the DMA tranfer from device in para= llel to copying page ends up with different data between parent and kid? Hillf > + } else if (page) { > get_page(page); > page_dup_rmap(page, false); > rss[mm_counter(page)]++; > --=20 > 2.35.1