Re: [PATCH] mm/mlock: skip __mm_populate() for MLOCK_ONFAULT

[Vlastimil Babka (SUSE)]

On 7/22/26 14:59, Lorenzo Stoakes (ARM) wrote:
> On Wed, Jul 22, 2026 at 08:51:33PM +0800, Wandun Chen wrote:
>> From: Wandun Chen <chenwandun@xxxxxxxxxxx>
>>
>> MLOCK_ONFAULT only locks pages on future faults, so there is no need to
>> fault in non-present pages during the mlock2/mlockall syscall.
>> populate_vma_page_range() already returns immediately for VMAs with
>> VM_LOCKONFAULT, which means __mm_populate() just loops over VMAs and
>> holds mmap_read_lock without doing useful work.
>>
>> Skip __mm_populate() when MLOCK_ONFAULT is set to avoid this unnecessary
>> work.
>>
>> Signed-off-by: Wandun Chen <chenwandun@xxxxxxxxxxx>
> 
> Well you take mmap read lock and release it (after having held the write lock),
> hardly earth-shattering.
> 
> And this has been this way for donkey's years I don't really see why we should
> care?
> 
> Do you have a workload that's heavily dependent on mlock2(..., MLOCK_ONFAULT) or
> mlockall(..., MCL_ONFAULT) as a hot path that is seriously contending the mmap
> lock?
> 
> I don't love how the VMA_LOCKONFAULT_BIT flag works but I'm not sure adding more
> churn and code for the sake of it here is really worth it.
> 
> Thanks, Lorenzo

While that's true, I don't see a problem taking the patch. Otherwise we'll
have to keep rejecting all future attempts (including LLM) doing the same
because it just seems like a low hanging fruit to stop doing an unnecessary
thing.

And it's not that much of a churn or new code, IMHO.

>> ---
>>  mm/mlock.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/mlock.c b/mm/mlock.c
>> index efa6716e4dfb..784bd4bfc3bb 100644
>> --- a/mm/mlock.c
>> +++ b/mm/mlock.c
>> @@ -658,9 +658,11 @@ static __must_check int do_mlock(unsigned long start, size_t len,
>>  	if (error)
>>  		return error;
>>
>> -	error = __mm_populate(start, len, 0);
>> -	if (error)
>> -		return __mlock_posix_error_return(error);
>> +	if (!vma_flags_test(flags, VMA_LOCKONFAULT_BIT)) {
>> +		error = __mm_populate(start, len, 0);
>> +		if (error)
>> +			return __mlock_posix_error_return(error);
>> +	}
>>  	return 0;
>>  }
>>
>> @@ -778,7 +780,7 @@ SYSCALL_DEFINE1(mlockall, int, flags)
>>  	    capable(CAP_IPC_LOCK))
>>  		ret = apply_mlockall_flags(flags);
>>  	mmap_write_unlock(current->mm);
>> -	if (!ret && (flags & MCL_CURRENT))
>> +	if (!ret && (flags & MCL_CURRENT) && !(flags & MCL_ONFAULT))
>>  		mm_populate(0, TASK_SIZE);
>>
>>  	return ret;
>> --
>> 2.43.0
>>