Re: [PATCH v7 10/16] mm: define clear_pages(), clear_user_pages()
From: David Hildenbrand
Date: Tue Sep 30 2025 - 05:43:26 EST
On 25.09.25 07:25, Ankur Arora wrote:
David Hildenbrand <david@xxxxxxxxxx> writes:
On 23.09.25 22:26, Ankur Arora wrote:
David Hildenbrand <david@xxxxxxxxxx> writes:
On 17.09.25 17:24, Ankur Arora wrote:So, comments that actually speak to the use instead of technically
Define fallback versions of clear_pages(), clear_user_pages().
In absence of architectural primitives, we just clear pages
sequentially.
Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
---
include/linux/mm.h | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1ae97a0b8ec7..0cde9b01da5e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3768,6 +3768,44 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
unsigned int order) {}
#endif /* CONFIG_DEBUG_PAGEALLOC */
+#ifndef clear_pages
+/**
+ * clear_pages() - clear a page range using a kernel virtual address.
I'd just call this "clear a page range for kernel-internal use"
+ * @addr: start address
+ * @npages: number of pages
+ *
+ * Assumes that (@addr, +@npages) references a kernel region.
And say here simply that "Use clear_user_pages() instead for clearing a page
range to be mapped to user space".
correct but unhelpful generalities :). Thanks, good lesson.
Makes sense.+ * Does absolutely no exception handling.
+ */
+static inline void clear_pages(void *addr, unsigned int npages)
+{
+ do {
+ clear_page(addr);
+ addr += PAGE_SIZE;
+ } while (--npages);
+}
+#endif
+
+#ifndef clear_user_pages
+/**
+ * clear_user_pages() - clear a page range mapped by the user.
I'd call this then "clear a page range to be mapped to user space"
Because it's usually called before we actually map it and it will properly flush
the dcache if required.
Will do. Also acking to the ones above.+ * @addr: kernel mapped address
"start address"
+ * @vaddr: user mapped address
"start address of the user mapping" ?
+ * @pg: start page
Please just call it "page". I know, clear_user_page() has this weird page vs. pg
thingy, but let's do it better here.
+ * @npages: number of pages
+ *
+ * Assumes that the region (@addr, +@npages) has been validated
+ * already so this does no exception handling.
+ */
+#define clear_user_pages(addr, vaddr, pg, npages) \
+do { \
+ clear_user_page(addr, vaddr, pg); \
+ addr += PAGE_SIZE; \
+ vaddr += PAGE_SIZE; \
+ pg++; \
+} while (--npages)
+#endif
Should indent with one tab.
Any reason this is not a static inline function?Alas yes. Most architecture code defines clear_user_page() as a macro
where, if they need a to flush the dcache or otherwise do something
special, they need access to some external primitive. And this primitive
which might not be visible in contexts that we include this header.
For instance this one on sparc:
https://lore.kernel.org/lkml/202509030338.DlQJTxIk-lkp@xxxxxxxxx/
Defining as a macro to get around that. But maybe there's a better
way?
Can we just move it to mm/utils.c and not have it be an inline function?
Thanks. Yeah, that's a good place for it.
If the function call overhead is ever a problem, we can look into reworking that.
--
Cheers
David / dhildenb