From: David Hildenbrand <david@redhat.com>

Let's drop all variants that effectively map to clear_page() and
provide it in a generic variant instead.

We'll use the macro clear_user_page to indicate whether an architecture
provides it's own variant.

We have to be a bit careful if an architecture provides a custom
clear_user_highpage(), because then it's very likely that some special
flushing magic is happening behind the scenes.

Maybe at some point these should be CONFIG_ options.

Note that for parisc, clear_page() and clear_user_page() map to
clear_page_asm(), so we can just get rid of the custom clear_user_page()
implementation. There is a clear_user_page_asm() function on parisc,
that seems to be unused. Not sure what's up with that.

Signed-off-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---

Notes:
    
   - For architectures which have a specialized version of clear_user_page(),
     define clear_user_page instead of __HAVE_ARCH_CLEAR_USER_PAGE.
   - m68k/include/asm/page_mm.h, sparc/include/asm/page_32.h already define
     clear_user_page() as a macro so we just use that instead.

 arch/alpha/include/asm/page.h      |  1 -
 arch/arc/include/asm/page.h        |  2 ++
 arch/arm/include/asm/page-nommu.h  |  1 -
 arch/arm64/include/asm/page.h      |  1 -
 arch/csky/abiv1/inc/abi/page.h     |  1 +
 arch/csky/abiv2/inc/abi/page.h     |  7 -------
 arch/hexagon/include/asm/page.h    |  1 -
 arch/loongarch/include/asm/page.h  |  1 -
 arch/m68k/include/asm/page_no.h    |  1 -
 arch/microblaze/include/asm/page.h |  1 -
 arch/mips/include/asm/page.h       |  1 +
 arch/nios2/include/asm/page.h      |  1 +
 arch/openrisc/include/asm/page.h   |  1 -
 arch/parisc/include/asm/page.h     |  1 -
 arch/powerpc/include/asm/page.h    |  1 +
 arch/riscv/include/asm/page.h      |  1 -
 arch/s390/include/asm/page.h       |  1 -
 arch/sparc/include/asm/page_64.h   |  1 +
 arch/um/include/asm/page.h         |  1 -
 arch/x86/include/asm/page.h        |  6 ------
 arch/xtensa/include/asm/page.h     |  1 -
 include/linux/mm.h                 | 22 ++++++++++++++++++++++
 22 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/arch/alpha/include/asm/page.h b/arch/alpha/include/asm/page.h
index 5ec4c77e432e..d71ef845deca 100644
--- a/arch/alpha/include/asm/page.h
+++ b/arch/alpha/include/asm/page.h
@@ -11,7 +11,6 @@
 #define STRICT_MM_TYPECHECKS
 
 extern void clear_page(void *page);
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 
 #define vma_alloc_zeroed_movable_folio(vma, vaddr) \
 	vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr)
diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h
index 9720fe6b2c24..38214e126c6d 100644
--- a/arch/arc/include/asm/page.h
+++ b/arch/arc/include/asm/page.h
@@ -32,6 +32,8 @@ struct page;
 
 void copy_user_highpage(struct page *to, struct page *from,
 			unsigned long u_vaddr, struct vm_area_struct *vma);
+
+#define clear_user_page clear_user_page
 void clear_user_page(void *to, unsigned long u_vaddr, struct page *page);
 
 typedef struct {
diff --git a/arch/arm/include/asm/page-nommu.h b/arch/arm/include/asm/page-nommu.h
index 7c2c72323d17..e74415c959be 100644
--- a/arch/arm/include/asm/page-nommu.h
+++ b/arch/arm/include/asm/page-nommu.h
@@ -11,7 +11,6 @@
 #define clear_page(page)	memset((page), 0, PAGE_SIZE)
 #define copy_page(to,from)	memcpy((to), (from), PAGE_SIZE)
 
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 /*
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index 258cca4b4873..0e8de245e283 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -36,7 +36,6 @@ struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
 bool tag_clear_highpages(struct page *to, int numpages);
 #define __HAVE_ARCH_TAG_CLEAR_HIGHPAGES
 
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 typedef struct page *pgtable_t;
diff --git a/arch/csky/abiv1/inc/abi/page.h b/arch/csky/abiv1/inc/abi/page.h
index 2d2159933b76..58307254e7e5 100644
--- a/arch/csky/abiv1/inc/abi/page.h
+++ b/arch/csky/abiv1/inc/abi/page.h
@@ -10,6 +10,7 @@ static inline unsigned long pages_do_alias(unsigned long addr1,
 	return (addr1 ^ addr2) & (SHMLBA-1);
 }
 
+#define clear_user_page clear_user_page
 static inline void clear_user_page(void *addr, unsigned long vaddr,
 				   struct page *page)
 {
diff --git a/arch/csky/abiv2/inc/abi/page.h b/arch/csky/abiv2/inc/abi/page.h
index cf005f13cd15..a5a255013308 100644
--- a/arch/csky/abiv2/inc/abi/page.h
+++ b/arch/csky/abiv2/inc/abi/page.h
@@ -1,11 +1,4 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-
-static inline void clear_user_page(void *addr, unsigned long vaddr,
-				   struct page *page)
-{
-	clear_page(addr);
-}
-
 static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
 				  struct page *page)
 {
diff --git a/arch/hexagon/include/asm/page.h b/arch/hexagon/include/asm/page.h
index 137ba7c5de48..f0aed3ed812b 100644
--- a/arch/hexagon/include/asm/page.h
+++ b/arch/hexagon/include/asm/page.h
@@ -113,7 +113,6 @@ static inline void clear_page(void *page)
 /*
  * Under assumption that kernel always "sees" user map...
  */
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 static inline unsigned long virt_to_pfn(const void *kaddr)
diff --git a/arch/loongarch/include/asm/page.h b/arch/loongarch/include/asm/page.h
index a3aaf34fba16..b83415fe4ffb 100644
--- a/arch/loongarch/include/asm/page.h
+++ b/arch/loongarch/include/asm/page.h
@@ -30,7 +30,6 @@
 extern void clear_page(void *page);
 extern void copy_page(void *to, void *from);
 
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 extern unsigned long shm_align_mask;
diff --git a/arch/m68k/include/asm/page_no.h b/arch/m68k/include/asm/page_no.h
index 39db2026a4b4..d2532bc407ef 100644
--- a/arch/m68k/include/asm/page_no.h
+++ b/arch/m68k/include/asm/page_no.h
@@ -10,7 +10,6 @@ extern unsigned long memory_end;
 #define clear_page(page)	memset((page), 0, PAGE_SIZE)
 #define copy_page(to,from)	memcpy((to), (from), PAGE_SIZE)
 
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 #define vma_alloc_zeroed_movable_folio(vma, vaddr) \
diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
index 90ac9f34b4b4..e1e396367ba7 100644
--- a/arch/microblaze/include/asm/page.h
+++ b/arch/microblaze/include/asm/page.h
@@ -45,7 +45,6 @@ typedef unsigned long pte_basic_t;
 # define copy_page(to, from)			memcpy((to), (from), PAGE_SIZE)
 # define clear_page(pgaddr)			memset((pgaddr), 0, PAGE_SIZE)
 
-# define clear_user_page(pgaddr, vaddr, page)	memset((pgaddr), 0, PAGE_SIZE)
 # define copy_user_page(vto, vfrom, vaddr, topg) \
 			memcpy((vto), (vfrom), PAGE_SIZE)
 
diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index bc3e3484c1bf..5ec428fcc887 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -90,6 +90,7 @@ static inline void clear_user_page(void *addr, unsigned long vaddr,
 	if (pages_do_alias((unsigned long) addr, vaddr & PAGE_MASK))
 		flush_data_cache_page((unsigned long)addr);
 }
+#define clear_user_page clear_user_page
 
 struct vm_area_struct;
 extern void copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/nios2/include/asm/page.h b/arch/nios2/include/asm/page.h
index 00a51623d38a..722956ac0bf8 100644
--- a/arch/nios2/include/asm/page.h
+++ b/arch/nios2/include/asm/page.h
@@ -45,6 +45,7 @@
 
 struct page;
 
+#define clear_user_page clear_user_page
 extern void clear_user_page(void *addr, unsigned long vaddr, struct page *page);
 extern void copy_user_page(void *vto, void *vfrom, unsigned long vaddr,
 				struct page *to);
diff --git a/arch/openrisc/include/asm/page.h b/arch/openrisc/include/asm/page.h
index 85797f94d1d7..d2cdbf3579bb 100644
--- a/arch/openrisc/include/asm/page.h
+++ b/arch/openrisc/include/asm/page.h
@@ -30,7 +30,6 @@
 #define clear_page(page)	memset((page), 0, PAGE_SIZE)
 #define copy_page(to, from)	memcpy((to), (from), PAGE_SIZE)
 
-#define clear_user_page(page, vaddr, pg)        clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)     copy_page(to, from)
 
 /*
diff --git a/arch/parisc/include/asm/page.h b/arch/parisc/include/asm/page.h
index 8f4e51071ea1..3630b36d07da 100644
--- a/arch/parisc/include/asm/page.h
+++ b/arch/parisc/include/asm/page.h
@@ -21,7 +21,6 @@ struct vm_area_struct;
 
 void clear_page_asm(void *page);
 void copy_page_asm(void *to, void *from);
-#define clear_user_page(vto, vaddr, page) clear_page_asm(vto)
 void copy_user_highpage(struct page *to, struct page *from, unsigned long vaddr,
 		struct vm_area_struct *vma);
 #define __HAVE_ARCH_COPY_USER_HIGHPAGE
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b28fbb1d57eb..f2bb1f98eebe 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -271,6 +271,7 @@ static inline const void *pfn_to_kaddr(unsigned long pfn)
 
 struct page;
 extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg);
+#define clear_user_page clear_user_page
 extern void copy_user_page(void *to, void *from, unsigned long vaddr,
 		struct page *p);
 extern int devmem_is_allowed(unsigned long pfn);
diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index ffe213ad65a4..061b60b954ec 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -50,7 +50,6 @@ void clear_page(void *page);
 #endif
 #define copy_page(to, from)			memcpy((to), (from), PAGE_SIZE)
 
-#define clear_user_page(pgaddr, vaddr, page)	clear_page(pgaddr)
 #define copy_user_page(vto, vfrom, vaddr, topg) \
 			memcpy((vto), (vfrom), PAGE_SIZE)
 
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index 9240a363c893..6635ba56d4b2 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -65,7 +65,6 @@ static inline void copy_page(void *to, void *from)
 		: : "memory", "cc");
 }
 
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 #define vma_alloc_zeroed_movable_folio(vma, vaddr) \
diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h
index d764d8a8586b..fd4dc85fb38b 100644
--- a/arch/sparc/include/asm/page_64.h
+++ b/arch/sparc/include/asm/page_64.h
@@ -43,6 +43,7 @@ void _clear_page(void *page);
 #define clear_page(X)	_clear_page((void *)(X))
 struct page;
 void clear_user_page(void *addr, unsigned long vaddr, struct page *page);
+#define clear_user_page clear_user_page
 #define copy_page(X,Y)	memcpy((void *)(X), (void *)(Y), PAGE_SIZE)
 void copy_user_page(void *to, void *from, unsigned long vaddr, struct page *topage);
 #define __HAVE_ARCH_COPY_USER_HIGHPAGE
diff --git a/arch/um/include/asm/page.h b/arch/um/include/asm/page.h
index 6f54254aaf44..8cea97a9c8f9 100644
--- a/arch/um/include/asm/page.h
+++ b/arch/um/include/asm/page.h
@@ -26,7 +26,6 @@ struct page;
 #define clear_page(page)	memset((void *)(page), 0, PAGE_SIZE)
 #define copy_page(to,from)	memcpy((void *)(to), (void *)(from), PAGE_SIZE)
 
-#define clear_user_page(page, vaddr, pg)	clear_page(page)
 #define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 
 typedef struct { unsigned long pte; } pte_t;
diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index 9265f2fca99a..416dc88e35c1 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -22,12 +22,6 @@ struct page;
 extern struct range pfn_mapped[];
 extern int nr_pfn_mapped;
 
-static inline void clear_user_page(void *page, unsigned long vaddr,
-				   struct page *pg)
-{
-	clear_page(page);
-}
-
 static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
 				  struct page *topage)
 {
diff --git a/arch/xtensa/include/asm/page.h b/arch/xtensa/include/asm/page.h
index 20655174b111..059493256765 100644
--- a/arch/xtensa/include/asm/page.h
+++ b/arch/xtensa/include/asm/page.h
@@ -126,7 +126,6 @@ void clear_user_highpage(struct page *page, unsigned long vaddr);
 void copy_user_highpage(struct page *to, struct page *from,
 			unsigned long vaddr, struct vm_area_struct *vma);
 #else
-# define clear_user_page(page, vaddr, pg)	clear_page(page)
 # define copy_user_page(to, from, vaddr, pg)	copy_page(to, from)
 #endif
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7c79b3369b82..6fa6c188f99a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3879,6 +3879,28 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
 				unsigned int order) {}
 #endif	/* CONFIG_DEBUG_PAGEALLOC */
 
+#ifndef clear_user_page
+/**
+ * clear_user_page() - clear a page to be mapped to user space
+ * @addr: the address of the page
+ * @vaddr: the address of the user mapping
+ * @page: the page
+ */
+static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
+{
+#ifdef clear_user_highpage
+	/*
+	 * If an architecture defines its own clear_user_highpage() variant,
+	 * then we have to be a bit more careful here and cannot simply
+	 * rely on clear_page().
+	 */
+	clear_user_highpage(page, vaddr);
+#else
+	clear_page(addr);
+#endif
+}
+#endif
+
 #ifdef __HAVE_ARCH_GATE_AREA
 extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
 extern int in_gate_area_no_mm(unsigned long addr);
-- 
2.31.1

Le 21/11/2025 à 21:23, Ankur Arora a écrit :
> From: David Hildenbrand <david@redhat.com>
> 
> Let's drop all variants that effectively map to clear_page() and
> provide it in a generic variant instead.
> 
> We'll use the macro clear_user_page to indicate whether an architecture
> provides it's own variant.
> 
> We have to be a bit careful if an architecture provides a custom
> clear_user_highpage(), because then it's very likely that some special
> flushing magic is happening behind the scenes.
> 
> Maybe at some point these should be CONFIG_ options.
> 
> Note that for parisc, clear_page() and clear_user_page() map to
> clear_page_asm(), so we can just get rid of the custom clear_user_page()
> implementation. There is a clear_user_page_asm() function on parisc,
> that seems to be unused. Not sure what's up with that.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>

...

> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7c79b3369b82..6fa6c188f99a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3879,6 +3879,28 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>   				unsigned int order) {}
>   #endif	/* CONFIG_DEBUG_PAGEALLOC */
>   
> +#ifndef clear_user_page
> +/**
> + * clear_user_page() - clear a page to be mapped to user space
> + * @addr: the address of the page
> + * @vaddr: the address of the user mapping
> + * @page: the page
> + */
> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
> +{
> +#ifdef clear_user_highpage
> +	/*
> +	 * If an architecture defines its own clear_user_highpage() variant,
> +	 * then we have to be a bit more careful here and cannot simply
> +	 * rely on clear_page().
> +	 */
> +	clear_user_highpage(page, vaddr);
> +#else
> +	clear_page(addr);
> +#endif
> +}
> +#endif
> +
>   #ifdef __HAVE_ARCH_GATE_AREA
>   extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
>   extern int in_gate_area_no_mm(unsigned long addr);


