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 0C9E3C433EF for ; Wed, 20 Jul 2022 20:38:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 78E046B0072; Wed, 20 Jul 2022 16:38:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 715D66B0073; Wed, 20 Jul 2022 16:38:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 591CE6B0074; Wed, 20 Jul 2022 16:38:45 -0400 (EDT) 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 444266B0072 for ; Wed, 20 Jul 2022 16:38:45 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 10AEF1406DD for ; Wed, 20 Jul 2022 20:38:45 +0000 (UTC) X-FDA: 79708641810.27.E88C46E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf15.hostedemail.com (Postfix) with ESMTP id 78889A0016 for ; Wed, 20 Jul 2022 20:38:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1658349524; 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=TYQic73id/AGT/OnkFuoiP1qth7L5pTxU/BQI9eja8w=; b=cPtsT6vE6Fob5SNsw2XMIgNTr5/dy8oksw/dPKqdoTAZC4DhlwwjnaPW5rQ2tbXl5QZj7s q1FLwm6n5qPpi79yTfvoxSB4i548o+Py2HNQ9S9Kw+xx9ucbbvClyRY7uA0DRjQnSquJxs K08DuQdFC5HXOWkDaCjWtqtiXzW+0Y4= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-312-f_9dGIzPNBWAzcN8ZTwT2w-1; Wed, 20 Jul 2022 16:38:43 -0400 X-MC-Unique: f_9dGIzPNBWAzcN8ZTwT2w-1 Received: by mail-wr1-f69.google.com with SMTP id q9-20020adfb189000000b0021e3e7d3242so1101050wra.19 for ; Wed, 20 Jul 2022 13:38:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:organization:subject :in-reply-to:content-transfer-encoding; bh=TYQic73id/AGT/OnkFuoiP1qth7L5pTxU/BQI9eja8w=; b=NsdPs+UcUt4U1bqShta1X6FRzsBWedqfb6qFtDMybYesR9g4yknYCRp8NFPP9svRYV esYI6PxcXblua8LYZqTpYRgVU7KtBJABIJRK3Ht5E8WyRMBdFUCG8rIlPvyMjWrK3WUP 5SUNTdnfqtajzieXabZBZ6/ZgpoXJxf+7bvwjaXTPHiCNIc1W1omqKGfCM+7Td/k6nrb jhApkx6NuXNP1z3xCt97qKe9+jRnRW7us3loki3iDxOWU05QK+xJWRFoytmOcp3/E2eW LPDddDJahN0nLCPpFTcgw3MymLhtjqiG2bnQQZ3uA/dm5RbUfRnAf0fLVowwX+K9vdVw fQbg== X-Gm-Message-State: AJIora80EtWNtVlfvLAOE6YyASxmB7RgIbWifQJBvlPHWRcN8vlfhhF0 sdSabmSQVGDrQ4sE9y0WFuDgOyznU1nJOoN6ZsqY2S/kyqvUgcvbzdXrnF09ZN16fyxagypvx2H wspqUhYBZLck= X-Received: by 2002:a7b:ce8e:0:b0:3a3:1b01:8e7 with SMTP id q14-20020a7bce8e000000b003a31b0108e7mr5439013wmj.31.1658349521748; Wed, 20 Jul 2022 13:38:41 -0700 (PDT) X-Google-Smtp-Source: AGRyM1shvxtvZfdWloOMKGoHi916GZzdqgeyDDvSUbHd4Jb9RPJid+Nm4xr6sP20RXU9GZq1BhAaVA== X-Received: by 2002:a7b:ce8e:0:b0:3a3:1b01:8e7 with SMTP id q14-20020a7bce8e000000b003a31b0108e7mr5438993wmj.31.1658349521491; Wed, 20 Jul 2022 13:38:41 -0700 (PDT) Received: from ?IPV6:2003:cb:c706:e00:8d96:5dba:6bc4:6e89? (p200300cbc7060e008d965dba6bc46e89.dip0.t-ipconnect.de. [2003:cb:c706:e00:8d96:5dba:6bc4:6e89]) by smtp.gmail.com with ESMTPSA id d3-20020adffd83000000b0021da61caa10sm16765583wrr.56.2022.07.20.13.38.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Jul 2022 13:38:41 -0700 (PDT) Message-ID: <468a7114-7541-0d5e-c1fc-083bbb95e78d@redhat.com> Date: Wed, 20 Jul 2022 22:38:39 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 To: Nadav Amit Cc: Peter Xu , Linux MM , LKML , Andrew Morton , Mike Rapoport , Axel Rasmussen , Andrea Arcangeli , Andrew Cooper , Andy Lutomirski , Dave Hansen , Peter Zijlstra , Thomas Gleixner , Will Deacon , Yu Zhao , Nick Piggin References: <20220718120212.3180-1-namit@vmware.com> <20220718120212.3180-2-namit@vmware.com> <017facf0-7ef8-3faf-138d-3013a20b37db@redhat.com> <2b4393ce-95c9-dd3e-8495-058a139e771e@redhat.com> <69022bad-d6f1-d830-224d-eb8e5c90d5c7@redhat.com> <4ad140b5-1d5b-2486-0893-7886a9cdfd76@redhat.com> <95320077-52CF-4CB0-92F9-523E1AE74A3D@gmail.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [RFC PATCH 01/14] userfaultfd: set dirty and young on writeprotect In-Reply-To: <95320077-52CF-4CB0-92F9-523E1AE74A3D@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658349524; a=rsa-sha256; cv=none; b=M3UbKbP1W2hCz4fU72U5+ExYFpojnyTy+3kPg+obfg702OTjMHEHFlL3dlxLEB2Fq4tGbT XaHllvZzHqLG7f+pLTD30Cq6aNzk5GHHZIIkBSMV0HjPHxXOWuItU/afz4l7CCp6bT1p16 ZgkdP85xlriaPEQ4Rsolt/6gxqCjoLY= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=cPtsT6vE; spf=none (imf15.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.133.124) 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=1658349524; 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=TYQic73id/AGT/OnkFuoiP1qth7L5pTxU/BQI9eja8w=; b=zV5EsrahTeh81Unm/pMkJHfBSH3ke+OdIDkkaMrLNmrheiCsdjkrhVX9IVsVOa2CEN0dg0 5V43Y2zrS7QV6O/d83mAPrXEKU2MNJvwgc0QEY+S71RzJpljlMdKs+wh5urvTmsyWgwdFL lfDxgDJsJwz2bCgfajemXrPoLmFX9do= X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 78889A0016 Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=cPtsT6vE; spf=none (imf15.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Stat-Signature: qwqaeuqna8jgzei6x3679tspa6yw84zy X-HE-Tag: 1658349524-819476 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 20.07.22 22:22, Nadav Amit wrote: > On Jul 20, 2022, at 12:55 PM, David Hildenbrand wrote: > >> On 20.07.22 21:48, Peter Xu wrote: >>> On Wed, Jul 20, 2022 at 09:33:35PM +0200, David Hildenbrand wrote: >>>> On 20.07.22 21:15, Peter Xu wrote: >>>>> On Wed, Jul 20, 2022 at 05:10:37PM +0200, David Hildenbrand wrote: >>>>>> For pagecache pages it may as well be *plain wrong* to bypass the write >>>>>> fault handler and simply mark pages dirty+map them writable. >>>>> >>>>> Could you elaborate? >>>> >>>> Write-fault handling for some filesystems (that even require this >>>> "slow path") is a bit special. >>>> >>>> For example, do_shared_fault() might have to call page_mkwrite(). >>>> >>>> AFAIK file systems use that for lazy allocation of disk blocks. >>>> If you simply go ahead and map a !dirty pagecache page writable >>>> and mark it dirty, it will not trigger page_mkwrite() and you might >>>> end up corrupting data. >>>> >>>> That's why we the old change_pte_range() code never touched >>>> anything if the pte wasn't already dirty. >>> >>> I don't think that pte_dirty() check was for the pagecache code. For any fs >>> that has page_mkwrite() defined, it'll already have vma_wants_writenotify() >>> return 1, so we'll never try to add write bit, hence we'll never even try >>> to check pte_dirty(). >> >> I might be too tired, but the whole reason we had this magic before my >> commit in place was only for the pagecache. >> >> With vma_wants_writenotify()=0 you can directly map the pages writable >> and don't have to do these advanced checks here. In a writable >> MAP_SHARED VMA you'll already have pte_write(). >> >> We only get !pte_write() in case we have vma_wants_writenotify()=1 ... >> >> try_change_writable = vma_wants_writenotify(vma, vma->vm_page_prot); >> >> and that's the code that checked the dirty bit after all to decide -- >> amongst other things -- if we can simply map it writable without going >> via the write fault handler and triggering do_shared_fault() . >> >> See crazy/ugly FOLL_FORCE code in GUP that similarly checks the dirty bit. > > I thought you want to get rid of it at least for anonymous pages. No? Yes. Especially for any MAP_PRIVATE mappings. If you want to write to something that's not mapped writable in a MAP_PRIVATE mapping it a) Has to be an exclusive anonymous page b) The pte has to be dirty In any other case, you clearly missed to COW or the modifications might get lost if the PTE is not dirty. MAP_SHARED is a bit more involved. But whether the pte is dirty might be good enough ... but this needs a lot more care. > >> >> But yeah, it's all confusing so I might just be wrong regarding >> pagecache pages. > > Just to note: I am not very courageous and I did not intend to change > condition for when non-anonymous pages are set as writable. That’s the > reason I did not change the dirty for non-writable non-anonymous entries (as > Peter said). And that’s the reason that setting the dirty bit (at least as I > should have done it) is only performed after we made the decision on the > write-bit. Good. As long as we stick to anonymous pages I roughly know what we we can and cannot do at this point :) The problem I see is that detection whether we can write requires the dirty bit ... and whether to set the dirty bit requires the information whether we can write. Again, for anonymous pages we should be able to relax the "dirty" requirement when detecting whether we can write. > > IOW, after you made your decision about the write-bit, then and only then > you may be able to set the dirty bit for writable entries. Since the entry > is already writeable (i.e., can be written without a fault later directly > from userspace), there should be no concern of correctness when you set it. That makes sense to me. What keeps confusing me are architectures with and without a hw-managed dirty bit ... :) -- Thanks, David / dhildenb