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 ED742C6FD1F for ; Tue, 26 Mar 2024 16:48:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7F9DE6B0098; Tue, 26 Mar 2024 12:48:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A9D86B009B; Tue, 26 Mar 2024 12:48:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6722B6B009C; Tue, 26 Mar 2024 12:48:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 573026B0098 for ; Tue, 26 Mar 2024 12:48:52 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 2C7AE80AF3 for ; Tue, 26 Mar 2024 16:48:52 +0000 (UTC) X-FDA: 81939774504.01.502B936 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf04.hostedemail.com (Postfix) with ESMTP id 47F4740010 for ; Tue, 26 Mar 2024 16:48:50 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=none; spf=pass (imf04.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711471730; a=rsa-sha256; cv=none; b=AoophPXb6gOekTolrCRpRuuZ5DHx8eNDEQyVh5GGMyFHVzYKTqhhfyKEeLKaGOqGzRlAR8 dkC4t08AojpWG05Id3hprAbjCvxSRXUaMWPKfk0eiM03BpbjS2HlwO/9rUWB3Bq08YTlVY 6Y/aA4GjKQbY6dZRfFB56E9FTFCqmus= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=none; spf=pass (imf04.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711471730; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dulVK/FwNF20i7tg1KimhCC74HYS54dkpyuQZQJsLiQ=; b=g3zOeqbJlciHI8Dw6Xs4SwDrSlrxruWx5w5mjsW4M/kZRi4KxP0IbNmKkyQOVhzqhPzkbc Z6DivfitPBtV+iRdU8/uEbnk0AFBoLkhUjsaH9PprlocdMEu1a/XOeoxiKzoizvfRcA/r4 07iVNTwfrXec2xXvUgxNC1mLKA795z8= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E5E172F4; Tue, 26 Mar 2024 09:49:22 -0700 (PDT) Received: from [10.1.29.179] (XHFQ2J9959.cambridge.arm.com [10.1.29.179]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 69D4D3F694; Tue, 26 Mar 2024 09:48:47 -0700 (PDT) Message-ID: <79ade347-419a-4c9e-84db-def06ec5f36a@arm.com> Date: Tue, 26 Mar 2024 16:48:45 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v1 2/4] mm/gup: Use ptep_get_lockless_norecency() Content-Language: en-GB To: David Hildenbrand , Mark Rutland , Catalin Marinas , Will Deacon , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , Andrew Morton , Muchun Song Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240215121756.2734131-1-ryan.roberts@arm.com> <20240215121756.2734131-3-ryan.roberts@arm.com> <5d80c368-7ce7-4a44-9cd7-aee3e1c9182b@redhat.com> From: Ryan Roberts In-Reply-To: <5d80c368-7ce7-4a44-9cd7-aee3e1c9182b@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 47F4740010 X-Stat-Signature: i37ja6xu7yz8i51hx941cy5pg7bscs3b X-HE-Tag: 1711471730-902046 X-HE-Meta: U2FsdGVkX1/54WihSEaQGUZSnYXqRT4XZie3EHJuZvZoX4TEfmDhAmNW8ZOjUK6gqj1lK91v6sphsU3GXjEEtDcAlhDGUqfLk3VCvbH1qYHxnZ6aRDX8dBx+5F0Fn3u1xyixJCNnXylAn4AGtIlWeZD2oJ63vMz99CPQA4FVYK0QNkttsbcmF9QeH/gn0pVDmF69OCffsK4sEC+hDG+MKCHhtvZqIyFUXEAclRDe2aqSy2Mj2FCVse5FaZn5P3wVbnIE1X3yD9uMx4yIg3xLNuAfcnPlJ8TAISp/TTciiIGRlHlIcILhZXC8JIyG8MRLhkDigJ+Vj2xob2tpnPoxbvvydfkylqU5B026LzwDGb0HwvbtjyImuyNZU7nGN/621awd0HuKM0xsYjsALkGVwYIsc+elfO8RauT5ExlSJhnTkwjFqca6+1hVTdPVA+11Aim3RRnQ9IA6rJT9XnThPk+PKVyfe9IzIOTvkwTIBuI23CjSklHWJM7PKw6c/htHDwL2ZzVbByK+iYTfV/JTJCjSHPly1tKdNGdXaQPy4f9DVoWO6ZUuK+OG8YgAqMxo/WtHGtT+k37U1cSknxOBYsW0DxoDRKCjlwAJFhHtjBkf3/CaelpYgxJprvUKSW0XkB3xFgeVrhIlQT0J7G7+c8WkQI56xnFsx9tXsoql/NOeHdXdH4Ci93Bg5w7JaKUUDxOYms2bh6Yua5Mvu+QTnkIweeHnaX9lBxsB3PggNrn6wpy02H2No++Frt+JQHqOa/wJmt2NvX14p5z46bqibKx0vJuyKNW9lxR3V01YehT5XfPcg+A8LkSSkUiu8biCvshMCqVZ+2Y8J+HXwuiBMMokoXmJ/b2fZ6ndL8ZtmTedbmR3TjWtqXx21m0fyF8tqNK8qhe/J9IcygNJ01OEyOv2Amg6EviQIcFpfHCBMI1wK23LAGBrcOdkE2bqDXCsddkSx0NyIv1kA/48jnO 7mfWEayK HKvVAguG6aLi6Fi6BOA4YzG6QjCfbzH+uQ2m/1qOhsLwX/IA/VMy9OEYAXXm6PE9n3gWfO5QmpOGqhTTviLDR5m5AGl2BKJ2YgLhcex5M30XMiiF0LSlHDqXPCYMknuFfYBeK5E6+p84+9hfGuYxT9OMcThSACApNXquU5ihaobG6eU8Z/BPaEoXraMxKZN1skFXDHnTeg3ii8r+T/Hggno4/QsZR4QaPrKT6T/FrG73gBFVMGtParAFPFe4jsjMDGPeen8QxV9L27FIWYqQBz+pJP7RlxoZQIHQkDfHWmLA6xPQ= 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: List-Subscribe: List-Unsubscribe: On 26/03/2024 16:30, David Hildenbrand wrote: > On 15.02.24 13:17, Ryan Roberts wrote: >> Gup needs to read ptes locklessly, so it uses ptep_get_lockless(). >> However, the returned access and dirty bits are unimportant so let's >> switch over to ptep_get_lockless_norecency(). >> >> The wrinkle is that gup needs to check that the pte hasn't changed once >> it has pinned the folio following this model: >> >>      pte = ptep_get_lockless_norecency(ptep) >>      ... >>      if (!pte_same(pte, ptep_get_lockless(ptep))) >>              // RACE! >>      ... >> >> And now that pte may not contain correct access and dirty information, >> the pte_same() comparison could spuriously fail. So let's introduce a >> new pte_same_norecency() helper which will ignore the access and dirty >> bits when doing the comparison. >> >> Note that previously, ptep_get() was being used for the comparison; this >> is technically incorrect because the PTL is not held. I've also >> converted the comparison to use the preferred pmd_same() helper instead >> of doing a raw value comparison. >> >> As a side-effect, this new approach removes the possibility of >> concurrent read/write to the page causing a spurious fast gup failure, >> because the access and dirty bits are no longer used in the comparison. >> >> Signed-off-by: Ryan Roberts >> --- > > [...] > >>   #ifndef __HAVE_ARCH_PTE_UNUSED >>   /* >>    * Some architectures provide facilities to virtualization guests >> diff --git a/mm/gup.c b/mm/gup.c >> index df83182ec72d..0f96d0a5ec09 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -2576,7 +2576,7 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, >> unsigned long addr, >>       if (!ptep) >>           return 0; >>       do { >> -        pte_t pte = ptep_get_lockless(ptep); >> +        pte_t pte = ptep_get_lockless_norecency(ptep); >>           struct page *page; >>           struct folio *folio; >> >> @@ -2617,8 +2617,9 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, >> unsigned long addr, >>               goto pte_unmap; >>           } >> >> -        if (unlikely(pmd_val(pmd) != pmd_val(*pmdp)) || >> -            unlikely(pte_val(pte) != pte_val(ptep_get(ptep)))) { >> +        if (unlikely(!pmd_same(pmd, *pmdp)) || >> +            unlikely(!pte_same_norecency(pte, >> +                    ptep_get_lockless_norecency(ptep)))) { >>               gup_put_folio(folio, 1, flags); >>               goto pte_unmap; > > We pass the pte into pte_access_permitted(). It would be good to mention that > you checked all implementations. TBH, I hadn't; I decided that since the "inaccurate access/dirty bits" was only possible on arm64, then only arm64's implementation needed checking. But given your comment, I just had a quick look at all impls. I didn't spot any problems where any impl needs the access/dirty bits. I'll add this to the commit log. > > Acked-by: David Hildenbrand >