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 1E056C4321E for ; Fri, 2 Dec 2022 17:11:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AF8836B0071; Fri, 2 Dec 2022 12:11:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A7FE16B0073; Fri, 2 Dec 2022 12:11:23 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8D3CC6B0074; Fri, 2 Dec 2022 12:11:23 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 773246B0071 for ; Fri, 2 Dec 2022 12:11:23 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 420F81A03C0 for ; Fri, 2 Dec 2022 17:11:23 +0000 (UTC) X-FDA: 80198007246.29.9DAD726 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf15.hostedemail.com (Postfix) with ESMTP id D878DA000B for ; Fri, 2 Dec 2022 17:11:22 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=HKMZQphs; spf=pass (imf15.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670001082; a=rsa-sha256; cv=none; b=mGeqTpkYD9KxiovrFdiye1P3Pqmdsdq2Uxgbiu1UtjWHdAGNJwUNxbsigQHC12UVFTNHbK J4pgPwSgiHCjZaukTZlX4uIziO+o4Ks+YRp8f1vUjoyFP2Ngvak39WkyJlBTRB+NNZ5986 Aee3LRbwbvbQWkP+9sssGwM0a5lJzdM= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=HKMZQphs; spf=pass (imf15.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670001082; 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:dkim-signature; bh=Usmp2CDAvNfy2M2omAE7j3vT3EFJoDk3WrwAZjTClPA=; b=xvbhuppTTs4NPbx0FMDfcN+Ow24eTTYhSy7Elxj4Gbw4wlYCqx60mkRmGHH88QrSRw6bpx l/GR0Fyyvjlk5cP2OGZ/VNcuACLLTsZprgpptbVbimZC4suE/a0toLpfYHemRxXR/fm3xm fke/V3vCBtAqZntvbgWFV8uwSHeNVq0= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670001082; h=from:from: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=Usmp2CDAvNfy2M2omAE7j3vT3EFJoDk3WrwAZjTClPA=; b=HKMZQphsszwE4Zv8uPwdzuyq6Hvzlg4UA9cmuQC0f0nx0+YdLhHW0E1/+d3uFv3BgciWHo 7joDQ8rljYIOSptt5L17X0lcOUT6vUxqYz+eup1E6cZLrku9oaCW/kXs4enmUxKtKHmVs9 wth1s/uz/U0+/ikXH6cpg5CBjQW9q3E= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-299-YfI2Px6MOg-vWbfu9waFqw-1; Fri, 02 Dec 2022 12:11:21 -0500 X-MC-Unique: YfI2Px6MOg-vWbfu9waFqw-1 Received: by mail-wr1-f72.google.com with SMTP id m24-20020adfa3d8000000b00242168ce9d1so1232840wrb.15 for ; Fri, 02 Dec 2022 09:11:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:content-language :references:cc:to:from:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Usmp2CDAvNfy2M2omAE7j3vT3EFJoDk3WrwAZjTClPA=; b=Fx6lKOasUku7z++jOR1KbmGwVbm4XXJ5/LTLN26eeYW2KgkFCoStuSQq5jGJjkwHRW A4a0O23wi7TcUzkJ5qVN2jtGYlM4WTmYq9Kuq9xhvFc65HFWLwMmsrLMu3qRMaGgmRMO qnjcIMjJh69/LDh1u0Cul9d4KSAgWLJ3Vl6ie5h7poUjqcLF52iwzFNPwK6nmIpGe2lX yG//Zo6+n/VwuB9cZetvv4R3shpjSXBc5WOLW6JlyK4WA9Xcexr6Umka1HdBqdWElAfW LAstxLq/iTLL3vIgEZ93bHdP075boPXsR5ry5XUJtBOKVS0N0fKh2lK4WqvgPLp8tng8 fvjA== X-Gm-Message-State: ANoB5plDAU7Qv9J64x1qE8FVbZiJ/KhcqjE/9u4SzMKLpKTnIfhw9zrJ sMhiamf2GjBTRkna4l4Gkv6iCSR5isW1tQT8rIW+E7kltfLXZLl/EOfjQ26zcHAqUxyNYscoI7W 5WK2heWgxWcs= X-Received: by 2002:a05:600c:4e09:b0:3cf:55bd:4944 with SMTP id b9-20020a05600c4e0900b003cf55bd4944mr55175170wmq.64.1670001079838; Fri, 02 Dec 2022 09:11:19 -0800 (PST) X-Google-Smtp-Source: AA0mqf5x3wkkVzcb4HhqTzh3nzX1sXgFuCWZUpG5xrlEzmhIcsg9cFkabOgq2bh23ViczIQ35ByNfQ== X-Received: by 2002:a05:600c:4e09:b0:3cf:55bd:4944 with SMTP id b9-20020a05600c4e0900b003cf55bd4944mr55175147wmq.64.1670001079507; Fri, 02 Dec 2022 09:11:19 -0800 (PST) Received: from ?IPV6:2003:cb:c703:7a00:852e:72cd:ed76:d72f? (p200300cbc7037a00852e72cded76d72f.dip0.t-ipconnect.de. [2003:cb:c703:7a00:852e:72cd:ed76:d72f]) by smtp.gmail.com with ESMTPSA id e18-20020a5d4e92000000b0024206ed539fsm7307560wru.66.2022.12.02.09.11.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 02 Dec 2022 09:11:19 -0800 (PST) Message-ID: <19800718-9cb6-9355-da1c-c7961b01e922@redhat.com> Date: Fri, 2 Dec 2022 18:11:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH RFC] mm/userfaultfd: enable writenotify while userfaultfd-wp is enabled for a VMA From: David Hildenbrand To: Peter Xu Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Ives van Hoorne , stable@vger.kernel.org, Andrew Morton , Hugh Dickins , Alistair Popple , Mike Rapoport , Nadav Amit , Andrea Arcangeli References: <20221202122748.113774-1-david@redhat.com> <690afe0f-c9a0-9631-b365-d11d98fdf56f@redhat.com> Organization: Red Hat In-Reply-To: <690afe0f-c9a0-9631-b365-d11d98fdf56f@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Spamd-Result: default: False [-1.40 / 9.00]; BAYES_HAM(-6.00)[100.00%]; SORBS_IRL_BL(3.00)[209.85.221.72:received]; SUSPICIOUS_RECIPS(1.50)[]; RCVD_NO_TLS_LAST(0.10)[]; MIME_GOOD(-0.10)[text/plain]; BAD_REP_POLICIES(0.10)[]; R_SPF_ALLOW(0.00)[+ip4:170.10.129.0/24]; RCPT_COUNT_SEVEN(0.00)[11]; DMARC_POLICY_ALLOW(0.00)[redhat.com,none]; DKIM_TRACE(0.00)[redhat.com:+]; ARC_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; FROM_EQ_ENVFROM(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; HAS_ORG_HEADER(0.00)[]; PREVIOUSLY_DELIVERED(0.00)[linux-mm@kvack.org]; TAGGED_RCPT(0.00)[]; RCVD_COUNT_THREE(0.00)[4]; ARC_SIGNED(0.00)[hostedemail.com:s=arc-20220608:i=1]; R_DKIM_ALLOW(0.00)[redhat.com:s=mimecast20190719]; FROM_HAS_DN(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[] X-Rspamd-Queue-Id: D878DA000B X-Rspamd-Server: rspam01 X-Stat-Signature: suif7rm9htotu8bdhht3tb5is4k7yhb5 X-HE-Tag: 1670001082-848939 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 02.12.22 17:56, David Hildenbrand wrote: > On 02.12.22 17:33, Peter Xu wrote: >> On Fri, Dec 02, 2022 at 01:27:48PM +0100, David Hildenbrand wrote: >>> Currently, we don't enable writenotify when enabling userfaultfd-wp on >>> a shared writable mapping (for now we only support SHMEM). The consequence >> >> and hugetlbfs >> >>> is that vma->vm_page_prot will still include write permissions, to be set >>> as default for all PTEs that get remapped (e.g., mprotect(), NUMA hinting, >>> page migration, ...). >> >> The thing is by default I think we want the write bit.. >> >> The simple example is (1) register UFFD_WP on shmem writable, (2) write a >> page. Here we didn't wr-protect anything, so we want the write bit there. >> >> Or the other example is when UFFDIO_COPY with flags==0 even if with >> VM_UFFD_WP. We definitely wants the write bit. >> >> We only doesn't want the write bit when uffd-wp is explicitly set. >> >> I think fundamentally the core is uffd-wp is pte-based, so the information >> resides in pte not vma. I'm not strongly objecting this patch, especially >> you mentioned auto-numa so I need to have a closer look later there. >> However I do think uffd-wp is slightly special because we always need to >> consider pte information anyway, so a per-vma information doesn't hugely >> help, IMHO. > > That's the same as softdirty tracking, IMHO. > > [...] > >>> Running the mprotect() reproducer [1] without this commit: >>> $ ./uffd-wp-mprotect >>> FAIL: uffd-wp did not fire >>> Running the mprotect() reproducer with this commit: >>> $ ./uffd-wp-mprotect >>> PASS: uffd-wp fired >>> >>> [1] https://lore.kernel.org/all/222fc0b2-6ec0-98e7-833f-ea868b248446@redhat.com/T/#u >> >> I still hope for a formal patch (non-rfc) we can have a reproducer outside >> mprotect(). IMHO mprotect() is really ambiguously here being used with >> uffd-wp, so not a good example IMO as I explained in the other thread [1]. > > I took the low hanging fruit to showcase that this is a more generic problem. > The reproducer is IMHO nice because it's simple and race-free. > >> >> I'll need to off-work most of the rest of today, but maybe I can also have >> a look in the weekend or Monday more on the numa paths. Before that, can >> we first reach a consensus that we have the mm/migrate patch there to be >> merged first? These are two issues, IMHO. >> >> I know you're against me for some reason, but until now I sincerely don't >> know why. That patch sololy recovers write bit status (by removing it for >> read-only) for a migration entry and that definitely makes sense to me. As >> I also mentioned in the old version of that thread, we can rework migration >> entries and merge READ|WRITE entries into a GENERIC entry one day if you >> think proper, but that's for later. > > I'm not against you. I'm against changing well-working, common code > when it doesn't make any sense to me to change it. And now we have proof that > mprotect() just behaves exactly the same way, using the basic rules of vma->vm_page_prot. > > Yes, there is broken sparc64 (below), but that shouldn't dictate our implementation. > > > What *would* make sense to me, as I raised, is: > > diff --git a/mm/migrate.c b/mm/migrate.c > index dff333593a8a..9fc181fd3c5a 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -213,8 +213,10 @@ static bool remove_migration_pte(struct folio *folio, > pte = pte_mkdirty(pte); > if (is_writable_migration_entry(entry)) > pte = maybe_mkwrite(pte, vma); > - else if (pte_swp_uffd_wp(*pvmw.pte)) > + else if (pte_swp_uffd_wp(*pvmw.pte)) { > pte = pte_mkuffd_wp(pte); > + pt = pte_wrprotect(pte); > + } > > if (folio_test_anon(folio) && !is_readable_migration_entry(entry)) > rmap_flags |= RMAP_EXCLUSIVE; > > > It still requires patch each and every possible code location, which I dislike as > described in the patch description. The fact that there are still uffd-wp bugs > with your patch makes that hopefully clear. I'd be interested if they can be > reproduced witht his patch. > And if NUMA hinting is indeed the problem, without this patch what would be required would most probably be: diff --git a/mm/memory.c b/mm/memory.c index 8a6d5c823f91..869d35ef0e24 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4808,6 +4808,8 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf) pte = pte_mkyoung(pte); if (was_writable) pte = pte_mkwrite(pte); + if (pte_uffd_wp(pte)) + pte = pte_wrprotect(pte); ptep_modify_prot_commit(vma, vmf->address, vmf->pte, old_pte, pte); update_mmu_cache(vma, vmf->address, vmf->pte); pte_unmap_unlock(vmf->pte, vmf->ptl); And just to make my point about the migration path clearer: doing it your way would be: diff --git a/mm/memory.c b/mm/memory.c index 8a6d5c823f91..a7c4c1a57f6a 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4808,6 +4808,8 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf) pte = pte_mkyoung(pte); if (was_writable) pte = pte_mkwrite(pte); + else + pte = pte_wrprotect(pte); ptep_modify_prot_commit(vma, vmf->address, vmf->pte, old_pte, pte); update_mmu_cache(vma, vmf->address, vmf->pte); pte_unmap_unlock(vmf->pte, vmf->ptl); And I don't think that's the right approach. -- Thanks, David / dhildenb