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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <933f4191-4075-4b02-998a-35b1711e778c@arm.com>
Date: Wed, 14 May 2025 16:31:31 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Will Deacon <will@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 David Hildenbrand <david@...hat.com>, Dave Chinner <david@...morbit.com>,
 Catalin Marinas <catalin.marinas@....com>,
 Kalesh Singh <kaleshsingh@...gle.com>, Zi Yan <ziy@...dia.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-fsdevel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [RFC PATCH v4 5/5] mm/filemap: Allow arch to request folio size
 for exec memory

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