* [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
@ 2024-09-20 5:30 Anshuman Khandual
2024-09-20 5:49 ` Dave Hansen
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Anshuman Khandual @ 2024-09-20 5:30 UTC (permalink / raw)
To: linux-mm, akpm
Cc: Anshuman Khandual, Paul Walmsley, Palmer Dabbelt,
Thomas Gleixner, Dave Hansen, David Hildenbrand, Ryan Roberts,
x86, linux-riscv, linux-kernel
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 <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
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 <david@redhat.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
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__ */
#endif /* _ASM_X86_PGTABLE_H */
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 2a6a3cccfc36..0bf88e505aad 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1050,44 +1050,6 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b)
}
#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); \
-})
-
#ifndef __HAVE_ARCH_DO_SWAP_PAGE
static inline void arch_do_swap_page_nr(struct mm_struct *mm,
struct vm_area_struct *vma,
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 5:30 [PATCH] mm: Move set_pxd_safe() helpers from generic to platform Anshuman Khandual
@ 2024-09-20 5:49 ` Dave Hansen
2024-09-20 6:42 ` Anshuman Khandual
2024-09-20 6:39 ` David Hildenbrand
2024-09-26 4:18 ` Anshuman Khandual
2 siblings, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2024-09-20 5:49 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
David Hildenbrand, Ryan Roberts, x86, linux-riscv, linux-kernel
On 9/19/24 22: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.
I just did a quick grep and don't see any difference between the _safe
and normal variants. A quick grep didn't turn up any actual users.
Did anyone actually double check that these are still needed on x86 in
the first place?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 5:49 ` Dave Hansen
@ 2024-09-20 6:42 ` Anshuman Khandual
2024-09-20 8:52 ` Dave Hansen
0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2024-09-20 6:42 UTC (permalink / raw)
To: Dave Hansen, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
David Hildenbrand, Ryan Roberts, x86, linux-riscv, linux-kernel
On 9/20/24 11:19, Dave Hansen wrote:
> On 9/19/24 22: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.
>
> I just did a quick grep and don't see any difference between the _safe
> and normal variants. A quick grep didn't turn up any actual users.
>
> Did anyone actually double check that these are still needed on x86 in
> the first place?
arch/x86/mm/init_64.c
#define DEFINE_ENTRY(type1, type2, 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); \
}
DEFINE_ENTRY(p4d, p4d, init)
DEFINE_ENTRY(pud, pud, init)
DEFINE_ENTRY(pmd, pmd, init)
DEFINE_ENTRY(pte, pte, init)
We had triggered a build problem after dropping off set_pte_safe()
which seemed not to be used after normal grep.
https://lore.kernel.org/linux-mm/202409131220.CJ5MlGCG-lkp@intel.com/
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 6:42 ` Anshuman Khandual
@ 2024-09-20 8:52 ` Dave Hansen
2024-09-20 9:27 ` Anshuman Khandual
0 siblings, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2024-09-20 8:52 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
David Hildenbrand, Ryan Roberts, x86, linux-riscv, linux-kernel
On 9/19/24 23:42, Anshuman Khandual wrote:
>> I just did a quick grep and don't see any difference between the _safe
>> and normal variants. A quick grep didn't turn up any actual users.
>>
>> Did anyone actually double check that these are still needed on x86 in
>> the first place?
> arch/x86/mm/init_64.c
Ahh, the #defines make them immune to grep. :)
Long-term, we should make sure these are still necessary. Short term
(in this patch), please just put the #defines in init_64.c if it is the
only site that needs them.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 8:52 ` Dave Hansen
@ 2024-09-20 9:27 ` Anshuman Khandual
0 siblings, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2024-09-20 9:27 UTC (permalink / raw)
To: Dave Hansen, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
David Hildenbrand, Ryan Roberts, x86, linux-riscv, linux-kernel
On 9/20/24 14:22, Dave Hansen wrote:
> On 9/19/24 23:42, Anshuman Khandual wrote:
>>> I just did a quick grep and don't see any difference between the _safe
>>> and normal variants. A quick grep didn't turn up any actual users.
>>>
>>> Did anyone actually double check that these are still needed on x86 in
>>> the first place?
>> arch/x86/mm/init_64.c
>
> Ahh, the #defines make them immune to grep. :)
>
> Long-term, we should make sure these are still necessary. Short term
Agreed, David H has also mentioned about that.
> (in this patch), please just put the #defines in init_64.c if it is the
> only site that needs them.
That is not the only site where they get used. set_pmd/pud/p4d/pgd_safe()
are also called from respective pmd/pud/p4d/pgd_populate_safe() helpers
in the header arch/x86/include/asm/pgalloc.h.
Besides also hit a road block in converting these macros as static inline
functions as suggested by David H earlier, because pmd/pud/p4d/pgd_same()
macros are defined in include/linux/pgtable.h, way after <asm/pgtable.h>
gets included.
I guess we will have just leave these macros is in arch/x86/include/asm/pgtable.h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 5:30 [PATCH] mm: Move set_pxd_safe() helpers from generic to platform Anshuman Khandual
2024-09-20 5:49 ` Dave Hansen
@ 2024-09-20 6:39 ` David Hildenbrand
2024-09-20 8:42 ` Anshuman Khandual
2024-09-26 4:18 ` Anshuman Khandual
2 siblings, 1 reply; 10+ messages in thread
From: David Hildenbrand @ 2024-09-20 6:39 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
Ryan Roberts, x86, linux-riscv, linux-kernel
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 <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 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 <david@redhat.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 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 :)
Ideally, we get rid of the macros here and just use inline functions ...
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 6:39 ` David Hildenbrand
@ 2024-09-20 8:42 ` Anshuman Khandual
2024-09-24 5:28 ` Anshuman Khandual
0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2024-09-20 8:42 UTC (permalink / raw)
To: David Hildenbrand, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
Ryan Roberts, x86, linux-riscv, linux-kernel
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 <paul.walmsley@sifive.com>
>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> 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 <david@redhat.com>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> 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);
+}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 8:42 ` Anshuman Khandual
@ 2024-09-24 5:28 ` Anshuman Khandual
2024-09-24 7:39 ` David Hildenbrand
0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2024-09-24 5:28 UTC (permalink / raw)
To: David Hildenbrand, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
Ryan Roberts, x86, linux-riscv, linux-kernel
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 <paul.walmsley@sifive.com>
>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> 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 <david@redhat.com>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>> 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
<asm/pgtable.h> gets included. I guess then the current patch should
be left as it is.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-24 5:28 ` Anshuman Khandual
@ 2024-09-24 7:39 ` David Hildenbrand
0 siblings, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2024-09-24 7:39 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
Ryan Roberts, x86, linux-riscv, linux-kernel
On 24.09.24 07:28, Anshuman Khandual wrote:
>
>
> 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 <paul.walmsley@sifive.com>
>>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>>>> Cc: David Hildenbrand <david@redhat.com>
>>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> 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 <david@redhat.com>
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>> 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
> <asm/pgtable.h> gets included. I guess then the current patch should
> be left as it is.
Just to clarify: My ideas was to get rid of set_pte_safe() *completely*
in x86 code, and turn set_pte_init() etc. into inline functions.
But we can do that on top, if Dave is fine with this patch.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: Move set_pxd_safe() helpers from generic to platform
2024-09-20 5:30 [PATCH] mm: Move set_pxd_safe() helpers from generic to platform Anshuman Khandual
2024-09-20 5:49 ` Dave Hansen
2024-09-20 6:39 ` David Hildenbrand
@ 2024-09-26 4:18 ` Anshuman Khandual
2 siblings, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2024-09-26 4:18 UTC (permalink / raw)
To: linux-mm, akpm, Dave Hansen, Palmer Dabbelt
Cc: Paul Walmsley, Palmer Dabbelt, Thomas Gleixner, Dave Hansen,
David Hildenbrand, Ryan Roberts, x86, linux-riscv, linux-kernel
On 9/20/24 11:00, 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 <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 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 <david@redhat.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 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__ */
>
> #endif /* _ASM_X86_PGTABLE_H */
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 2a6a3cccfc36..0bf88e505aad 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1050,44 +1050,6 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b)
> }
> #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); \
> -})
> -
> #ifndef __HAVE_ARCH_DO_SWAP_PAGE
> static inline void arch_do_swap_page_nr(struct mm_struct *mm,
> struct vm_area_struct *vma,
Hello Dave/Palmer,
Unless there is still any more objection from x86 or riscv, could this
patch be pulled ?
- Anshuman
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-09-26 4:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-20 5:30 [PATCH] mm: Move set_pxd_safe() helpers from generic to platform Anshuman Khandual
2024-09-20 5:49 ` Dave Hansen
2024-09-20 6:42 ` Anshuman Khandual
2024-09-20 8:52 ` Dave Hansen
2024-09-20 9:27 ` Anshuman Khandual
2024-09-20 6:39 ` David Hildenbrand
2024-09-20 8:42 ` Anshuman Khandual
2024-09-24 5:28 ` Anshuman Khandual
2024-09-24 7:39 ` David Hildenbrand
2024-09-26 4:18 ` Anshuman Khandual
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox