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=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL 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 6CAC2C433ED for ; Tue, 27 Apr 2021 16:57:54 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0F82A60240 for ; Tue, 27 Apr 2021 16:57:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0F82A60240 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7B6976B0075; Tue, 27 Apr 2021 12:57:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 73FF86B0078; Tue, 27 Apr 2021 12:57:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5929E6B0082; Tue, 27 Apr 2021 12:57:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0043.hostedemail.com [216.40.44.43]) by kanga.kvack.org (Postfix) with ESMTP id 384486B0075 for ; Tue, 27 Apr 2021 12:57:53 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id E4AAD181AF5EA for ; Tue, 27 Apr 2021 16:57:52 +0000 (UTC) X-FDA: 78078753984.25.8A023B0 Received: from mail-il1-f171.google.com (mail-il1-f171.google.com [209.85.166.171]) by imf10.hostedemail.com (Postfix) with ESMTP id 4D99640002E6 for ; Tue, 27 Apr 2021 16:57:42 +0000 (UTC) Received: by mail-il1-f171.google.com with SMTP id i22so7299130ila.11 for ; Tue, 27 Apr 2021 09:57:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=AfWvlXdic+4MOs0KOPuGPjE9kmD8XQr/sjmlrkTPLaI=; b=V8ZOgRJcn6Dtqzn/dNTeT1u9mccvCFMWItwlB2UU5IVebFRpf/CJ71oNx7sVg0gTxn D5QqGQXHstJUFYUStJ6czBmZ5CVbVKd68zvdjnQN0+nn6k8spWRQg3ZN7hButLBVmrkd P1MDMudaGLf3+9zuwY8ZNeRQKEyHpqW3V3aCC0uIRCK/1HCaAzUuTyKbou4oHr50bPfa YPzAtJHZwnrp7epFgbmMoOkqVtlH4iiovgBMyUhDoYOdMTUgzCSkjgSkc05mSJZLjATs GXnDvOYHzpyTJUrMKD0nW67S6wLNYpiHRMkDClN/cndD1/qRlKFgvyLf7CYSGi8lGL66 jzVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=AfWvlXdic+4MOs0KOPuGPjE9kmD8XQr/sjmlrkTPLaI=; b=L3trbbIBcaG3UfD6rv7YTBvbOs+351Mhb647DwaXvkHXmIfFHWg7/x47BiiU0bY9Lm ydPFGOzgwuG4vSa3XcoUH9JYSLKNfJBwWdgt7uO8u57kmoxvj/+wQRAxArkj5gyfXJB9 Lrq2foZtvZzcfjA7VD9l126Rlm5Ay29kyjW/XoVFMlFOBb8iJ+rbwUF7chfykof8dqNe 2hpe+S7FAWyssTtFVgqNNdIOFe70TxtwWjKazT++dh8VLUaExJXHFx4tmXG6Q6b8iZhB 39zTXhzZX7ZKTUoJLOePmv5L/iHBbKvj0j3iDkAhIaixV/wsVPmhG7UlOMOBN1szi+yV QnVA== X-Gm-Message-State: AOAM530YMfhvO1DHP97jHvRErtvJS2nTvJr1oB9m1z0iwV48QmTRJcaV w0ogN/BY1icI9unQTzDDZKfxyxaLqMK46gkNKyacIg== X-Google-Smtp-Source: ABdhPJwGFKRq3YkbLr0YvaOXUWBlmiWFSZOzKAFBSpiZs1nWwxElMS0Tlq3jxLLjVUri2MfWpajIppi1lnvNcSqEz38= X-Received: by 2002:a05:6e02:1d06:: with SMTP id i6mr19319768ila.165.1619542671555; Tue, 27 Apr 2021 09:57:51 -0700 (PDT) MIME-Version: 1.0 References: <20210420220804.486803-1-axelrasmussen@google.com> <20210420220804.486803-4-axelrasmussen@google.com> <20210427155414.GB6820@xz-x1> In-Reply-To: <20210427155414.GB6820@xz-x1> From: Axel Rasmussen Date: Tue, 27 Apr 2021 09:57:16 -0700 Message-ID: Subject: Re: [PATCH v4 03/10] userfaultfd/shmem: support UFFDIO_CONTINUE for shmem To: Peter Xu Cc: Hugh Dickins , Alexander Viro , Andrea Arcangeli , Andrew Morton , Jerome Glisse , Joe Perches , Lokesh Gidra , Mike Kravetz , Mike Rapoport , Shaohua Li , Shuah Khan , Stephen Rothwell , Wang Qing , linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, LKML , linux-kselftest@vger.kernel.org, Linux MM , Brian Geffon , "Dr . David Alan Gilbert" , Mina Almasry , Oliver Upton Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 4D99640002E6 X-Stat-Signature: pgf7c5zzrwzd44iju598d1sfreyktcmy Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf10; identity=mailfrom; envelope-from=""; helo=mail-il1-f171.google.com; client-ip=209.85.166.171 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1619542662-178002 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: I'd prefer to keep them separate, as they are not tiny patches (they are roughly +200/-150 each). And, they really are quite independent - at least in the sense that I can reorder them via rebase with no conflicts, and the code builds at each commit in either orientation. I think this implies they're easier to review separately, rather than squashed. I don't have a strong feeling about the order. I slightly prefer swapping them compared to this v4 series: first introduce minor faults, then introduce CONTINUE. Since Peter also has no strong opinion, and Hugh it sounds like you prefer it the other way around, I'll swap them as we had in some previous version of this series: first introduce minor faults, then introduce CONTINUE. On Tue, Apr 27, 2021 at 8:54 AM Peter Xu wrote: > > On Mon, Apr 26, 2021 at 07:19:58PM -0700, Hugh Dickins wrote: > > On Tue, 20 Apr 2021, Axel Rasmussen wrote: > > > > > With this change, userspace can resolve a minor fault within a > > > shmem-backed area with a UFFDIO_CONTINUE ioctl. The semantics for this > > > match those for hugetlbfs - we look up the existing page in the page > > > cache, and install a PTE for it. > > > > > > This commit introduces a new helper: mcopy_atomic_install_pte. > > > > > > Why handle UFFDIO_CONTINUE for shmem in mm/userfaultfd.c, instead of in > > > shmem.c? The existing userfault implementation only relies on shmem.c > > > for VM_SHARED VMAs. However, minor fault handling / CONTINUE work just > > > fine for !VM_SHARED VMAs as well. We'd prefer to handle CONTINUE for > > > shmem in one place, regardless of shared/private (to reduce code > > > duplication). > > > > > > Why add a new mcopy_atomic_install_pte helper? A problem we have with > > > continue is that shmem_mcopy_atomic_pte() and mcopy_atomic_pte() are > > > *close* to what we want, but not exactly. We do want to setup the PTEs > > > in a CONTINUE operation, but we don't want to e.g. allocate a new page, > > > charge it (e.g. to the shmem inode), manipulate various flags, etc. Also > > > we have the problem stated above: shmem_mcopy_atomic_pte() and > > > mcopy_atomic_pte() both handle one-half of the problem (shared / > > > private) continue cares about. So, introduce mcontinue_atomic_pte(), to > > > handle all of the shmem continue cases. Introduce the helper so it > > > doesn't duplicate code with mcopy_atomic_pte(). > > > > > > In a future commit, shmem_mcopy_atomic_pte() will also be modified to > > > use this new helper. However, since this is a bigger refactor, it seems > > > most clear to do it as a separate change. > > > > > > Signed-off-by: Axel Rasmussen > > > > If this "03/10" had been numbered 04/10, I would have said > > Acked-by: Hugh Dickins > > > > But I find this new ordering incomprehensible - I'm surprised that it > > even builds this way around (if it does): this patch is so much about > > what has been enabled in "04/10" (references to UFFDIO_CONTINUE shmem > > VMAs etc). > > > > Does Peter still think this way round is better? If he does, then we > > shall have to compromise by asking you just to squash the two together. > > Hi, Hugh, Axel, > > I have no strong opinion. To me, UFFDIO_CONTINUE can be introduced earlier like > this. As long as we don't enable the feature (which is done in the next patch), > no one will be able to call it, then it looks clean. Merging them also looks > good to me. > > Thanks, > > -- > Peter Xu >