Re: [RFC PATCH 1/4] mm: pagewalk: add the ability to install PTEs

[Christoph Hellwig]

Hi Lorenzo,

sorry for only replying to this so late.

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!

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.

> +	/* 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.