On 14/05/2025 16:14, Will Deacon wrote:
> 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;
Yes sorry, I lost my mind when doing that patch... I intended to do it for the
VM_RAND_READ as you suggested, but my fingers did something completely different.
>
> 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.
Anyway, I totally agree with this. So I'll avoid the early return VM_RAND_READ
if VM_EXEC is also set.
Thanks,
Ryan
>
> Will
