On Mon, Oct 14, 2024 at 11:47:58PM -0700, Christoph Hellwig wrote:
Hi Lorenzo,
sorry for only replying to this so late.
No worries, and thanks for taking a look! :)
On Fri, Sep 27, 2024 at 01:51:11PM +0100, Lorenzo Stoakes wrote:
The existing generic pagewalk logic permits the walking of page tables, invoking callbacks at individual page table levels via user-provided mm_walk_ops callbacks.
This is useful for traversing existing page table entries, but precludes the ability to establish new ones.
Existing mechanism for performing a walk which also installs page table entries if necessary are heavily duplicated throughout the kernel, each with semantic differences from one another and largely unavailable for use elsewhere.
I do like the idea of having common code for installing page tables!
Awesome.
Minor nits below:
+int walk_page_range_mm(struct mm_struct *mm, unsigned long start, unsigned long end, const struct mm_walk_ops *ops, void *private)
It would be good to have a minimum level of documentation for this function, including how it differs from walk_page_range and why it should remain internal.
Will add on respin!
- /* For internal use only. */
- if (ops->install_pte)
return -EINVAL;And this should probably be expanded a bit, including that no exported symbol should allow inserting arbitrary PTEs. Maybe best done with a helper to share that comment with the other places that have this check.
Yeah a helper makes sense actually, a more general 'are these ops valid?' thing. Will update on next respin with some explanation.
The next iteration I plan to un-RFC as seems generally the concept is unopposed for this series, will make these changes then.
Thanks!