Isn't it chicken and egg with clear_user_highpage() in linux/highmem.h ? :

#ifndef clear_user_highpage
static inline void clear_user_highpage(struct page *page, unsigned long 
vaddr)
{
	void *addr = kmap_local_page(page);
	clear_user_page(addr, vaddr, page);
	kunmap_local(addr);
}
#endif

And at the end this function is the only caller of clear_user_page() so 
there is apparently no need for a generic clear_user_page(), at least 
not when clear_user_highpage() is defined.

I think is would be simpler and cleaner to instead add the following in 
linux/highmem.c:

#ifndef clear_user_page
/**
  * clear_user_page() - clear a page to be mapped to user space
  * @addr: the address of the page
  * @vaddr: the address of the user mapping
  * @page: the page
  */
static inline void clear_user_page(void *addr, unsigned long vaddr, 
struct page *page)
{
	clear_page(addr);
}
#endif

On 11/23/25 12:53, Christophe Leroy (CS GROUP) wrote:
> 
> 
> Le 21/11/2025 à 21:23, Ankur Arora a écrit :
>> From: David Hildenbrand <david@redhat.com>
>>
>> Let's drop all variants that effectively map to clear_page() and
>> provide it in a generic variant instead.
>>
>> We'll use the macro clear_user_page to indicate whether an architecture
>> provides it's own variant.
>>
>> We have to be a bit careful if an architecture provides a custom
>> clear_user_highpage(), because then it's very likely that some special
>> flushing magic is happening behind the scenes.
>>
>> Maybe at some point these should be CONFIG_ options.
>>
>> Note that for parisc, clear_page() and clear_user_page() map to
>> clear_page_asm(), so we can just get rid of the custom clear_user_page()
>> implementation. There is a clear_user_page_asm() function on parisc,
>> that seems to be unused. Not sure what's up with that.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> 
> ...
> 
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 7c79b3369b82..6fa6c188f99a 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3879,6 +3879,28 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>>    				unsigned int order) {}
>>    #endif	/* CONFIG_DEBUG_PAGEALLOC */
>>    
>> +#ifndef clear_user_page
>> +/**
>> + * clear_user_page() - clear a page to be mapped to user space
>> + * @addr: the address of the page
>> + * @vaddr: the address of the user mapping
>> + * @page: the page
>> + */
>> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
>> +{
>> +#ifdef clear_user_highpage
>> +	/*
>> +	 * If an architecture defines its own clear_user_highpage() variant,
>> +	 * then we have to be a bit more careful here and cannot simply
>> +	 * rely on clear_page().
>> +	 */
>> +	clear_user_highpage(page, vaddr);
>> +#else
>> +	clear_page(addr);
>> +#endif
>> +}
>> +#endif
>> +
>>    #ifdef __HAVE_ARCH_GATE_AREA
>>    extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
>>    extern int in_gate_area_no_mm(unsigned long addr);
> 
> 
> Isn't it chicken and egg with clear_user_highpage() in linux/highmem.h ? :

