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=-11.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 35CB8C433E1 for ; Fri, 21 Aug 2020 10:13:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C727A20656 for ; Fri, 21 Aug 2020 10:13:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C727A20656 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 336FD8D001B; Fri, 21 Aug 2020 06:13:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2E8E78D0006; Fri, 21 Aug 2020 06:13:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1FE2E8D001B; Fri, 21 Aug 2020 06:13:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0046.hostedemail.com [216.40.44.46]) by kanga.kvack.org (Postfix) with ESMTP id 093BB8D0006 for ; Fri, 21 Aug 2020 06:13:36 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id BDF3F1F06 for ; Fri, 21 Aug 2020 10:13:35 +0000 (UTC) X-FDA: 77174163990.05.fruit77_1e05a2427038 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 946761802EAC5 for ; Fri, 21 Aug 2020 10:13:35 +0000 (UTC) X-HE-Tag: fruit77_1e05a2427038 X-Filterd-Recvd-Size: 6712 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Fri, 21 Aug 2020 10:13:34 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id A8F65AE38; Fri, 21 Aug 2020 10:14:01 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 3B8F31E1312; Fri, 21 Aug 2020 12:13:33 +0200 (CEST) Date: Fri, 21 Aug 2020 12:13:33 +0200 From: Jan Kara To: Linus Torvalds Cc: Peter Xu , Andrea Arcangeli , Linux-MM , Linux Kernel Mailing List , Andrew Morton , Marty Mcfadden , "Maya B . Gokhale" , Jann Horn , Christoph Hellwig , Oleg Nesterov , Kirill Shutemov , Jan Kara Subject: Re: [PATCH v3] mm/gup: Allow real explicit breaking of COW Message-ID: <20200821101333.GA3432@quack2.suse.cz> References: <20200811183950.10603-1-peterx@redhat.com> <20200811214255.GE6353@xz-x1> <20200820215449.GB358043@xz-x1> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Rspamd-Queue-Id: 946761802EAC5 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 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 20-08-20 15:01:00, Linus Torvalds wrote: > On Thu, Aug 20, 2020 at 2:54 PM Peter Xu wrote: > > > > I kind of prefer the new suggestion to remove code rather than adding new > > codes. I definitely don't know enough on the side effect of it, especially > > performance-wise on either ksm or swap, but... IIUC the worst case is we'll get > > some perf report later on, and it seems also not hard to revert the patch later > > if we want. > > Well, would you be willing to try this patch out? > > After you apply that patch, you should be able to remove the > should_force_cow_break() games entirely, because a write to the page > should always break cow towards the writer if there are any GUP users > around (put another way: away from the GUP). > > However, to make the test meaningful, it really should do some swap testing too. > > Linus > From f41082844ea82ad1278e167fe6e973fa4efc974a Mon Sep 17 00:00:00 2001 > From: Linus Torvalds > Date: Tue, 11 Aug 2020 14:23:04 -0700 > Subject: [PATCH] Trial do_wp_page() simplification > > How about we just make sure we're the only possible valid user fo the > page before we bother to reuse it? > > Simplify, simplify, simplify. > > And get rid of the nasty serialization on the page lock at the same time. > > Signed-off-by: Linus Torvalds > --- > mm/memory.c | 58 +++++++++++++++-------------------------------------- > 1 file changed, 16 insertions(+), 42 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index 602f4283122f..a43004dd2ff6 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -2927,50 +2927,24 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf) > * not dirty accountable. > */ > if (PageAnon(vmf->page)) { > - int total_map_swapcount; > - if (PageKsm(vmf->page) && (PageSwapCache(vmf->page) || > - page_count(vmf->page) != 1)) > + struct page *page = vmf->page; > + > + if (page_count(page) != 1) > + goto copy; > + if (!trylock_page(page)) > + goto copy; > + if (page_mapcount(page) != 1 && page_count(page) != 1) { So this condition looks strange to me... Did you mean: if (page_mapcount(page) != 1 || page_count(page) != 1) ? Because page mapcount is >= 1 here, and if mapcount is > 1, then page_count is certainly greater than 1 as well... > + unlock_page(page); > goto copy; > - if (!trylock_page(vmf->page)) { > - get_page(vmf->page); > - pte_unmap_unlock(vmf->pte, vmf->ptl); > - lock_page(vmf->page); > - vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, > - vmf->address, &vmf->ptl); > - if (!pte_same(*vmf->pte, vmf->orig_pte)) { > - update_mmu_tlb(vma, vmf->address, vmf->pte); > - unlock_page(vmf->page); > - pte_unmap_unlock(vmf->pte, vmf->ptl); > - put_page(vmf->page); > - return 0; > - } > - put_page(vmf->page); > - } > - if (PageKsm(vmf->page)) { Also I know nothing about KSM but looking at reuse_ksm_page() I can see it plays some tricks with page index & mapping even for pages with page_count == 1 so you cannot just drop those bits AFAICT. > - bool reused = reuse_ksm_page(vmf->page, vmf->vma, > - vmf->address); > - unlock_page(vmf->page); > - if (!reused) > - goto copy; > - wp_page_reuse(vmf); > - return VM_FAULT_WRITE; > - } > - if (reuse_swap_page(vmf->page, &total_map_swapcount)) { Also I'm not sure if dropping this is safe for THP - reuse_swap_page() seems to be a misnomer and seems to do also some THP handling. In particular a comment before page_trans_huge_mapcount() states that page_mapcount() isn't fully accurate for THP and page_trans_huge_mapcount() should be used instead for checking whether a copy is needed on write fault. Honza > - if (total_map_swapcount == 1) { > - /* > - * The page is all ours. Move it to > - * our anon_vma so the rmap code will > - * not search our parent or siblings. > - * Protected against the rmap code by > - * the page lock. > - */ > - page_move_anon_rmap(vmf->page, vma); > - } > - unlock_page(vmf->page); > - wp_page_reuse(vmf); > - return VM_FAULT_WRITE; > } > - unlock_page(vmf->page); > + /* > + * Ok, we've got the only map reference, and the only > + * page count reference, and the page is locked, > + * it's dark out, and we're wearing sunglasses. Hit it. > + */ > + wp_page_reuse(vmf); > + unlock_page(page); > + return VM_FAULT_WRITE; > } else if (unlikely((vma->vm_flags & (VM_WRITE|VM_SHARED)) == > (VM_WRITE|VM_SHARED))) { > return wp_page_shared(vmf); > -- > 2.28.0.218.gc12ef3d349 > -- Jan Kara SUSE Labs, CR