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 56FF0C77B7E for ; Tue, 2 May 2023 17:34:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D628A6B0075; Tue, 2 May 2023 13:34:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CEC236B0078; Tue, 2 May 2023 13:34:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B65AE6B007B; Tue, 2 May 2023 13:34:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) by kanga.kvack.org (Postfix) with ESMTP id 643E26B0075 for ; Tue, 2 May 2023 13:34:18 -0400 (EDT) Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-3f1cfed93e2so41057445e9.3 for ; Tue, 02 May 2023 10:34:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683048857; x=1685640857; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=3UgLddjkHIg2a43YemgHaCt+QBmXeXskji4qIT5rtw8=; b=Ao5JI1alD7DgI+d6pCbXv0LEtRlV3U84XxStQ9NleMu8V3R9GYgBnVWm8YoSAm71Wa ntGorG7vEZGvwySwKgSU+FLhXKtfU5/fd7yY1Xazt8ZYJoKTUTNk8+zllRlulgFcSoT5 Acthh5do5CP+EjXcnNOHCIzvFr49g2Lo75XuSe/t9Uup4GIphVF9JV5+ryxpbr4BDMdS kLkBMMTQRh2So04dLmKVXOiBw683mIUB2gQR14AMDnBnrRidhTKwECAhsAv2riseoeLy HDsDwvjLuxRKVX7odLIm5oijlEHVqccUX1MpFrga45j8iJcLePlh9GpxVRIvO+UVw4zm 6LSw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683048857; x=1685640857; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=3UgLddjkHIg2a43YemgHaCt+QBmXeXskji4qIT5rtw8=; b=Tgo0xQldaALWNLYiHx8nVLVMduU1UflgkP9Sx6dK4aYo1idRKwcMTlJGDBjqVjBHnv en2Wusc9MkUtXoPhW5ZMqR8ZZSeDNsNNbtYm+LUvdH2hSTdkPIVxvd2FPhNrLxVYRLoG 7BGxZhlfh197dizRVOtWds1Kqi/hNY22+lyR09+CUwBqlVnh1BuIei55mLr82Zo23MrN SWKhPccho2311eVYEjKno/BQqqsp1oG97+rr9nseE9/B6LpP4Lob9QqYt+ArSodr6Ql0 9hpJU5Oj3pYLAViyCDEV6nJ5h7HqEUBTyJ7ygpLrESqIH2Q4xOmJOwnH+EUyjJvohaoL 44rw== X-Gm-Message-State: AC+VfDx1CZeveoQ6rVaOWqA3r6rCZ8Ah/SwdYutbbleXcTbGo8etaQ0Y 7onHUFBsWC/t4YGXYMRjtog= X-Google-Smtp-Source: ACHHUZ7b85L6+LUsBFJD+tnhEjxFFCRDnSzY0aCIqOHHgnTb5/okLOWnD8QQ17TrR/t6156C/0fZWQ== X-Received: by 2002:a05:600c:21c8:b0:3f1:745d:821 with SMTP id x8-20020a05600c21c800b003f1745d0821mr12599496wmj.23.1683048857363; Tue, 02 May 2023 10:34:17 -0700 (PDT) Received: from localhost (host86-156-84-164.range86-156.btcentralplus.com. [86.156.84.164]) by smtp.gmail.com with ESMTPSA id m1-20020a7bca41000000b003f179fc6d8esm35912012wml.44.2023.05.02.10.34.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 May 2023 10:34:16 -0700 (PDT) Date: Tue, 2 May 2023 18:34:16 +0100 From: Lorenzo Stoakes To: Peter Zijlstra Cc: David Hildenbrand , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Jason Gunthorpe , Jens Axboe , Matthew Wilcox , Dennis Dalessandro , Leon Romanovsky , Christian Benvenuti , Nelson Escobar , Bernard Metzler , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , Bjorn Topel , Magnus Karlsson , Maciej Fijalkowski , Jonathan Lemon , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Christian Brauner , Richard Cochran , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , linux-fsdevel@vger.kernel.org, linux-perf-users@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org, Oleg Nesterov , Jason Gunthorpe , John Hubbard , Jan Kara , "Kirill A . Shutemov" , Pavel Begunkov , Mika Penttila , Dave Chinner , Theodore Ts'o , Peter Xu , Matthew Rosato , "Paul E . McKenney" , Christian Borntraeger , Mike Rapoport Subject: Re: [PATCH v7 3/3] mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings Message-ID: References: <1691115d-dba4-636b-d736-6a20359a67c3@redhat.com> <20230502172231.GH1597538@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230502172231.GH1597538@hirez.programming.kicks-ass.net> 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, May 02, 2023 at 07:22:32PM +0200, Peter Zijlstra wrote: > On Tue, May 02, 2023 at 07:13:49PM +0200, David Hildenbrand wrote: > > [...] > > > > > +{ > > > + struct address_space *mapping; > > > + > > > + /* > > > + * GUP-fast disables IRQs - this prevents IPIs from causing page tables > > > + * to disappear from under us, as well as preventing RCU grace periods > > > + * from making progress (i.e. implying rcu_read_lock()). > > > + * > > > + * This means we can rely on the folio remaining stable for all > > > + * architectures, both those that set CONFIG_MMU_GATHER_RCU_TABLE_FREE > > > + * and those that do not. > > > + * > > > + * We get the added benefit that given inodes, and thus address_space, > > > + * objects are RCU freed, we can rely on the mapping remaining stable > > > + * here with no risk of a truncation or similar race. > > > + */ > > > + lockdep_assert_irqs_disabled(); > > > + > > > + /* > > > + * If no mapping can be found, this implies an anonymous or otherwise > > > + * non-file backed folio so in this instance we permit the pin. > > > + * > > > + * shmem and hugetlb mappings do not require dirty-tracking so we > > > + * explicitly whitelist these. > > > + * > > > + * Other non dirty-tracked folios will be picked up on the slow path. > > > + */ > > > + mapping = folio_mapping(folio); > > > + return !mapping || shmem_mapping(mapping) || folio_test_hugetlb(folio); > > > > "Folios in the swap cache return the swap mapping" -- you might disallow > > pinning anonymous pages that are in the swap cache. > > > > I recall that there are corner cases where we can end up with an anon page > > that's mapped writable but still in the swap cache ... so you'd fallback to > > the GUP slow path (acceptable for these corner cases, I guess), however > > especially the comment is a bit misleading then. > > > > So I'd suggest not dropping the folio_test_anon() check, or open-coding it > > ... which will make this piece of code most certainly easier to get when > > staring at folio_mapping(). Or to spell it out in the comment (usually I > > prefer code over comments). > > So how stable is folio->mapping at this point? Can two subsequent reads > get different values? (eg. an actual mapping and NULL) > > If so, folio_mapping() itself seems to be missing a READ_ONCE() to avoid > the compiler from emitting the load multiple times. Yes that actually feels like a bit of a flaw in folio_mapping(). I suppose we run the risk of mapping being reset (e.g. to NULL) even if any mapping we get being guaranteed to be safe due to the RCU thing. Based on David's feedback I think I'll recode to something more direct where we READ_ONCE() the mapping, then check mapping flags for anon, avoid the swap case + check shmem.