No really, because we make use of clear_user_highpage() only when the 
arch defines it, so the highmem.h variant is ignored?

Not that I particularly enjoy this way of handling it, so something 
cleaner would be nice :)

(in particular, relying on highmem.h defines in mm.h is a bit suboptimal)

> 
> #ifndef clear_user_highpage
> static inline void clear_user_highpage(struct page *page, unsigned long
> vaddr)
> {
> 	void *addr = kmap_local_page(page);
> 	clear_user_page(addr, vaddr, page);
> 	kunmap_local(addr);
> }
> #endif
> 
> And at the end this function is the only caller of clear_user_page() so
> there is apparently no need for a generic clear_user_page(), at least
> not when clear_user_highpage() is defined.
> 
> I think is would be simpler and cleaner to instead add the following in
> linux/highmem.c:

I assume you mean highmem.h

It's not really highmem.h material, but if it makes things cleaner, sure.

Might be that the compiler will not be happy about that.

@Ankur can you play with that and see if we can make compilers happy one 
way or the other?

-- 
Cheers

David

David Hildenbrand (Red Hat) <david@kernel.org> writes:

> On 11/23/25 12:53, Christophe Leroy (CS GROUP) wrote:
>>
>> Le 21/11/2025 à 21:23, Ankur Arora a écrit :
>>> From: David Hildenbrand <david@redhat.com>
>>>
>>> Let's drop all variants that effectively map to clear_page() and
>>> provide it in a generic variant instead.
>>>
>>> We'll use the macro clear_user_page to indicate whether an architecture
>>> provides it's own variant.
>>>
>>> We have to be a bit careful if an architecture provides a custom
>>> clear_user_highpage(), because then it's very likely that some special
>>> flushing magic is happening behind the scenes.
>>>
>>> Maybe at some point these should be CONFIG_ options.
>>>
>>> Note that for parisc, clear_page() and clear_user_page() map to
>>> clear_page_asm(), so we can just get rid of the custom clear_user_page()
>>> implementation. There is a clear_user_page_asm() function on parisc,
>>> that seems to be unused. Not sure what's up with that.
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>> ...
>>
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index 7c79b3369b82..6fa6c188f99a 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -3879,6 +3879,28 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>>>    				unsigned int order) {}
>>>    #endif	/* CONFIG_DEBUG_PAGEALLOC */
>>>    +#ifndef clear_user_page
>>> +/**
>>> + * clear_user_page() - clear a page to be mapped to user space
>>> + * @addr: the address of the page
>>> + * @vaddr: the address of the user mapping
>>> + * @page: the page
>>> + */
>>> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
>>> +{
>>> +#ifdef clear_user_highpage
>>> +	/*
>>> +	 * If an architecture defines its own clear_user_highpage() variant,
>>> +	 * then we have to be a bit more careful here and cannot simply
>>> +	 * rely on clear_page().
>>> +	 */
>>> +	clear_user_highpage(page, vaddr);
>>> +#else
>>> +	clear_page(addr);
>>> +#endif
>>> +}
>>> +#endif
>>> +
>>>    #ifdef __HAVE_ARCH_GATE_AREA
>>>    extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
>>>    extern int in_gate_area_no_mm(unsigned long addr);
>>
>> Isn't it chicken and egg with clear_user_highpage() in linux/highmem.h ? :
>
> No really, because we make use of clear_user_highpage() only when the arch
> defines it, so the highmem.h variant is ignored?
>
> Not that I particularly enjoy this way of handling it, so something cleaner
> would be nice :)
>
> (in particular, relying on highmem.h defines in mm.h is a bit suboptimal)
>
>> #ifndef clear_user_highpage
>> static inline void clear_user_highpage(struct page *page, unsigned long
>> vaddr)
>> {
>> 	void *addr = kmap_local_page(page);
>> 	clear_user_page(addr, vaddr, page);
>> 	kunmap_local(addr);
>> }
>> #endif
>> And at the end this function is the only caller of clear_user_page() so
>> there is apparently no need for a generic clear_user_page(), at least
>> not when clear_user_highpage() is defined.
>> I think is would be simpler and cleaner to instead add the following in
>> linux/highmem.c:
>
> I assume you mean highmem.h
>
> It's not really highmem.h material, but if it makes things cleaner, sure.
>
> Might be that the compiler will not be happy about that.
>
> @Ankur can you play with that and see if we can make compilers happy one way or
> the other?

