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 A1385CF9C72 for ; Tue, 24 Sep 2024 05:28:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0C3126B0082; Tue, 24 Sep 2024 01:28:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 072486B0083; Tue, 24 Sep 2024 01:28:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EA2646B0085; Tue, 24 Sep 2024 01:28:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id CC1896B0082 for ; Tue, 24 Sep 2024 01:28:57 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 6B6C5C17E4 for ; Tue, 24 Sep 2024 05:28:57 +0000 (UTC) X-FDA: 82598502714.28.EDBEEF1 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf30.hostedemail.com (Postfix) with ESMTP id 370888000C for ; Tue, 24 Sep 2024 05:28:54 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=none; spf=pass (imf30.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@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=1727155699; 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=kHrE2Z1EqhSgKEwE0N12BX+hY2lMxv9DsGJ7vKk1bos=; b=slG1ABXVnPa7tALOHKbwLCH4SnmUS7QMrP8B9L8MeBo+rLLuhTrSPj+OBmgEEhDwat+Dih gUVd2wFhv1B1xK/amvRju/XuFzq6OIaIbnLmTh2iigYNh0bbt33D9IG46ygPaMdA4oTaqk 7b1I4kinvU+VRRj6L7IlQhJD6DUKB0A= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=none; spf=pass (imf30.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727155699; a=rsa-sha256; cv=none; b=I0T/M0Zcv2OVSb5PmTcTpGvb4bMRjkBYKDPApuSahPxbcKwCBXs90rzUMxBsfySgMrfH70 vnDzQdgAiwW359OBMubaEQykcrHFDd4s4QCTuLgQZCe96cMGEAKA9CSYJ7ssKrcrHEPF4t KMeZGjclBswE9AwB/xthpnB8gdGVR6Q= 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 ACFA3DA7; Mon, 23 Sep 2024 22:29:22 -0700 (PDT) Received: from [10.163.37.113] (unknown [10.163.37.113]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 055823F6A8; Mon, 23 Sep 2024 22:28:49 -0700 (PDT) Message-ID: <0245d69d-8544-4068-9739-2d999c1fc599@arm.com> Date: Tue, 24 Sep 2024 10:58:45 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform From: Anshuman Khandual To: David Hildenbrand , linux-mm@kvack.org, akpm@linux-foundation.org Cc: Paul Walmsley , Palmer Dabbelt , Thomas Gleixner , Dave Hansen , Ryan Roberts , x86@kernel.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org References: <20240920053017.2514920-1-anshuman.khandual@arm.com> <4aad893a-f8ba-4b2e-bded-b01a35ec0a94@redhat.com> <6f178f3c-40fd-45f6-a380-a218429852ca@arm.com> Content-Language: en-US In-Reply-To: <6f178f3c-40fd-45f6-a380-a218429852ca@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: bd71i3nzhetr99pqpecbndcxdp94unqe X-Rspamd-Queue-Id: 370888000C X-Rspamd-Server: rspam11 X-HE-Tag: 1727155734-572741 X-HE-Meta: U2FsdGVkX19sHwiL3c3b6EqK481hBYi1jqvkHg6y804GLP575cAhIEHk+SuDYDTNIkpwhkNVfDUwZx3EBfNZFqp2l0j/IvANqa09PFK+PFLgVOWeCGy19mxtJZnxP1HufQl8ewYtwaXWMvgTtAV4yYr5mRnf1z5DpQJr1ovMQkVlzicv2ykkVUfqIyklEsWN3RCPdERCs1c4RpsSqLVA+MCyk8nL8Z23eTA2FUx5PQCnX9sj1qPjBliLp6N0XLEYOUAg+9jx4QnDawuMKGUHwIGRN65biF/S3hZkzxIocCE5wBRJ7xAQjN3ptsLXkDvrC7gMtpyH1pr3Z7nuloG0AaXh19jc3qA8U6lbWXs8+zuA4iqrX+qcwPf3xHmWOK/natPX/vhltDE0dyW0mRUy7cAIUzPr88NOTKJxTyh//B03eTO2BTYG5KL2lSaBKcqnb2Fo27hrp6uYE8XiEySnKV7vWP1NpFWMcc9h7mBjoi0xwnW/pJHUT1FvXEVi+j+KsLidBauZutTgzBQyJqKHtvq9eHJtsJZ78hL0wK7dMsfF9zNtKybDyRR4wGZ1e+bXm5/J1pYXCpwwtkxaCwk+O8aYO8s+kozIyW7Tfm+FlVGQOknSuNPQD7+RdtqPW6yWO1baRiFOv5UJS4VKiS7ezbWfhzsAW5ffIsRqnss81GeW/nbG7Quw0uHLV7SxtU5oe0y81mQWWk9BlXze1wQlSgFYLpEYgUBgZOfr69syGI6dRi7CGGg9xQckXxmTKepdG1wR/LKnsjSYiANFBZuSs65FFRYm/wQ4BhHT53U5AraloyNJlXx/cXhmiUMTCFEel+2PUqisi78vro2yvhdXCSYkryswxFhyT8twJF8je+ZqbS0zSuzSVU2hnEn7BSLh2c4n2aM775KDJTp3YPbUNJZHxI9yzmwKl0a/uiUXFn71yNW7mxyj7AtvdRD1USLiRnneqYuJ82KcvYlW1sA gd4tqHEZ 0a8+e4yNqK9/h0JhqPE/qf7NRfo2AzBYqNtZmAwbTjuscITfBRWHpb/f/H4pUrA+P1L8lsysbhGTeUerCJF+YGH5zcUW7PPZ8MqJMDhL0sUPs70vcTq7DM+BMLPAcNulq8vyEZ/ktlv5eSWGjtueM+KP0qzpcA5cTkIwLGSwsQXTtu9wI8g3mFWgrXqnUH1Y645+zOAPjIuF6N3rBXLNR8G9Yq70/VCuH0yBBlElXDmAENSQb0OiauL1ZY4BpnEP4Y6dENlT9ySO0zN26yIo3r+XD4gvomz+ZqY2KszNHv/KzUURiEMsHqJMJkQgRXKUrCGnoVIvL75YB/+bThPOqbqCLnkwlpSeNY9xR4oVR05Bxxk9lR9Y7m85hXgohczl/94nQjsjNSKQOOhxBiWHqGY0i/HZ8BSHHqeLypaiL4Nr3B2iDMQ3KXEY0s/uPFjYLBUza/VRe1bwfa0XnqCi4LTvzgi7fF3Ill3HWO4WWgYwDKcgH+EJWbemKV5Vdo+FhQSZ8EnaVtxyrmdXrPXMggR0xT08AIvRSep8j0ZUJrbhsAE8IdHeRu8Pk8QFs8OIzeDyjcmbb4Ofo2wcg8fBAqZw6UfqCyDRNX84ik7tPRr3AqnNMdY4DPBVSX28xmM5ikU6OlnYPQ2n8xrw= 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/20/24 14:12, Anshuman Khandual wrote: > > > On 9/20/24 12:09, David Hildenbrand wrote: >> On 20.09.24 07:30, Anshuman Khandual wrote: >>> set_pxd_safe() helpers that serve a specific purpose for both x86 and riscv >>> platforms, do not need to be in the common memory code. Otherwise they just >>> unnecessarily make the common API more complicated. This moves the helpers >>> from common code to platform instead. >>> >>> Cc: Paul Walmsley >>> Cc: Palmer Dabbelt >>> Cc: Thomas Gleixner >>> Cc: Dave Hansen >>> Cc: David Hildenbrand >>> Cc: Ryan Roberts >>> Cc: Andrew Morton >>> Cc: x86@kernel.org >>> Cc: linux-mm@kvack.org >>> Cc: linux-riscv@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Suggested-by: David Hildenbrand >>> Signed-off-by: Anshuman Khandual >>> --- >>>   arch/riscv/include/asm/pgtable.h | 19 ++++++++++++++++ >>>   arch/x86/include/asm/pgtable.h   | 37 +++++++++++++++++++++++++++++++ >>>   include/linux/pgtable.h          | 38 -------------------------------- >>>   3 files changed, 56 insertions(+), 38 deletions(-) >>> >>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h >>> index 089f3c9f56a3..39ca652c5ebe 100644 >>> --- a/arch/riscv/include/asm/pgtable.h >>> +++ b/arch/riscv/include/asm/pgtable.h >>> @@ -957,6 +957,25 @@ void misc_mem_init(void); >>>   extern unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]; >>>   #define ZERO_PAGE(vaddr) (virt_to_page(empty_zero_page)) >>>   +/* >>> + * Use set_p*_safe(), and elide TLB flushing, when confident that *no* >>> + * TLB flush will be required as a result of the "set". For example, use >>> + * in scenarios where it is known ahead of time that the routine is >>> + * setting non-present entries, or re-setting an existing entry to the >>> + * same value. Otherwise, use the typical "set" helpers and flush the >>> + * TLB. >>> + */ >>> +#define set_p4d_safe(p4dp, p4d) \ >>> +({ \ >>> +    WARN_ON_ONCE(p4d_present(*p4dp) && !p4d_same(*p4dp, p4d)); \ >>> +    set_p4d(p4dp, p4d); \ >>> +}) >>> + >>> +#define set_pgd_safe(pgdp, pgd) \ >>> +({ \ >>> +    WARN_ON_ONCE(pgd_present(*pgdp) && !pgd_same(*pgdp, pgd)); \ >>> +    set_pgd(pgdp, pgd); \ >>> +}) >>>   #endif /* !__ASSEMBLY__ */ >>>     #endif /* _ASM_RISCV_PGTABLE_H */ >>> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h >>> index e39311a89bf4..fefb52bb6b4d 100644 >>> --- a/arch/x86/include/asm/pgtable.h >>> +++ b/arch/x86/include/asm/pgtable.h >>> @@ -1701,6 +1701,43 @@ bool arch_is_platform_page(u64 paddr); >>>   #define arch_is_platform_page arch_is_platform_page >>>   #endif >>>   +/* >>> + * Use set_p*_safe(), and elide TLB flushing, when confident that *no* >>> + * TLB flush will be required as a result of the "set". For example, use >>> + * in scenarios where it is known ahead of time that the routine is >>> + * setting non-present entries, or re-setting an existing entry to the >>> + * same value. Otherwise, use the typical "set" helpers and flush the >>> + * TLB. >>> + */ >>> +#define set_pte_safe(ptep, pte) \ >>> +({ \ >>> +    WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \ >>> +    set_pte(ptep, pte); \ >>> +}) >>> + >>> +#define set_pmd_safe(pmdp, pmd) \ >>> +({ \ >>> +    WARN_ON_ONCE(pmd_present(*pmdp) && !pmd_same(*pmdp, pmd)); \ >>> +    set_pmd(pmdp, pmd); \ >>> +}) >>> + >>> +#define set_pud_safe(pudp, pud) \ >>> +({ \ >>> +    WARN_ON_ONCE(pud_present(*pudp) && !pud_same(*pudp, pud)); \ >>> +    set_pud(pudp, pud); \ >>> +}) >>> + >>> +#define set_p4d_safe(p4dp, p4d) \ >>> +({ \ >>> +    WARN_ON_ONCE(p4d_present(*p4dp) && !p4d_same(*p4dp, p4d)); \ >>> +    set_p4d(p4dp, p4d); \ >>> +}) >>> + >>> +#define set_pgd_safe(pgdp, pgd) \ >>> +({ \ >>> +    WARN_ON_ONCE(pgd_present(*pgdp) && !pgd_same(*pgdp, pgd)); \ >>> +    set_pgd(pgdp, pgd); \ >>> +}) >>>   #endif    /* __ASSEMBLY__ */ >> >> I'm wondering if we can completely get rid of these, for example via: >> >> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c >> index d8dbeac8b206..bc71c25930bb 100644 >> --- a/arch/x86/mm/init_64.c >> +++ b/arch/x86/mm/init_64.c >> @@ -79,10 +79,8 @@ DEFINE_POPULATE(pmd_populate_kernel, pmd, pte, init) >>  static inline void set_##type1##_init(type1##_t *arg1,         \ >>                         type2##_t arg2, bool init)              \ >>  {                                                              \ >> -       if (init)                                               \ >> -               set_##type1##_safe(arg1, arg2);                 \ >> -       else                                                    \ >> -               set_##type1(arg1, arg2);                        \ >> +       WARN_ON_ONCE(init && ##type1##_present(*arg1) && !##type1##_same(*arg1, arg2)); \ >> +       set_##type1(arg1, arg2);                                \ >>  } >>   >> >> We might be able to handle the pgd_populate etc part similarly, possibly getting >> rid of the pgd_populate_safe etc as well. >> >> Assuming I don't miss anything important :) > > Sounds feasible but will just leave that upto the x86 platform folks to > change later on, after this patch which just moves these helpers inside > the platform code. > >> >> Ideally, we get rid of the macros here and just use inline functions ... >> > > Sure, makes sense. Will change these as inline functions. > > -#define set_pte_safe(ptep, pte) \ > -({ \ > - WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \ > - set_pte(ptep, pte); \ > -}) > +static inline void set_pte_safe(pte_t *ptep, pte_t pte) > +{ > + WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); > + set_pte(ptep, pte); > +} > This has hit a road block in converting these macros as static inline functions as suggested earlier, because pmd/pud/p4d/pgd_same() macros are defined in generic header include/linux/pgtable.h, but way after gets included. I guess then the current patch should be left as it is.