Re: [RFC PATCH] binfmt_elf: Align eligible read-only PT_LOAD segments to PMD_SIZE for THP

[hev <r@hev.cc>]

On Tue, Mar 3, 2026 at 12:46 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Mar 02, 2026 at 11:50:46PM +0800, WANG Rui wrote:
> > +config ELF_RO_LOAD_THP_ALIGNMENT
> > +     bool "Align read-only ELF load segments for THP (EXPERIMENTAL)"
> > +     depends on READ_ONLY_THP_FOR_FS
>
> This doesn't deserve a config option.

This optimization is not entirely free. Increasing PT_LOAD alignment
can waste virtual address space, which is especially significant on
32-bit systems, and it also reduces ASLR entropy by limiting the
number of possible load addresses.

In addition, coarser alignment may have secondary microarchitectural
effects (eg. on indirect branch prediction), depending on the
workload. Because this change affects address space layout and
security-related properties, providing users with a way to opt out is
reasonable, rather than making it completely unconditional. This
behavior fits naturally under READ_ONLY_THP_FOR_FS.

>
> > +#if defined(CONFIG_ELF_RO_LOAD_THP_ALIGNMENT) && PMD_SIZE <= SZ_32M
>
> Why 32MB?  This is weird and not justified anywhere.

The 32MB limit is intended as a practical upper bound. With 16KB base
pages, which are commonly used on some 64-bit architectures, PMD_SIZE
is 32MB. Larger PMD sizes (eg. 128MB or more with 32KB pages) exist
but represent relatively extreme configurations where alignment costs,
virtual address space padding, and ASLR entropy loss grow
disproportionately relative to the expected THP benefit.

On 32-bit systems, this issue is even more pronounced: PMD_SIZE can
reach 128MB or 256MB depending on configuration, which is clearly
unreasonable as an alignment requirement given the already constrained
virtual address space. The cap therefore avoids these pathological
cases while still covering all realistic and widely deployed PMD huge
page sizes.

Thanks,
Rui

>
> > +                     if (hugepage_global_always() && !(cmds[i].p_flags & PF_W)
> > +                             && IS_ALIGNED(cmds[i].p_vaddr | cmds[i].p_offset, PMD_SIZE)
> > +                             && cmds[i].p_filesz >= PMD_SIZE && p_align < PMD_SIZE)
> > +                             p_align = PMD_SIZE;
>
> Normal style is to put the '&&' at the end of the line:
>
>                         if (!(cmds[i].p_flags & PF_W) &&
>                             IS_ALIGNED(cmds[i].p_vaddr | cmds[i].p_offset, PMD_SIZE) &&
>                             cmds[i].p_filesz >= PMD_SIZE && p_align < PMD_SIZE))
>                                 p_align = PMD_SIZE;
>
> But this conditional is too complex to be at this level of indentation.
> Factor it out into a helper:
>
>                         if (align_to_pmd(cmds) && p_align < PMD_SIZE)
>                                 p_align = PMD_SIZE;
>