This looks like a good change (and from my tests on a couple of
configs seems to build fine.)

Though clear_user_pages() is also only called from clear_user_highpages().
Would be good to treat that similarly.

(I don't think it can be done because how the arch code treats
both quite differently.)

Anyway I'll play with that a bit more.

--
ankur

Ankur Arora <ankur.a.arora@oracle.com> writes:

> David Hildenbrand (Red Hat) <david@kernel.org> writes:
>
>> On 11/23/25 12:53, Christophe Leroy (CS GROUP) wrote:
>>>
>>> Le 21/11/2025 à 21:23, Ankur Arora a écrit :
>>>> From: David Hildenbrand <david@redhat.com>
>>>>
>>>> Let's drop all variants that effectively map to clear_page() and
>>>> provide it in a generic variant instead.
>>>>
>>>> We'll use the macro clear_user_page to indicate whether an architecture
>>>> provides it's own variant.
>>>>
>>>> We have to be a bit careful if an architecture provides a custom
>>>> clear_user_highpage(), because then it's very likely that some special
>>>> flushing magic is happening behind the scenes.
>>>>
>>>> Maybe at some point these should be CONFIG_ options.
>>>>
>>>> Note that for parisc, clear_page() and clear_user_page() map to
>>>> clear_page_asm(), so we can just get rid of the custom clear_user_page()
>>>> implementation. There is a clear_user_page_asm() function on parisc,
>>>> that seems to be unused. Not sure what's up with that.
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>>> ...
>>>
>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>>> index 7c79b3369b82..6fa6c188f99a 100644
>>>> --- a/include/linux/mm.h
>>>> +++ b/include/linux/mm.h
>>>> @@ -3879,6 +3879,28 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>>>>    				unsigned int order) {}
>>>>    #endif	/* CONFIG_DEBUG_PAGEALLOC */
>>>>    +#ifndef clear_user_page
>>>> +/**
>>>> + * clear_user_page() - clear a page to be mapped to user space
>>>> + * @addr: the address of the page
>>>> + * @vaddr: the address of the user mapping
>>>> + * @page: the page
>>>> + */
>>>> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
>>>> +{
>>>> +#ifdef clear_user_highpage
>>>> +	/*
>>>> +	 * If an architecture defines its own clear_user_highpage() variant,
>>>> +	 * then we have to be a bit more careful here and cannot simply
>>>> +	 * rely on clear_page().
>>>> +	 */
>>>> +	clear_user_highpage(page, vaddr);
>>>> +#else
>>>> +	clear_page(addr);
>>>> +#endif
>>>> +}
>>>> +#endif
>>>> +
>>>>    #ifdef __HAVE_ARCH_GATE_AREA
>>>>    extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
>>>>    extern int in_gate_area_no_mm(unsigned long addr);
>>>
>>> Isn't it chicken and egg with clear_user_highpage() in linux/highmem.h ? :
>>
>> No really, because we make use of clear_user_highpage() only when the arch
>> defines it, so the highmem.h variant is ignored?
>>
>> Not that I particularly enjoy this way of handling it, so something cleaner
>> would be nice :)
>>
>> (in particular, relying on highmem.h defines in mm.h is a bit suboptimal)
>>
>>> #ifndef clear_user_highpage
>>> static inline void clear_user_highpage(struct page *page, unsigned long
>>> vaddr)
>>> {
>>> 	void *addr = kmap_local_page(page);
>>> 	clear_user_page(addr, vaddr, page);
>>> 	kunmap_local(addr);
>>> }
>>> #endif
>>> And at the end this function is the only caller of clear_user_page() so
>>> there is apparently no need for a generic clear_user_page(), at least
>>> not when clear_user_highpage() is defined.
>>> I think is would be simpler and cleaner to instead add the following in
>>> linux/highmem.c:
>>
>> I assume you mean highmem.h
>>
>> It's not really highmem.h material, but if it makes things cleaner, sure.
>>
>> Might be that the compiler will not be happy about that.
>>
>> @Ankur can you play with that and see if we can make compilers happy one way or
>> the other?
>
> This looks like a good change (and from my tests on a couple of
> configs seems to build fine.)
>
> Though clear_user_pages() is also only called from clear_user_highpages().
> Would be good to treat that similarly.
>
> (I don't think it can be done because how the arch code treats
> both quite differently.)
>
> Anyway I'll play with that a bit more.

