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.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 94C6FC433DB for ; Tue, 2 Feb 2021 17:15:27 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1C0D764ECE for ; Tue, 2 Feb 2021 17:15:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1C0D764ECE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 625936B0071; Tue, 2 Feb 2021 12:15:26 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5AE876B0072; Tue, 2 Feb 2021 12:15:26 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 44EF16B0073; Tue, 2 Feb 2021 12:15:26 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0077.hostedemail.com [216.40.44.77]) by kanga.kvack.org (Postfix) with ESMTP id 2A0636B0071 for ; Tue, 2 Feb 2021 12:15:26 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id E157D1EE6 for ; Tue, 2 Feb 2021 17:15:25 +0000 (UTC) X-FDA: 77773979010.15.home51_1917450275cc Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin15.hostedemail.com (Postfix) with ESMTP id 85E761814B0C7 for ; Tue, 2 Feb 2021 17:15:25 +0000 (UTC) X-HE-Tag: home51_1917450275cc X-Filterd-Recvd-Size: 9188 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Tue, 2 Feb 2021 17:15:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612286121; 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=lEa8TASFowTmr9kKp6sPUUyPmHU/46QcAZCj48yANfk=; b=AZ0CfETHGRvFQCDRQSA0rgYSi7NvRdy+TEqi14WSeRJA8P6W9fW7OQeN8zhn+/5Xj9+LGR faYQjxjYr1XUjy5I4iaPGhKU01OMRJgPtIR0v8ZbWL0LHGWyX2ISwl3oX7+2pT2X/oxVgR nczncEjnX8HdN+w/a3TK4HpwSDjrXz4= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-13-1OMbFV4uP86521W3EzOxhw-1; Tue, 02 Feb 2021 12:15:19 -0500 X-MC-Unique: 1OMbFV4uP86521W3EzOxhw-1 Received: by mail-qk1-f198.google.com with SMTP id m9so17692806qka.22 for ; Tue, 02 Feb 2021 09:15:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=lEa8TASFowTmr9kKp6sPUUyPmHU/46QcAZCj48yANfk=; b=GFju06GX4YuORT2x8SKtHHfXb0ySm6POyVRs2HEcDq8u3xFvkUd+oEsTr1Rvj2+TWj 0YyI5msJ3ee78ignO8CruhOaGR2wlvO6xQSRfteFnKyGwy7S6HwyDM/jidmhk4LmfDMP 3Nvzh4vXCW/ewKuhykWQcnDy2v+xfGdN+7gBEoR2txwyG+FjyTq9WFZbi4GVXShIBRrq Kx8RNBtscX+nIy8rA+/+/NOiSf9GpUWhLodpRFBKOPEY//FHyzw9PMxE9TCqEkO4EV78 t3t/gBEUOyS4l7tCEjIjDSIozCCypV7V0hJHj4gXNI3lpaJMcbgXQtqC2LY2Fazmca5H ckBQ== X-Gm-Message-State: AOAM5329do6LPAXlRCSU8EK6LSs+tPELSNSCXpUiJUoekZvFTfE1fNQx fJCjYJQPrGAQaCYH9ffyfI3WjHK3SW8elqVACqaroB4JSk8pck9w6GQ+O/lwdfCQxWLwt9DnoeX kVO4boCJMfHs= X-Received: by 2002:a37:8b81:: with SMTP id n123mr22342733qkd.242.1612286118804; Tue, 02 Feb 2021 09:15:18 -0800 (PST) X-Google-Smtp-Source: ABdhPJzcppnC7lKM0JkQf/E3mgt/JhPqhdB0jg+3iHEp7UqjdhDN2d+ysu+mJtwxQ/4NGaXhYRD90Q== X-Received: by 2002:a37:8b81:: with SMTP id n123mr22342688qkd.242.1612286118538; Tue, 02 Feb 2021 09:15:18 -0800 (PST) Received: from xz-x1 (bras-vprn-toroon474qw-lp130-20-174-93-89-182.dsl.bell.ca. [174.93.89.182]) by smtp.gmail.com with ESMTPSA id p11sm17044941qtb.62.2021.02.02.09.15.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Feb 2021 09:15:17 -0800 (PST) Date: Tue, 2 Feb 2021 12:15:15 -0500 From: Peter Xu To: Axel Rasmussen Cc: Alexander Viro , Alexey Dobriyan , Andrea Arcangeli , Andrew Morton , Anshuman Khandual , Catalin Marinas , Chinwen Chang , Huang Ying , Ingo Molnar , Jann Horn , Jerome Glisse , Lokesh Gidra , "Matthew Wilcox (Oracle)" , Michael Ellerman , Michal =?utf-8?Q?Koutn=C3=BD?= , Michel Lespinasse , Mike Kravetz , Mike Rapoport , Nicholas Piggin , Shaohua Li , Shawn Anastasio , Steven Rostedt , Steven Price , Vlastimil Babka , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Adam Ruprecht , Cannon Matthews , "Dr . David Alan Gilbert" , David Rientjes , Oliver Upton Subject: Re: [PATCH v3 5/9] userfaultfd: add minor fault registration mode Message-ID: <20210202171515.GF6468@xz-x1> References: <20210128224819.2651899-1-axelrasmussen@google.com> <20210128224819.2651899-6-axelrasmussen@google.com> <20210201183159.GF260413@xz-x1> MIME-Version: 1.0 In-Reply-To: <20210201183159.GF260413@xz-x1> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=peterx@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline 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 Mon, Feb 01, 2021 at 01:31:59PM -0500, Peter Xu wrote: > On Thu, Jan 28, 2021 at 02:48:15PM -0800, Axel Rasmussen wrote: > > This feature allows userspace to intercept "minor" faults. By "minor" > > faults, I mean the following situation: > > > > Let there exist two mappings (i.e., VMAs) to the same page(s) (shared > > memory). One of the mappings is registered with userfaultfd (in minor > > mode), and the other is not. Via the non-UFFD mapping, the underlying > > pages have already been allocated & filled with some contents. The UFFD > > mapping has not yet been faulted in; when it is touched for the first > > time, this results in what I'm calling a "minor" fault. As a concrete > > example, when working with hugetlbfs, we have huge_pte_none(), but > > find_lock_page() finds an existing page. > > > > This commit adds the new registration mode, and sets the relevant flag > > on the VMAs being registered. In the hugetlb fault path, if we find > > that we have huge_pte_none(), but find_lock_page() does indeed find an > > existing page, then we have a "minor" fault, and if the VMA has the > > userfaultfd registration flag, we call into userfaultfd to handle it. > > When re-read, now I'm thinking whether we should restrict the minor fault > scenario with shared mappings always, assuming there's one mapping with uffd > and the other one without, while the non-uffd can modify the data before an > UFFDIO_CONTINUE kicking the uffd process. > > To me, it's really more about page cache and that's all.. > > So I'm wondering whether below would be simpler and actually clearer on > defining minor faults, comparing to the above whole two paragraphs. For > example, the scemantics do not actually need two mappings: > > For shared memory, userfaultfd missing fault used to only report the event > if the page cache does not exist for the current fault process. Here we > define userfaultfd minor fault as the case where the missing page fault > does have a backing page cache (so only the pgtable entry is missing). > > It should not affect most of your code, but only one below [1]. OK it could be slightly more than that... E.g. we'd need to make UFFDIO_COPY to not install the write bit if it's UFFDIO_CONTINUE and if it's private mappings. In hugetlb_mcopy_atomic_pte() now we apply the write bit unconditionally: _dst_pte = make_huge_pte(dst_vma, page, dst_vma->vm_flags & VM_WRITE); That'll need a touch-up otherwise. It's just the change seems still very small so I'd slightly prefer to support it all. However I don't want to make your series complicated and blocking it, so please feel free to still make it shared memory if that's your preference. The worst case is if someone would like to enable this (if with a valid user scenario) we'd export a new uffd feature flag. > > [...] > > > @@ -1302,9 +1301,26 @@ static inline bool vma_can_userfault(struct vm_area_struct *vma, > > unsigned long vm_flags) > > { > > /* FIXME: add WP support to hugetlbfs and shmem */ > > - return vma_is_anonymous(vma) || > > - ((is_vm_hugetlb_page(vma) || vma_is_shmem(vma)) && > > - !(vm_flags & VM_UFFD_WP)); > > + if (vm_flags & VM_UFFD_WP) { > > + if (is_vm_hugetlb_page(vma) || vma_is_shmem(vma)) > > + return false; > > + } > > + > > + if (vm_flags & VM_UFFD_MINOR) { > > + /* > > + * The use case for minor registration (intercepting minor > > + * faults) is to handle the case where a page is present, but > > + * needs to be modified before it can be used. This requires > > + * two mappings: one with UFFD registration, and one without. > > + * So, it only makes sense to do this with shared memory. > > + */ > > + /* FIXME: Add minor fault interception for shmem. */ > > + if (!(is_vm_hugetlb_page(vma) && (vma->vm_flags & VM_SHARED))) > > + return false; > > [1] > > So here we also restrict the mapping be shared. My above comment on the commit > message is also another way to ask whether we could also allow it to happen > with non-shared mappings as long as there's a page cache. If so, we could drop > the VM_SHARED check here. It won't affect your existing use case for sure, it > just gives more possibility that maybe it could also be used on non-shared > mappings due to some reason in the future. > > What do you think? > > The rest looks good to me. > > Thanks, > > -- > Peter Xu -- Peter Xu