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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E730CC433EF for ; Tue, 28 Sep 2021 21:37:39 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6F0AA6135F for ; Tue, 28 Sep 2021 21:37:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6F0AA6135F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 5C5E4940007; Tue, 28 Sep 2021 17:37:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 57606900002; Tue, 28 Sep 2021 17:37:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 43C6F940007; Tue, 28 Sep 2021 17:37:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0248.hostedemail.com [216.40.44.248]) by kanga.kvack.org (Postfix) with ESMTP id 31B7C900002 for ; Tue, 28 Sep 2021 17:37:38 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id E3EE38249980 for ; Tue, 28 Sep 2021 21:37:37 +0000 (UTC) X-FDA: 78638294154.01.47FDFE6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf26.hostedemail.com (Postfix) with ESMTP id 7E4BE20019C6 for ; Tue, 28 Sep 2021 21:37:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632865057; 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: in-reply-to:in-reply-to:references:references; bh=k0oTxkz7NtuCdfXi6wiYu/Dbe5jAm1i2ab7g2h0GdR0=; b=NbPpsB0+lr6h8bCtbNr2GkoiThnKLmjFSo6rVv4fsMAbHfSC3mk80AMWNaih5pShA47w9s n74P4p1FwzfkndCv62VpSR0pIjs0c5ZL5gU8RtwIpBbPZGKNiSugXiyVsG8JysjoZC0myq 60+8cGFv3y2nljDWI2oTowtNxijajGM= Received: from mail-il1-f198.google.com (mail-il1-f198.google.com [209.85.166.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-170-DC0ak0t9O46zK54wKvy3hA-1; Tue, 28 Sep 2021 17:37:35 -0400 X-MC-Unique: DC0ak0t9O46zK54wKvy3hA-1 Received: by mail-il1-f198.google.com with SMTP id y16-20020a927d10000000b00245291ad122so410993ilc.9 for ; Tue, 28 Sep 2021 14:37:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=k0oTxkz7NtuCdfXi6wiYu/Dbe5jAm1i2ab7g2h0GdR0=; b=MwIQjtIg1zdIF3Z+DEWrw2uPHAoaQYYqfnWMGGTGBlmzOKr2yMslnEtn8OQF8l4Nxn t5bWM1gCepRyGACCwvoOc6l7bImndgkNwphKCPIDVvg7C/FruN6bB7LU2j+VcILgNAgh T4qXk6A3hrUGMnrrhdL6wcBLZLYqT6a+Hu3DSJSEGgbYW4k0Q/uRAeX67UWul/L+pCu4 fVdehMLt5sJTp+ItQm7yhP3d3Advt5ZDF9BMdBAMz40LGQmPNsn5h3QHrn9riCpokd11 vZz7iXMzzzkAZ4sUud/PhButmPhnnbreBe19JoSr7Jrcf+fiK+oLcv71N/+/oXTPPTBa JKcA== X-Gm-Message-State: AOAM532oMSvZQ+N5mwhnCuQ7G9hkHdraMG2qgau7d+pIORQqlB2EMcm+ UpgdaY4Has9RHv7IsJhmNb63PU91bSpBm20h0Ue3iPffQxeQfbQ3bf1xh9xj8sAuuJ4aN63DJ5i 6r1pxXGYifi0= X-Received: by 2002:a92:cda8:: with SMTP id g8mr5799290ild.1.1632865054516; Tue, 28 Sep 2021 14:37:34 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzncef8egk2flstm1KV0toHZLOw1cqmPWcIxsJLhlr4ajexL7LvOez53i+1SFiWl4ZQ5ku7nw== X-Received: by 2002:a92:cda8:: with SMTP id g8mr5799277ild.1.1632865054226; Tue, 28 Sep 2021 14:37:34 -0700 (PDT) Received: from t490s ([2607:fea8:56a2:9100::d3ec]) by smtp.gmail.com with ESMTPSA id e9sm167781iob.52.2021.09.28.14.37.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Sep 2021 14:37:33 -0700 (PDT) Date: Tue, 28 Sep 2021 17:37:31 -0400 From: Peter Xu To: Hugh Dickins Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrea Arcangeli , Liam Howlett , Mike Rapoport , Yang Shi , David Hildenbrand , "Kirill A . Shutemov" , Jerome Glisse , Alistair Popple , Miaohe Lin , Matthew Wilcox , Axel Rasmussen Subject: Re: [PATCH v4 1/4] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte Message-ID: References: <20210915181456.10739-1-peterx@redhat.com> <20210915181456.10739-2-peterx@redhat.com> <49fddb9a-4a52-1df-8b7c-dde2a89330bf@google.com> <256c72c4-ac99-94fb-d76-fab08e5cf5f4@google.com> MIME-Version: 1.0 In-Reply-To: <256c72c4-ac99-94fb-d76-fab08e5cf5f4@google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=NbPpsB0+; spf=none (imf26.hostedemail.com: domain of peterx@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 7E4BE20019C6 X-Stat-Signature: wgxxsq18b1gduyzo9p9y17gcnf5x8b3d X-HE-Tag: 1632865057-457964 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 Tue, Sep 28, 2021 at 12:28:40PM -0700, Hugh Dickins wrote: > > That's why I really like this patch to happen, I want to save time for myself, > > and for anyone who will be fighting for another dirty lost issues. > > I've lost time on missed dirties too, so I ought to be more sympathetic > to your argument than I am: I'm afraid I read it as saying that you don't > really understand "dirty", so want to do it more often to be "safe". > Not a persuasive argument. I'd hope I didn't mis-understood dirty bit, please let me know otherwise, then I must have been making a very serious mistake, and also then I'll be more than glad to learn to make things right, as long as you could help me to achieve it. FWICT, I was trying to argue it's not worth it to worry the corner case, but obviously I failed. :) But that's fine and understandable, because it happens. > > > > > > > > > I haven't looked again (I have a pile of mails to respond to!), > > > but when I looked before I think I found that the vmf flags are > > > not available to the userfaultfd ioctler. If so, then it would > > > be more appropriate to just leave the mkdirty to the hardware on > > > return from fault (except - and again I cannot spend time researching > > > this - perhaps I'm too x86-centric, and there are other architectures > > > on which the software *must* do the mkdirty fixup to avoid refaulting > > > forever - though probably userfaultfd state would itself prevent that). > > > > If it's based on the fact that we'll set PageDirty for file-backed, then it > > looks okay, but not usre. > > > > One thing to mention is pte_mkdirty() also counts in soft dirty by nature. I'm > > imagining a program that was soft-dirty tracked and somehow using UFFDIO_COPY > > as the major data filler (so the task itself may not write to the page directly > > hence HW won't set dirty bit there). If with pte_mkdirty the other userspace > > tracker with soft-dirty can still detect this, while with PageDirty I believe > > it can't. From that POV I'm not sure whether I can say that as proactively > > doing pte_mkdirty is a safer approach just in case such an use case exist, as > > myself can't say they're illegal, so pte_dirty is a superset of PageDirty not > > vice versa. > > And this is not persuasive either: without much deeper analysis > (which I'll decline to do!), it's impossible to tell whether an excess of > pte_mkdirty()s is good or bad for the hypothetical uffd+softdirty tracker: > you're guessing good, I'm guessing bad. > > How about a compromise (if you really want to continue with this patch): > you leave the SetPageDirty(page) in shmem_mfill_atomic_pte(), where I > feel a responsibility for it; but you do whatever works for you with > pte_mkdirty() at the mm/userfaultfd.c end? Sure. Duplicating dirty bit is definitely fine to me as it achieves the same goal as I hoped - we're still 100% clear we won't free a uffd page without being noticed, then that's enough to me for the goal of this patch. I won't initiate that NACK myself since I still think duplicating is unnecessary no matter it resides in shmem or uffd code, but please go ahead doing that and I'll be fine with it, just in case Andrew didn't follow the details. Actually, I can feel how strong opinion you're at this point on keeping the old way on this patch, and yes I definitely touched the shmem code (your preference has its reasoning; I have mine too and I had a preference on the other side, but I failed to persuade you..). So also feel free to NACK the whole patch with both the SetPageDirty and condition checks on pte_mkdirty(), I'm fine with it too. I can keep the conditions in my future series too. Not like the other zap_details patch where I believe I must need at some point, this patch is something I hoped a lot to have, but it's not a must. I know this "hole" already so I won't fall into it so hard the 3rd time (actually on the 2nd time I debugged faster and quickly noticed I just fell into this same hole I've used to fall). Let's wish the same to the others. > > (In the course of writing this, it has occurred to me that a much nicer > solution might be to delete mfill_atomic_install_pte() altogether, and > change the userfaultfd protocol so that handle_userfault() returns the > page supplied by ioctler, for process to map into its own userspace > in its usual way. But that's a big change, that neither of would be > keen to make; and it would not be surprising if it turned out actually > to be a very bad change - perhaps tried and abandoned before the "atomic" > functions were decided on. I wouldn't even dare mention it, unless that > direction might happen to fit in with something else you're plannng.) Dangling pages will be hard to make right, imho. E.g., please considering one thread faulted in with uffd-missing, we'll request UFFDIO_COPY and deliver the page to the faulted thread, then a 2nd thread faulted on the same address before the 1st faulted thread resolved the page fault in the pgtables. It'll start to be challenging from that point, imho. >From that POV I think current uffdio-copy is doing great on using pgtable and the pgtable lock to keep all these things very well serialized. We may have false positives as the 2nd faulted thread will generate a dup message, but that can be easily resolved by the user app. Thanks, -- Peter Xu