How about something like this for clear_user_page() (though maybe I
should be always defining clear_user_page() and not conditioning it on
the existence of the generic clear_user_highpage()):

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index abc20f9810fd..ca9d28aa29b2 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -197,6 +197,22 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
 }
 #endif

+#if !defined(clear_user_page) && !defined(clear_user_highpage)
+/**
+ * clear_user_page() - clear a page to be mapped to user space
+ * @addr: the address of the page
+ * @vaddr: the address of the user mapping
+ * @page: the page
+ *
+ * The sole user of clear_user_page() is clear_user_highpage().
+ * Define it if the arch does not and only if needed.
+ */
+static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
+{
+       clear_page(addr);
+}
+#endif
+
 /* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
 #ifndef clear_user_highpage
 static inline void clear_user_highpage(struct page *page, unsigned long vaddr)

And for clear_user_pages():

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index ca9d28aa29b2..b9b3cc76a91a 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -223,6 +223,35 @@ static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
 }
 #endif

+/**
+ * clear_user_pages() - clear a page range to be mapped to user space
+ * @addr: start address
+ * @vaddr: start address of the user mapping
+ * @page: start page
+ * @npages: number of pages
+ *
+ * Assumes that the region (@addr, +@npages) has been validated
+ * already so this does no exception handling.
+ */
+static inline void clear_user_pages(void *addr, unsigned long vaddr,
+               struct page *page, unsigned int npages)
+{
+#ifdef clear_user_page
+       do {
+               clear_user_page(addr, vaddr, page);
+               addr += PAGE_SIZE;
+               vaddr += PAGE_SIZE;
+               page++;
+       } while (--npages);
+#else
+       /*
+        * Prefer clear_pages() to allow for architectural optimizations
+        * when operations on contiguous page ranges.
+        */
+       clear_pages(addr, npages);
+#endif
+}
+

Tree with changes: github.com/terminus/linux clear-pages.v10-rc2

This seems to be fine on a variety of configurations.

David, could you take a quick look at it?

Thanks
--
ankur

Hi Ankur,

Le 28/11/2025 à 00:57, Ankur Arora a écrit :
> 
> Ankur Arora <ankur.a.arora@oracle.com> writes:
> 
> 
> How about something like this for clear_user_page() (though maybe I
> should be always defining clear_user_page() and not conditioning it on
> the existence of the generic clear_user_highpage()):
> 
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index abc20f9810fd..ca9d28aa29b2 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -197,6 +197,22 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
>   }
>   #endif
> 
> +#if !defined(clear_user_page) && !defined(clear_user_highpage)
> +/**
> + * clear_user_page() - clear a page to be mapped to user space
> + * @addr: the address of the page
> + * @vaddr: the address of the user mapping
> + * @page: the page
> + *
> + * The sole user of clear_user_page() is clear_user_highpage().
> + * Define it if the arch does not and only if needed.
> + */
> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
> +{
> +       clear_page(addr);
> +}
> +#endif

