Re: [PATCH v2 3/4] elf: align ET_DYN base to max folio size for PTE coalescing

Usama Arif
2026-03-27T16:52:09Z

On 20/03/2026 18:58, Matthew Wilcox wrote:
> On Fri, Mar 20, 2026 at 06:58:53AM -0700, Usama Arif wrote:
>> -static unsigned long maximum_alignment(struct elf_phdr *cmds, int nr)
>> +static unsigned long maximum_alignment(struct elf_phdr *cmds, int nr,
>> + struct file *filp)
>> {
>> unsigned long alignment = 0;
>> + unsigned long max_folio_size = PAGE_SIZE;
>> int i;
>>
>> + if (filp && filp->f_mapping)
>> + max_folio_size = mapping_max_folio_size(filp->f_mapping);
>
> Under what circumstances can bprm->file be NULL?

Yeah its unnecessary here. Its used in other places and this is never
checked, so we can remove it.

>
> Also we tend to prefer the name "file" rather than "filp" for new code
> (yes, there's a lot of old code out there).
>

ack. will change in next revision.

>> +
>> + /*
>> + * Try to align the binary to the largest folio
>> + * size that the page cache supports, so the
>> + * hardware can coalesce PTEs (e.g. arm64
>> + * contpte) or use PMD mappings for large folios.
>> + *
>> + * Use the largest power-of-2 that fits within
>> + * the segment size, capped by what the page
>> + * cache will allocate. Only align when the
>> + * segment's virtual address and file offset are
>> + * already aligned to the folio size, as
>> + * misalignment would prevent coalescing anyway.
>> + *
>> + * The segment size check avoids reducing ASLR
>> + * entropy for small binaries that cannot
>> + * benefit.
>> + */
>> + if (!cmds[i].p_filesz)
>> + continue;
>> + size = rounddown_pow_of_two(cmds[i].p_filesz);
>> + size = min(size, max_folio_size);
>> + if (size > PAGE_SIZE &&
>> + IS_ALIGNED(cmds[i].p_vaddr, size) &&
>> + IS_ALIGNED(cmds[i].p_offset, size))
>> + alignment = max(alignment, size);
>
> Can this not all be factored out into a different function? Also, I
> think it was done a bit better here:
> https://lore.kernel.org/linux-fsdevel/20260313005211.882831-1-r@hev.cc/
>
> + if (!IS_ALIGNED(cmd->p_vaddr | cmd->p_offset, PMD_SIZE))
> + return false;
>

ack, will try and address this accordingly.

Thanks for the reviews!!