On 7 Dec 2024, at 10:31, Mathieu Desnoyers wrote: > On 2024-12-06 12:42, Zi Yan wrote: >> For architectures setting cpu_dcache_is_aliasing() to true, which require >> flushing cache, and arc, which changes folio->flags after clearing a user >> folio, __GFP_ZERO using only clear_page() is not enough to zero user >> folios and clear_user_(high)page() must be used. Otherwise, user data >> will be corrupted. >> >> Fix it by always clearing user folios with clear_user_(high)page() when >> cpu_dcache_is_aliasing() is true or architecture is arc. Rename >> alloc_zeroed() to alloc_need_zeroing() and invert the logic to clarify its >> intend. >> >> Fixes: 5708d96da20b ("mm: avoid zeroing user movable page twice with init_on_alloc=1") >> Reported-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> >> Closes: https://lore.kernel.org/linux-mm/CAMuHMdV1hRp_NtR5YnJo=HsfgKQeH91J537Gh4gKk3PFZhSkbA@xxxxxxxxxxxxxx/ >> Tested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> >> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx> >> --- >> include/linux/highmem.h | 8 +++++++- >> include/linux/mm.h | 17 +++++++++++++++++ >> mm/huge_memory.c | 9 +++++---- >> mm/internal.h | 6 ------ >> mm/memory.c | 10 +++++----- >> 5 files changed, 34 insertions(+), 16 deletions(-) >> >> diff --git a/include/linux/highmem.h b/include/linux/highmem.h >> index 6e452bd8e7e3..d9beb8371daa 100644 >> --- a/include/linux/highmem.h >> +++ b/include/linux/highmem.h >> @@ -224,7 +224,13 @@ static inline >> struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma, >> unsigned long vaddr) >> { >> - return vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr); >> + struct folio *folio; >> + >> + folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vaddr); >> + if (folio && alloc_need_zeroing()) >> + clear_user_highpage(&folio->page, vaddr); >> + >> + return folio; >> } >> #endif >> diff --git a/include/linux/mm.h b/include/linux/mm.h >> index c39c4945946c..ca8df5871213 100644 >> --- a/include/linux/mm.h >> +++ b/include/linux/mm.h >> @@ -31,6 +31,7 @@ >> #include <linux/kasan.h> >> #include <linux/memremap.h> >> #include <linux/slab.h> >> +#include <linux/cacheinfo.h> >> struct mempolicy; >> struct anon_vma; >> @@ -4175,6 +4176,22 @@ static inline int do_mseal(unsigned long start, size_t len_in, unsigned long fla >> } >> #endif >> +/* >> + * alloc_need_zeroing checks if a user folio from page allocator needs to be >> + * zeroed or not. >> + */ >> +static inline bool alloc_need_zeroing(void) >> +{ >> + /* >> + * for user folios, arch with cache aliasing requires cache flush and >> + * arc changes folio->flags, so always return false to make caller use >> + * clear_user_page()/clear_user_highpage() >> + */ >> + return (cpu_dcache_is_aliasing() || IS_ENABLED(CONFIG_ARC)) || > > Nack. > > Can we please not go back to re-introducing arch-specific > conditionals in generic mm code after the cleanup I did when > introducing cpu_dcache_is_aliasing() in commit 8690bbcf3b70 ? OK > > Based on commit eacd0e950dc2, AFAIU what you appear to need here > is to introduce a: > > cpu_icache_is_aliasing() -> note the "i" for instruction cache > > It would typically be directly set to > > #define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() > > except on architecture like ARC when the icache vs dcache > is aliasing, but not dcache vs dcache. > > So for ARC it would be defined as: > > #define cpu_dcache_is_aliasing() false > #define cpu_icache_is_aliasing() true > > And the Kconfig ARCH_HAS_CPU_CACHE_ALIASING=y would be set for ARC > again. > Sounds good to me. > I'm not entirely sure if we want to go for the wording "is_aliasing" > or "is_incoherent" when talking about icache vs dcache, so I'm open > to ideas here. > Let me know if the code below looks good to you. I will use (cpu_icache_is_aliasing() || cpu_dcache_is_aliasing()) instead of (cpu_dcache_is_aliasing() || IS_ENABLED(CONFIG_ARC)) in my next version. diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig index 5b2488142041..e96935373796 100644 --- a/arch/arc/Kconfig +++ b/arch/arc/Kconfig @@ -6,6 +6,7 @@ config ARC def_bool y select ARC_TIMERS + select ARCH_HAS_CPU_CACHE_ALIASING select ARCH_HAS_CACHE_LINE_SIZE select ARCH_HAS_DEBUG_VM_PGTABLE select ARCH_HAS_DMA_PREP_COHERENT diff --git a/arch/arc/include/asm/cachetype.h b/arch/arc/include/asm/cachetype.h new file mode 100644 index 000000000000..acd3b6cb4bf5 --- /dev/null +++ b/arch/arc/include/asm/cachetype.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_ARC_CACHETYPE_H +#define __ASM_ARC_CACHETYPE_H + +#define cpu_dcache_is_aliasing() false +#define cpu_icache_is_aliasing() true + +#endif diff --git a/arch/arm/include/asm/cachetype.h b/arch/arm/include/asm/cachetype.h index b9dbe1d4c8fe..80725010c38f 100644 --- a/arch/arm/include/asm/cachetype.h +++ b/arch/arm/include/asm/cachetype.h @@ -21,6 +21,7 @@ extern unsigned int cacheid; #define icache_is_pipt() cacheid_is(CACHEID_PIPT) #define cpu_dcache_is_aliasing() (cache_is_vivt() || cache_is_vipt_aliasing()) +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() /* * __LINUX_ARM_ARCH__ is the minimum supported CPU architecture diff --git a/arch/csky/include/asm/cachetype.h b/arch/csky/include/asm/cachetype.h index 98cbe3af662f..838c7e708d19 100644 --- a/arch/csky/include/asm/cachetype.h +++ b/arch/csky/include/asm/cachetype.h @@ -5,5 +5,6 @@ #include <linux/types.h> #define cpu_dcache_is_aliasing() true +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/arch/m68k/include/asm/cachetype.h b/arch/m68k/include/asm/cachetype.h index 7fad5d9ab8fe..4993846354f4 100644 --- a/arch/m68k/include/asm/cachetype.h +++ b/arch/m68k/include/asm/cachetype.h @@ -5,5 +5,6 @@ #include <linux/types.h> #define cpu_dcache_is_aliasing() true +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/arch/mips/include/asm/cachetype.h b/arch/mips/include/asm/cachetype.h index 9f4ba2fe1155..aac25ee27cd1 100644 --- a/arch/mips/include/asm/cachetype.h +++ b/arch/mips/include/asm/cachetype.h @@ -5,5 +5,6 @@ #include <asm/cpu-features.h> #define cpu_dcache_is_aliasing() cpu_has_dc_aliases +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/arch/nios2/include/asm/cachetype.h b/arch/nios2/include/asm/cachetype.h index eb9c416b8a1c..4b492fbb4a39 100644 --- a/arch/nios2/include/asm/cachetype.h +++ b/arch/nios2/include/asm/cachetype.h @@ -6,5 +6,6 @@ #include <asm/cache.h> #define cpu_dcache_is_aliasing() (NIOS2_DCACHE_SIZE > PAGE_SIZE) +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/arch/parisc/include/asm/cachetype.h b/arch/parisc/include/asm/cachetype.h index e0868a1d3c47..53180ff9d09a 100644 --- a/arch/parisc/include/asm/cachetype.h +++ b/arch/parisc/include/asm/cachetype.h @@ -5,5 +5,6 @@ #include <linux/types.h> #define cpu_dcache_is_aliasing() true +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/arch/sh/include/asm/cachetype.h b/arch/sh/include/asm/cachetype.h index a5fffe536068..f711631b9451 100644 --- a/arch/sh/include/asm/cachetype.h +++ b/arch/sh/include/asm/cachetype.h @@ -5,5 +5,6 @@ #include <linux/types.h> #define cpu_dcache_is_aliasing() true +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/arch/xtensa/include/asm/cachetype.h b/arch/xtensa/include/asm/cachetype.h index 51bd49e2a1c5..89560b325006 100644 --- a/arch/xtensa/include/asm/cachetype.h +++ b/arch/xtensa/include/asm/cachetype.h @@ -6,5 +6,6 @@ #include <asm/page.h> #define cpu_dcache_is_aliasing() (DCACHE_WAY_SIZE > PAGE_SIZE) +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #endif diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h index 108060612bb8..a2dd3d00e080 100644 --- a/include/linux/cacheinfo.h +++ b/include/linux/cacheinfo.h @@ -155,6 +155,7 @@ static inline int get_cpu_cacheinfo_id(int cpu, int level) #ifndef CONFIG_ARCH_HAS_CPU_CACHE_ALIASING #define cpu_dcache_is_aliasing() false +#define cpu_icache_is_aliasing() cpu_dcache_is_aliasing() #else #include <asm/cachetype.h> #endif Best Regards, Yan, Zi