WOuld be more obvious if you enclose that inside the same #ifdef as 
clear_user_highpage(), something like:

#ifndef clear_user_highpage

#ifndef clear_user_page
static inline void clear_user_page(void *addr, unsigned long vaddr, 
struct page *page)
{
	clear_page(addr);
}
#endif

static inline void clear_user_highpage(struct page *page, unsigned long 
vaddr)
{
	void *addr = kmap_local_page(page);
	clear_user_page(addr, vaddr, page);
	kunmap_local(addr);
}
#endif

> +
>   /* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
>   #ifndef clear_user_highpage
>   static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
> 
> And for clear_user_pages():
> 
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index ca9d28aa29b2..b9b3cc76a91a 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -223,6 +223,35 @@ static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
>   }
>   #endif
> 
> +/**
> + * clear_user_pages() - clear a page range to be mapped to user space
> + * @addr: start address
> + * @vaddr: start address of the user mapping
> + * @page: start page
> + * @npages: number of pages
> + *
> + * Assumes that the region (@addr, +@npages) has been validated
> + * already so this does no exception handling.
> + */
> +static inline void clear_user_pages(void *addr, unsigned long vaddr,
> +               struct page *page, unsigned int npages)
> +{
> +#ifdef clear_user_page
> +       do {
> +               clear_user_page(addr, vaddr, page);
> +               addr += PAGE_SIZE;
> +               vaddr += PAGE_SIZE;
> +               page++;
> +       } while (--npages);
> +#else
> +       /*
> +        * Prefer clear_pages() to allow for architectural optimizations
> +        * when operations on contiguous page ranges.
> +        */
> +       clear_pages(addr, npages);
> +#endif
> +}

Not sure to understand the logic. You say this is not expected to be 
overriden by architectures in the near future, then why do we need that 
? Can't we do everything inside clear_user_highpages() for clarity ?

At the time being clear_user_page() is used exclusively by 
clear_user_highpage() so I expect clear_user_page() to only exist when 
CONFIG_HIGHMEM is enabled. And in that case clear_user_highpages() 
doesn't call clear_user_pages() so at the end only the else part 
remains, which is a simple call to clear_pages(). Why not just call 
clear_pages() directly from clear_user_highpages() and drop 
clear_user_pages() ?

Christophe

Christophe Leroy (CS GROUP) <chleroy@kernel.org> writes:

> Hi Ankur,
>
> Le 28/11/2025 à 00:57, Ankur Arora a écrit :
>> Ankur Arora <ankur.a.arora@oracle.com> writes:
>>
>> How about something like this for clear_user_page() (though maybe I
>> should be always defining clear_user_page() and not conditioning it on
>> the existence of the generic clear_user_highpage()):
>> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
>> index abc20f9810fd..ca9d28aa29b2 100644
>> --- a/include/linux/highmem.h
>> +++ b/include/linux/highmem.h
>> @@ -197,6 +197,22 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
>>   }
>>   #endif
>> +#if !defined(clear_user_page) && !defined(clear_user_highpage)
>> +/**
>> + * clear_user_page() - clear a page to be mapped to user space
>> + * @addr: the address of the page
>> + * @vaddr: the address of the user mapping
>> + * @page: the page
>> + *
>> + * The sole user of clear_user_page() is clear_user_highpage().
>> + * Define it if the arch does not and only if needed.
>> + */
>> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
>> +{
>> +       clear_page(addr);
>> +}
>> +#endif
>
> WOuld be more obvious if you enclose that inside the same #ifdef as
> clear_user_highpage(), something like:

Yeah I was debating whether to do that or not.

> #ifndef clear_user_highpage
>
> #ifndef clear_user_page
> static inline void clear_user_page(void *addr, unsigned long vaddr, struct page
> *page)
> {
> 	clear_page(addr);
> }
> #endif
>
> static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
> {
> 	void *addr = kmap_local_page(page);
> 	clear_user_page(addr, vaddr, page);
> 	kunmap_local(addr);
> }
> #endif

On second thoughts after the discussion below it's less confusing if I
didn't tie these two together:

 #ifndef clear_user_page
 static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
 {
 	clear_page(addr);
 }
 #endif

 #ifndef clear_user_highpage
 static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
 {
 	void *addr = kmap_local_page(page);
 	clear_user_page(addr, vaddr, page);
 	kunmap_local(addr);
 }
 #endif

