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 A7F52CF58F1 for ; Fri, 20 Sep 2024 08:42:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1834B6B0082; Fri, 20 Sep 2024 04:42:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 132F56B0083; Fri, 20 Sep 2024 04:42:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 020AB6B0085; Fri, 20 Sep 2024 04:42:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D802C6B0082 for ; Fri, 20 Sep 2024 04:42:51 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 485B3140172 for ; Fri, 20 Sep 2024 08:42:51 +0000 (UTC) X-FDA: 82584476142.16.E1F9715 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf28.hostedemail.com (Postfix) with ESMTP id 0CFB8C000E for ; Fri, 20 Sep 2024 08:42:47 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; spf=pass (imf28.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=1726821654; 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=CGCp17AT3N8uPG2cIQPm92OAmRPhjyDBF+wGK9J4oy8=; b=iTO6O/vHEAsR181pbiAPaJkg9hjPUKvTGyCjNU93yVutSZeW9OrMgL85Ejt/0eN+KMHfTX FfJaVQIfuXd22LcMFND8zvBjRqtiz3sEGRGyvl0W8ZMOd9Ss/DDAJWhrqTKl/0qipc4c5R j4N6t9+e0BaxJChlh2kRzyMpxMH2gF4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1726821654; a=rsa-sha256; cv=none; b=upj5gmiLMsjFS0JhlzL2HNsrNpuFDdfYwp9x7LzOGOt0ZkoKpXQ8ryMVo1cH7218VrRRgH UmdDIoMJtNX3d09Gq3zf5nkZeVzjqQ84dhgOITH0DClcW6uLDA+XP/FImPE3GKD8jNc6kd OBhv4o83DgQ3ZMcYo+9BuGkOUul7dnI= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; spf=pass (imf28.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 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 8D4BE1007; Fri, 20 Sep 2024 01:43:16 -0700 (PDT) Received: from [10.163.35.184] (unknown [10.163.35.184]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2611A3F64C; Fri, 20 Sep 2024 01:42:42 -0700 (PDT) Message-ID: <6f178f3c-40fd-45f6-a380-a218429852ca@arm.com> Date: Fri, 20 Sep 2024 14:12:40 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform 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> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <4aad893a-f8ba-4b2e-bded-b01a35ec0a94@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Stat-Signature: w66ookfpo7hktkiac1a76hup5gtfiwp9 X-Rspamd-Queue-Id: 0CFB8C000E X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1726821767-434819 X-HE-Meta: U2FsdGVkX18Ql6LFwR9GwfhBHISi7d2NF13jW9jjO9Ie7Zx0E4mTSk9tUlZPWaUGvQuLCJvf3APLIe4QPxOK/YvCsVneGzaAx22dlPsUwLX4bP4PDW2KrGxkUNleoG1jgrqCTKsuquXCJC3TwotmK+wb14yLyfxq4+kzbWi5pBvEowqeM8D0HIX3uy8te0pwsCGTD30m3jl4JAyUV0tSsSvUs1Yzjphw8oLYozSYSjIQe2OjXodiepXXqFXKEKMTEx5oUrXEE9OmNFLj2SHRHMfLDXakDH7bwNpbeE6CrLdgdNCMc3EId5IebcUHLNP8XXYLtH76KEdbRenT7j9zO/J8CeYLnUbGvV1w0rbS0UvbbhJzgpL2IlHa5ZILyIU8Qf+SJ3R6VZBeELCDqaiGS/LNTD3Zhc4zIJrxo6ZS34pl/BWX/Mm/uHQyHu/yAJQw7F+XNMBPHhOFMeEwPfa+ogXrDUOwwkinsl9fOfGfTnErXHM3Y2KMFIS3MoV7K1YuQvtgH7xSeepTwAoYnEm59FpHcJ315Unj6orG4lL89azW/qTrgaMGG6fTqknPEgTjODnI9fYy0UInT+ixL/Vl3HMRuLJdUUedkEkFHH9g5339XeZR9Sc/bDOz9GEyfFch4+sjTGZaS5pRfTLcfv/hplbNHZpwWgOZIyoTKvAcNEPmBAY9gIr+/Ks7T6C6FcOSR7JKob9PV6UJXmJbKN/LLVm1yheZ4qMTfjBYGq05h3eXIAVv9vY9ot/nqsI8LsPZnw7MbdvJhYVOgWmW5NIUbvUhaVEBgrQ458cRRNSpGTGCMHH1+Rxkhb+SwcTtt8onginAcT+v+Ci+tkV5FaRGzXfLdYLpfbKj0nBtl5QsvcdeaV9g9x1RFifxH16KtbIn8mZjz8M23cP9nEoNA9QlucIY1gPCihTm3hOd3dB1/AUcSfh0E/FFzWhY5ti+H7Y26G7mq6F85XHcjWqhgOI Tgq/RpbH GDf1ljBkSPMPKOCn8YTk7Fc/MV1DUhI0eoSAsSeNBREarPwfIN/1cmUOULCH4WFPa3nojNUCkdm8s1VTSv5t4mWRfSjpukari1DjXon2enwkDMeyd8X/El9Xk6i6Zfvtu9fPXeyIqb0otAcvoYDZl8mm/Fnj8OasiU/A5jrLSxg6hD8y++zqLstqSSO3fYGHs7bjE5BkTDXTs32LRE7wDpP/5YLqIKg+B3rjR5Z0sKNQRcSedwdtYohjHhTBkIMkyzys2mCK832rGcYXCWkHjrXmmvZNSv60iE/r45tkkfja+RGV4CRoQIZAmZWLP5oxfK9E3OpoaHQwxD0H+vrs/CLn0VpZpEUj3NU1CclkgRSHvtJZpnhZ+9L0Ofy339LmxUwBBW8UAEBQrKKa6KFcJAt4KojjSodor+ApkulT7p/K/BGJQTYSxn5rV5N8CfRcoBDcEOqQOEksSuUBkiSkLrWpoJWa00iImr2WWIKeMeFNM1KJ7cy61OWNoCngku5HlzsRJeb6kQUxFFu+aTUz6iVmbe+m67xKSqPY31Hrr2aNYwGV2GJs+kh86caILixzzY/WTjB1uzGyOF/Bz90+P1Ec7r4wkxh7+lXu25Qk3rQxd97w2hdqLqui2mT4cboDuq7iB0pPEOvVuh44= 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 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); +}