Re: [RFC PATCH v2 1/3] mm/huge_memory: make persistent huge zero folio read-only

[Lance Yang <lance.yang@linux.dev>]

Hi Dave,

Thanks for taking the time to review.

On Tue, Jun 09, 2026 at 12:33:36PM -0700, Dave Hansen wrote:
>On 6/9/26 07:37, Xueyuan Chen wrote:
>> +bool __weak arch_make_pages_readonly(struct page *page, int nr_pages)
>> +{
>> +	return false;
>> +}
>
>This is a rather wonky function. It's going to cause all kinds of fun if
>it is used like this:
>
>	arch_make_pages_readonly(syscall_table, 1);

Ouch, yeah, it is ...

>It's also kinda weird to have it return a bool, and not check that bool
>at the single call site. Some things come to mind:
>
>1. This function needs commenting. It needs to say what it does, when
>   architectures should override it and what their implementations
>   should look like. It needs to be clear that this can't be used for
>   anything really important. What should architectures do with alias
>   mappings? Are they allowed to touch non-direct map aliases? Are they
>   required to?

Agreed. Needs a real comment ...

Just meant as a best-effort direct/linear-map permission chang, nothing
stronger than that. I should spell out what happens, or does not happen,
to non-direct-map aliases, if anything, and make clear callers cannot
treat this as a hard guarantee :D

>2. The return type needs to be reconsidered. Is 'bool' even acceptable?
>   Should it just be 'void' if callers can't do anything when it fails?

Maybe ignoring it is OK now, but someone may need the return value later?

>3. What should the naming be? "readonly" vs "ro". Should it have a
>   "maybe" since it's kinda optional?

Fair point. "make" may be overstating it a bit ...

With a return value, arch_try_make_pages_readonly() sounds about right
to me. If we end up with void and pure best-effort semantics, maybe
arch_maybe_make_pages_readonly() fits better :)

>4. Should this new API be folio or page-based in the first place?

For page vs folio, I was mostly following David's RFC v1 suggestion.

Current caller is a folio, sure, but the page-range helper leaves room
for non-folio users later. Happy to add a simple folio wrapper if that
reads better ;)

>5. Is mm/huge_memory.c the right place to define a generic mm function,
>   even a stub?

Ah, you're right! My bad, wrong place for a generic stub. Will move it
out for RFC v3.

Thanks, Lance