Re: [PATCH v4 02/15] mm: Make empty_zero_page __ro_after_init

[Mike Rapoport <rppt@kernel.org>]

On Tue, May 12, 2026 at 02:56:16PM +0200, Ard Biesheuvel wrote:
> On Mon, 11 May 2026, at 16:40, Jann Horn wrote:
> > On Mon, May 11, 2026 at 10:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >> I think we should simply do something along the lines of the below,
> >> considering that the size of a data object tends to correlate with
> >> its minimum alignment.
> >>
> >> I do find it rather puzzling that the compiler emits empty_zero_page
> >> *after* zero_page_pfn - ideally, we'd combine the below with
> >> -fdata-sections so that the linker sees all individual objects, but
> >> I suspect that would create some problems elsewhere.
> >>
> >>
> >> --- a/include/asm-generic/vmlinux.lds.h
> >> +++ b/include/asm-generic/vmlinux.lds.h
> >> @@ -452,7 +452,7 @@
> >>  #define RO_AFTER_INIT_DATA                                   \
> >>         . = ALIGN(8);                                         \
> >>         __start_ro_after_init = .;                            \
> >> -       *(.data..ro_after_init)                               \
> >> +       *(SORT_BY_ALIGNMENT(.data..ro_after_init))            \
> >
> > Oh, neat, I didn't realize that's possible. That seems like a nicer
> > approach...
> 
> Neat but rather ineffective, unfortunately. (I don't see a size
> difference with the arm64 defconfig kernel)
> 
> Given that empty_zero_page only ever gets its address taken, we
> might just move it into the linker script if that requires tweaking
> anyway. We can just place it at the start of .rodata, which is
> already page aligned on most architectures (and will become page
> aligned unless EMPTY_ZERO_PAGE is #define'd by the arch linker
> script to something else)
> 
> 
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -472,6 +472,17 @@
>  #endif
>  #endif
>  
> +#ifndef EMPTY_ZERO_PAGE
> +#ifndef __HAVE_COLOR_ZERO_PAGE

I don't think we want let architectures that don't use colored zero pages
redefine it.
If it will be really required we can add the ability to redefine
EMPTY_ZERO_PAGE later.

> +#define EMPTY_ZERO_PAGE                                        \
> +       . = ALIGN(PAGE_SIZE);                                   \
> +       empty_zero_page = .;                                    \
> +       . += PAGE_SIZE;
> +#else
> +#define EMPTY_ZERO_PAGE
> +#endif
> +#endif
> +
>  /*
>   * Read only Data
>   */
> @@ -479,6 +490,7 @@
>         . = ALIGN((align));                                     \
>         .rodata           : AT(ADDR(.rodata) - LOAD_OFFSET) {   \
>                 __start_rodata = .;                             \
> +               EMPTY_ZERO_PAGE                                 \
>                 *(.rodata) *(.rodata.*) *(.data.rel.ro*)        \
>                 SCHED_DATA                                      \
> 

-- 
Sincerely yours,
Mike.