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=-6.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 97956C4727C for ; Tue, 22 Sep 2020 19:11:21 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 148432371F for ; Tue, 22 Sep 2020 19:11:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="fSgjqGxh" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 148432371F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7B65E6B0055; Tue, 22 Sep 2020 15:11:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 718448E0003; Tue, 22 Sep 2020 15:11:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5B8C38E0001; Tue, 22 Sep 2020 15:11:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0158.hostedemail.com [216.40.44.158]) by kanga.kvack.org (Postfix) with ESMTP id 420D86B0055 for ; Tue, 22 Sep 2020 15:11:20 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id ED7D4180AD807 for ; Tue, 22 Sep 2020 19:11:19 +0000 (UTC) X-FDA: 77291640678.25.loaf13_1c01edb27150 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin25.hostedemail.com (Postfix) with ESMTP id C49931804E3A0 for ; Tue, 22 Sep 2020 19:11:19 +0000 (UTC) X-HE-Tag: loaf13_1c01edb27150 X-Filterd-Recvd-Size: 7246 Received: from mail-qv1-f65.google.com (mail-qv1-f65.google.com [209.85.219.65]) by imf01.hostedemail.com (Postfix) with ESMTP for ; Tue, 22 Sep 2020 19:11:19 +0000 (UTC) Received: by mail-qv1-f65.google.com with SMTP id p15so10089073qvk.5 for ; Tue, 22 Sep 2020 12:11:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=nJzybE70QTOPMpDnlJVwD/ZPuk6VG/Jsid2ppZBVbg8=; b=fSgjqGxhSdA2emNUxN5J3kJsAInQNePdNNyRlh0DibVU1PK6HY+XFWRbK0A+GZ8xd6 f/f5ds0qAFNBOg/U6Kubc4kxUul6pSup+XlKjeg6JTC8TFPClElz8ajgvmqV0c6nVXJm h0GaGzvAK1FAYVnCL5mi7mjykrgni4pNZFgFZP9suG42agx6xnJrNZgG7BIgD2SSGo8y JG+UNIjCh9Swt0JMp1rDO6dEczR0DW8MpWYWY/z+hV32QPbbaJW9gyBOzE7bwGU3u/HP wvFt40309MaHemZoodAJzlGd9DbvQwOhiAX/7GJRSrJojhwLigngsm4oqW9XcD/Gy3Ne bo1w== 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=nJzybE70QTOPMpDnlJVwD/ZPuk6VG/Jsid2ppZBVbg8=; b=Lu/1gBNB8ChnA0IYM9GDDl+1TrERcu3eiSifGYT6bom6PTMPp3STm4L+eZeu72EMYM oTCRfwNl6nHauSFkp/PMLSeZijWEVN8ESYYuXEzZYj1iOkPTIVP940qkUQ6bFr2UgUzf wmMksQknw3xEtbBVcDGuMU9q1a2ZG/DB+PqjtNXPdn26vQl8BbcN80mwY52PRU1ewW+K gwRpIICsasBYA9e0YSmE97cj1OhUtMsZc51dc8lmX0UEdH1qx5xan/c0epfTwO4D8IBP hWYo00hOTIv+Md3V3/jjLw2cUj36IasKRqkc5L+THRZJPt8wiZjTY+4l3xmdEW3DGLWJ xXqA== X-Gm-Message-State: AOAM532qLNJDNTzVg2mSlCfAexcx4YiQDOtMsfylF+kYKMOTjUp742Nz iNiM7IKxIRXcOVcrlgaNzysxeQ== X-Google-Smtp-Source: ABdhPJxgNsemk9TCKYufv5zWpiXrJqwTy+d9RJH+Ya2JMq9rnWECwJGQxJNp6HGFkTkZ5UY03xNq8w== X-Received: by 2002:a0c:c492:: with SMTP id u18mr7589580qvi.18.1600801878171; Tue, 22 Sep 2020 12:11:18 -0700 (PDT) Received: from ziepe.ca (hlfxns017vw-156-34-48-30.dhcp-dynamic.fibreop.ns.bellaliant.net. [156.34.48.30]) by smtp.gmail.com with ESMTPSA id e24sm11954844qka.76.2020.09.22.12.11.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 22 Sep 2020 12:11:17 -0700 (PDT) Received: from jgg by mlx with local (Exim 4.94) (envelope-from ) id 1kKnh6-003EqE-UY; Tue, 22 Sep 2020 16:11:16 -0300 Date: Tue, 22 Sep 2020 16:11:16 -0300 From: Jason Gunthorpe To: Peter Xu Cc: John Hubbard , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Jan Kara , Michal Hocko , Kirill Tkhai , Kirill Shutemov , Hugh Dickins , Christoph Hellwig , Andrea Arcangeli , Oleg Nesterov , Leon Romanovsky , Linus Torvalds , Jann Horn Subject: Re: [PATCH 1/5] mm: Introduce mm_struct.has_pinned Message-ID: <20200922191116.GK8409@ziepe.ca> References: <20200921211744.24758-1-peterx@redhat.com> <20200921211744.24758-2-peterx@redhat.com> <224908c1-5d0f-8e01-baa9-94ec2374971f@nvidia.com> <20200922151736.GD19098@xz-x1> <20200922161046.GB731578@ziepe.ca> <20200922175415.GI19098@xz-x1> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200922175415.GI19098@xz-x1> 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 22, 2020 at 01:54:15PM -0400, Peter Xu wrote: > diff --git a/mm/memory.c b/mm/memory.c > index 8f3521be80ca..6591f3f33299 100644 > +++ b/mm/memory.c > @@ -888,8 +888,8 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm, > * Because we'll need to release the locks before doing cow, > * pass this work to upper layer. > */ > - if (READ_ONCE(src_mm->has_pinned) && wp && > - page_maybe_dma_pinned(page)) { > + if (wp && page_maybe_dma_pinned(page) && > + READ_ONCE(src_mm->has_pinned)) { > /* We've got the page already; we're safe */ > data->cow_old_page = page; > data->cow_oldpte = *src_pte; > > I can also add some more comment to emphasize this. It is not just that, but the ptep_set_wrprotect() has to be done earlier. Otherwise it races like: pin_user_pages_fast() fork() atomic_set(has_pinned, 1); [..] atomic_read(page->_refcount) //false // skipped atomic_read(has_pinned) atomic_add(page->_refcount) ordered check write protect() ordered set write protect() And now have a write protect on a DMA pinned page, which is the invarient we are trying to create. The best algorithm I've thought of is something like: pte_map_lock() if (page) { if (wp) { ptep_set_wrprotect() /* Order with try_grab_compound_head(), either we see * page_maybe_dma_pinned(), or they see the wrprotect */ get_page(); if (page_maybe_dma_pinned() && READ_ONCE(src_mm->has_pinned)) { put_page(); ptep_clear_wrprotect() // do copy return } } else { get_page(); } page_dup_rmap() pte_unmap_lock() Then the do_wp_page() path would have to detect that the page is not write protected under the pte lock inside the fault handler and just do nothing. Ie the set/clear could be visible to the CPU and trigger a spurious fault, but never trigger a COW. Thus 'wp' becomes a 'lock' that prevents GUP from returning this page. Very tricky, deserves a huge comment near the ptep_clear_wrprotect() Consider the above algorithm beside the gup_fast() algorithm: if (!pte_access_permitted(pte, flags & FOLL_WRITE)) goto pte_unmap; [..] head = try_grab_compound_head(page, 1, flags); if (!head) goto pte_unmap; if (unlikely(pte_val(pte) != pte_val(*ptep))) { put_compound_head(head, 1, flags); goto pte_unmap; That last *ptep will check that the WP is not set after making page_maybe_dma_pinned() true. It still looks reasonable, the extra work is still just the additional atomic in page_maybe_dma_pinned(), just everything else has to be very carefully sequenced due to unlocked page table accessors. > I think the WRITE_ONCE/READ_ONCE can actually be kept, because atomic ops > should contain proper memory barriers already so the memory access orders > should be guaranteed I always have to carefully check ORDERING in Documentation/atomic_t.txt when asking those questions.. It seems very subtle to me, but yes, try_grab_compound_head() and page_maybe_dma_pinned() are already paired ordering barriers, so both the pte_val() on the GUP side and the READ_ONCE(has_pinned) look OK. Jason