>> +
>>   /* when CONFIG_HIGHMEM is not set these will be plain clear/copy_page */
>>   #ifndef clear_user_highpage
>>   static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
>> And for clear_user_pages():
>> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
>> index ca9d28aa29b2..b9b3cc76a91a 100644
>> --- a/include/linux/highmem.h
>> +++ b/include/linux/highmem.h
>> @@ -223,6 +223,35 @@ static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
>>   }
>>   #endif
>> +/**
>> + * clear_user_pages() - clear a page range to be mapped to user space
>> + * @addr: start address
>> + * @vaddr: start address of the user mapping
>> + * @page: start page
>> + * @npages: number of pages
>> + *
>> + * Assumes that the region (@addr, +@npages) has been validated
>> + * already so this does no exception handling.
>> + */
>> +static inline void clear_user_pages(void *addr, unsigned long vaddr,
>> +               struct page *page, unsigned int npages)
>> +{
>> +#ifdef clear_user_page
>> +       do {
>> +               clear_user_page(addr, vaddr, page);
>> +               addr += PAGE_SIZE;
>> +               vaddr += PAGE_SIZE;
>> +               page++;
>> +       } while (--npages);
>> +#else
>> +       /*
>> +        * Prefer clear_pages() to allow for architectural optimizations
>> +        * when operations on contiguous page ranges.
>> +        */
>> +       clear_pages(addr, npages);
>> +#endif
>> +}
>
> Not sure to understand the logic. You say this is not expected to be overriden
> by architectures in the near future, then why do we need that ? Can't we do
> everything inside clear_user_highpages() for clarity ?

clear_user_page[s]() is semantically different enough from clear_user_highpages()
that I don't think it makes sense to fuse the two.

For one thing, we would have to disentangle them again if/once CONFIG_HIGHMEM
goes away.

> At the time being clear_user_page() is used exclusively by clear_user_highpage()
> so I expect clear_user_page() to only exist when CONFIG_HIGHMEM is enabled. And

The generic clear_user_page() only exists if the generic clear_user_highpage()
is defined. The arch might provide a clear_user_page() of its own (ex.
arm/sparc etc).

> in that case clear_user_highpages() doesn't call clear_user_pages() so at the
> end only the else part remains, which is a simple call to clear_pages(). Why not
> just call clear_pages() directly from clear_user_highpages() and drop
> clear_user_pages() ?

For !HIGHMEM, the clear_user_pages() might be just clear_pages() or a
loop around an architecture specific clear_user_page().

--
ankur

On 11/24/25 11:17, David Hildenbrand (Red Hat) wrote:
> On 11/23/25 12:53, Christophe Leroy (CS GROUP) wrote:
>>
>>
>> Le 21/11/2025 à 21:23, Ankur Arora a écrit :
>>> From: David Hildenbrand <david@redhat.com>
>>>
>>> Let's drop all variants that effectively map to clear_page() and
>>> provide it in a generic variant instead.
>>>
>>> We'll use the macro clear_user_page to indicate whether an architecture
>>> provides it's own variant.
>>>
>>> We have to be a bit careful if an architecture provides a custom
>>> clear_user_highpage(), because then it's very likely that some special
>>> flushing magic is happening behind the scenes.
>>>
>>> Maybe at some point these should be CONFIG_ options.
>>>
>>> Note that for parisc, clear_page() and clear_user_page() map to
>>> clear_page_asm(), so we can just get rid of the custom clear_user_page()
>>> implementation. There is a clear_user_page_asm() function on parisc,
>>> that seems to be unused. Not sure what's up with that.
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>>
>> ...
>>
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index 7c79b3369b82..6fa6c188f99a 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -3879,6 +3879,28 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
>>>     				unsigned int order) {}
>>>     #endif	/* CONFIG_DEBUG_PAGEALLOC */
>>>     
>>> +#ifndef clear_user_page
>>> +/**
>>> + * clear_user_page() - clear a page to be mapped to user space
>>> + * @addr: the address of the page
>>> + * @vaddr: the address of the user mapping
>>> + * @page: the page
>>> + */
>>> +static inline void clear_user_page(void *addr, unsigned long vaddr, struct page *page)
>>> +{
>>> +#ifdef clear_user_highpage
>>> +	/*
>>> +	 * If an architecture defines its own clear_user_highpage() variant,
>>> +	 * then we have to be a bit more careful here and cannot simply
>>> +	 * rely on clear_page().
>>> +	 */
>>> +	clear_user_highpage(page, vaddr);
>>> +#else
>>> +	clear_page(addr);
>>> +#endif
>>> +}
>>> +#endif
>>> +
>>>     #ifdef __HAVE_ARCH_GATE_AREA
>>>     extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
>>>     extern int in_gate_area_no_mm(unsigned long addr);
>>
>>
>> Isn't it chicken and egg with clear_user_highpage() in linux/highmem.h ? :
> 
> No really, because we make use of clear_user_highpage() only when the
> arch defines it, so the highmem.h variant is ignored?
> 
> Not that I particularly enjoy this way of handling it, so something
> cleaner would be nice :)
> 
> (in particular, relying on highmem.h defines in mm.h is a bit suboptimal)

Correction: there is no highmem.h dependency, as clear_user_highpage() 
comes directly from the architecture helpers in page.h

-- 
Cheers

David