Re: [RFC PATCH v4 5/5] mm/filemap: Allow arch to request folio size for exec memory

[Will Deacon <will@kernel.org>]

On Tue, May 13, 2025 at 01:46:06PM +0100, Ryan Roberts wrote:
> On 09/05/2025 14:52, Will Deacon wrote:
> > On Wed, Apr 30, 2025 at 03:59:18PM +0100, Ryan Roberts wrote:
> >> diff --git a/mm/filemap.c b/mm/filemap.c
> >> index e61f374068d4..37fe4a55c00d 100644
> >> --- a/mm/filemap.c
> >> +++ b/mm/filemap.c
> >> @@ -3252,14 +3252,40 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
> >>  	if (mmap_miss > MMAP_LOTSAMISS)
> >>  		return fpin;
> >>  
> >> -	/*
> >> -	 * mmap read-around
> >> -	 */
> >>  	fpin = maybe_unlock_mmap_for_io(vmf, fpin);
> >> -	ra->start = max_t(long, 0, vmf->pgoff - ra->ra_pages / 2);
> >> -	ra->size = ra->ra_pages;
> >> -	ra->async_size = ra->ra_pages / 4;
> >> -	ra->order = 0;
> >> +	if (vm_flags & VM_EXEC) {
> >> +		/*
> >> +		 * Allow arch to request a preferred minimum folio order for
> >> +		 * executable memory. This can often be beneficial to
> >> +		 * performance if (e.g.) arm64 can contpte-map the folio.
> >> +		 * Executable memory rarely benefits from readahead, due to its
> >> +		 * random access nature, so set async_size to 0.
> > 
> > In light of this observation (about randomness of instruction fetch), do
> > you think it's worth ignoring VM_RAND_READ for VM_EXEC?
> 
> Hmm, yeah that makes sense. Something like:
> 
> ---8<---
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 7b90cbeb4a1a..6c8bf5116c54 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3233,7 +3233,8 @@ static struct file *do_sync_mmap_readahead(struct vm_fault
> *vmf)
>         if (!ra->ra_pages)
>                 return fpin;
> 
> -       if (vm_flags & VM_SEQ_READ) {
> +       /* VM_EXEC case below is already intended for random access */
> +       if ((vm_flags & (VM_SEQ_READ | VM_EXEC)) == VM_SEQ_READ) {
>                 fpin = maybe_unlock_mmap_for_io(vmf, fpin);
>                 page_cache_sync_ra(&ractl, ra->ra_pages);
>                 return fpin;
> ---8<---

I was thinking about the:

	if (vm_flags & VM_RAND_READ)
		return fpin;

code above this which bails if VM_RAND_READ is set. That seems contrary
to the code you're adding which says that, even for random access
patterns where readahead doesn't help, it's still worth sizing the folio
appropriately for contpte mappings.

Will