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 52FFFCE8D61 for ; Thu, 19 Sep 2024 09:20:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CAA866B0085; Thu, 19 Sep 2024 05:20:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C5A7C6B0088; Thu, 19 Sep 2024 05:20:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B48796B0089; Thu, 19 Sep 2024 05:20:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 9791C6B0085 for ; Thu, 19 Sep 2024 05:20:10 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 3BEEF120BE5 for ; Thu, 19 Sep 2024 09:20:10 +0000 (UTC) X-FDA: 82580941380.14.2FB6B49 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf27.hostedemail.com (Postfix) with ESMTP id 3FF1E40014 for ; Thu, 19 Sep 2024 09:20:08 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf27.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1726737496; 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=Di6WOZpK0vZIlV10s5uClF60KLfnVE1RQxRILeythSs=; b=Q1D4VLuwyg92Q3sN4eJrkh08RK+xiPeeuMALMOuAQBYweG8TRSu1DrJZML4OTEpM2SnNIx jIBLl4tpJgNB82zRHJk/WGVklKCfiuMeYCMaaUhxbyGIsUSlzJeL4izgWt0920Wcm78eg1 CgjamqPAHKMYMCSwK4OELvtiY3Umtqo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1726737496; a=rsa-sha256; cv=none; b=mVI3iLR8f0sDIvbrPrOrGvPn7l4JlI6NoDjCBOkZN7uhns5P19a0IxTAmZ0mrYScGSkq1K xTAUn6Ym31GDLlIluhW5WqO17Ayek7Gbaz79SloRTGWjL+12YV1tmcmQ54zTMygbTOMasf xISrZOIOC80ala87LO4A+FMu35+FL6U= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf27.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com 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 A9BC71007; Thu, 19 Sep 2024 02:20:36 -0700 (PDT) Received: from [10.163.34.169] (unknown [10.163.34.169]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 402293F86F; Thu, 19 Sep 2024 02:20:02 -0700 (PDT) Message-ID: <3ac8c39c-e842-41e4-960a-6b41cd83848d@arm.com> Date: Thu, 19 Sep 2024 14:50:00 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V2 3/7] mm: Use ptep_get() for accessing PTE entries To: David Hildenbrand , linux-mm@kvack.org Cc: Andrew Morton , Ryan Roberts , "Mike Rapoport (IBM)" , Arnd Bergmann , x86@kernel.org, linux-m68k@lists.linux-m68k.org, linux-fsdevel@vger.kernel.org, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org References: <20240917073117.1531207-1-anshuman.khandual@arm.com> <20240917073117.1531207-4-anshuman.khandual@arm.com> <8cafe140-35cf-4e9d-8218-dfbfc156ca69@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 3FF1E40014 X-Stat-Signature: ez3pym8ngnjp6ksrb3dxd4p1kuw1rdn6 X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1726737608-352583 X-HE-Meta: U2FsdGVkX19rL60ZmvpAX8M42iWoewp58JWqO5bUQVt1YCxY3q7jTcv3gzKUBB/pw9Yn3zw/k50xIeFjzLTHNMfHyaygO9oLtxj/EJLqoofgsqycQYLIRtqZEq+FpV9UaEbxqgj0P+GKhg1x8Xt4J4hwGKycryXJdmgrAs1HkTj6SNDQNBOkikGVpcbiV9gxrIL02TmMyq2QSOVFYfINU3pbYjwGXLQU0hI95rM53dFofCl8xDw7knVJ8rfYy4SQNM+rMY+2FpA4IvD7EgFY78rAL6sfyFJJ/jBl6PELMqRvmR8lWnJPumnylE2PXOG6DlhwIcljkFbnF1dMLkr0GOkWAqTpoZ8mYGypEwkiT3t2qgmHJosCCaRvgEJ6FSfOZu1XEvKrjEwhhq9O7OLt+Ub/DxcaOnjHCkzNTsdO/Up3mkShUWO77XNdcavfmNarqZsU+zzBrJij6ZVHTIoCyJj5GeZ5H3zWKK8P4/grrIN+URXBs0gD/Axn2LiwuUv0ndxCuwkPFcF6KXPLI3j8gXNb1+aI5SjVReHm3xWSxQ5bdhNPT09A1O+PPn1wjrM0D6ngh1i2a+s7WJjxxBjiIhrcIFDAXbw228M+RBNibjpEAWT07TtRznxIS6heJLHepTE3uXzS5Qsu5wJyNtk4lIzBxEY2Q6t0gnvIq8c/2ZUPMgPZ4QVmQ+CYrgVYNVjod68iYXuG+iHcowKQMBMQC/7SADuGHY/YASclb3jlWq6wnuq9i9jj7r+bn0DDbBxCOz1b/OyHPZ/ruU8bxeq2hwYQacQO5JJp73J4KvxqGzFlSIn926Jfdf3olnugDiO1M1vUCFO9HI/0LrkNO5zWv/QnPWBrRPFOJmyNuMlI+K6XqdPAPeI1WB44hnmNzk7k9qwVt1wBtH/JSSBvjiJ1Fb5s43NcounsjEvSJa2R7XQdk6fX+sMm+d/1JDnb7MrA8JPflADCHLMGQBBYi35 oYQEu7B2 uvvrrU+X3WMvT4+UNPGgSA2dn0Tc10XKj7ussSpIoVfLTpm0+kkuzoCn/OrLvwB/Q60FE+HVzuHvJ1H+jDI6RcTtMfMPRpO6uJ7qo5ffy/8u8I8QSw5JnothhK0+2lQeRxOFYzCA+EVqCZPqF2KiUkd9kaAo1tweJ6IoLhm4RKNeaGucJgM7it3xAsZ9jC150a4L2M+kyB5vI3NBRdGf5bcjVSJJo9tLTuPU9MkukOZ0G3X7nXeTyFl+hbDYnUYOdeVSvCm9MlZYRD6uZlLr/OhBbVX6f5+3FnmmGMQXUBzBwnliL8Wbdxywoo+ygpz4vSRdAPXRzo8z4oS/g7kAWUnt1QCCFpJTjvfl5LXLupUpGTkF0d+gwEH1EQmpZjfTpaAK7kGlSGquklto7lz7KOw45Oz7Gjr2Rej0m9SPT338m/Uw5zxT2ylUjIWkLN6EuBKaiqhWqFrFInqSKn7xPcVQqeQZ6+yr1X+7b 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 9/19/24 13:34, David Hildenbrand wrote: > On 18.09.24 08:32, Anshuman Khandual wrote: >> >> >> On 9/17/24 15:58, David Hildenbrand wrote: >>> On 17.09.24 09:31, Anshuman Khandual wrote: >>>> Convert PTE accesses via ptep_get() helper that defaults as READ_ONCE() but >>>> also provides the platform an opportunity to override when required. This >>>> stores read page table entry value in a local variable which can be used in >>>> multiple instances there after. This helps in avoiding multiple memory load >>>> operations as well possible race conditions. >>>> >>> >>> Please make it clearer in the subject+description that this really only involves set_pte_safe(). >> >> I will update the commit message with some thing like this. >> >> mm: Use ptep_get() in set_pte_safe() >> >> This converts PTE accesses in set_pte_safe() via ptep_get() helper which >> defaults as READ_ONCE() but also provides the platform an opportunity to >> override when required. This stores read page table entry value in a local >> variable which can be used in multiple instances there after. This helps >> in avoiding multiple memory load operations as well as some possible race >> conditions. >> >>> >>> >>>> Cc: Andrew Morton >>>> Cc: David Hildenbrand >>>> Cc: Ryan Roberts >>>> Cc: "Mike Rapoport (IBM)" >>>> Cc: linux-mm@kvack.org >>>> Cc: linux-kernel@vger.kernel.org >>>> Signed-off-by: Anshuman Khandual >>>> --- >>>>    include/linux/pgtable.h | 3 ++- >>>>    1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h >>>> index 2a6a3cccfc36..547eeae8c43f 100644 >>>> --- a/include/linux/pgtable.h >>>> +++ b/include/linux/pgtable.h >>>> @@ -1060,7 +1060,8 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b) >>>>     */ >>>>    #define set_pte_safe(ptep, pte) \ >>>>    ({ \ >>>> -    WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \ >>>> +    pte_t __old = ptep_get(ptep); \ >>>> +    WARN_ON_ONCE(pte_present(__old) && !pte_same(__old, pte)); \ >>>>        set_pte(ptep, pte); \ >>>>    }) >>>>    >>> >>> I don't think this is necessary. PTE present cannot flip concurrently, that's the whole reason of the "safe" part after all. >> >> Which is not necessary ? Converting de-references to ptep_get() OR caching >> the page table read value in a local variable ? ptep_get() conversion also >> serves the purpose providing an opportunity for platform to override. > > Which arch override are you thinking of where this change here would make a real difference? Would it even make a difference with cont-pte on arm? As we figured out already this code is not used any where other than x86 platform. So changing this, won't make a difference for arm64 unless I am missing something. The idea behind the series is to ensure that, there are no direct de-referencing of page table entries in generic MM code and all accesses should go via available helpers instead. But if we move these set_pxd_safe() helpers into platform code as you have suggested earlier, those changes will not be necessary anymore. > >> >>> >>> Can we just move these weird set_pte/pmd_safe() stuff to x86 init code and be done with it? Then it's also clear *where* it is getting used and for which reason. >>> >> set_pte/pmd_safe() can be moved to x86 platform - as that is currently the >> sole user for these helpers. But because set_pgd_safe() gets used in riscv >> platform, just wondering would it be worth moving only the pte/pmd helpers >> but not the pgd one ? > > My take would be just to move them where they are used, and possibly even inlining them. > > The point is that it's absolutely underdocumented what "_safe" is supposed to be here, and I don't really see the reason to have this in common code (making the common API more complicated). Agreed, it makes sense for these helpers to be in the platform code instead where they get used (x86, riscv). Will move them as required.