linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc()
@ 2025-09-18  9:34 Dev Jain
  2025-09-18 10:33 ` Mike Rapoport
  2025-09-22  6:02 ` Anshuman Khandual
  0 siblings, 2 replies; 6+ messages in thread
From: Dev Jain @ 2025-09-18  9:34 UTC (permalink / raw)
  To: akpm; +Cc: rppt, ryan.roberts, linux-mm, linux-kernel, Dev Jain

When using vmalloc with VM_ALLOW_HUGE_VMAP flag, it will set the alignment
to PMD_SIZE internally, if it deems huge mappings to be eligible.
Therefore, setting the alignment in execmem_vmalloc is redundant. Apart
from this, it also reduces the probability of allocation in case vmalloc
fails to allocate hugepages - in the fallback case, vmalloc tries to use
the original alignment and allocate basepages, which unfortunately will
again be PMD_SIZE passed over from execmem_vmalloc, thus constraining
the search for a free space in vmalloc region.

Therefore, remove this constraint.

Signed-off-by: Dev Jain <dev.jain@arm.com>
---
mm-selftests pass, but I am not sure if they touch execmem code, and I
have no experience with this code.

 mm/execmem.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/mm/execmem.c b/mm/execmem.c
index 0822305413ec..810a4ba9c924 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -38,9 +38,6 @@ static void *execmem_vmalloc(struct execmem_range *range, size_t size,
 	if (kasan)
 		vm_flags |= VM_DEFER_KMEMLEAK;
 
-	if (vm_flags & VM_ALLOW_HUGE_VMAP)
-		align = PMD_SIZE;
-
 	p = __vmalloc_node_range(size, align, start, end, gfp_flags,
 				 pgprot, vm_flags, NUMA_NO_NODE,
 				 __builtin_return_address(0));
-- 
2.30.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc()
  2025-09-18  9:34 [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc() Dev Jain
@ 2025-09-18 10:33 ` Mike Rapoport
  2025-09-18 13:06   ` Ryan Roberts
  2025-09-22  6:02 ` Anshuman Khandual
  1 sibling, 1 reply; 6+ messages in thread
From: Mike Rapoport @ 2025-09-18 10:33 UTC (permalink / raw)
  To: Dev Jain; +Cc: akpm, ryan.roberts, linux-mm, linux-kernel

On Thu, Sep 18, 2025 at 03:04:53PM +0530, Dev Jain wrote:
> When using vmalloc with VM_ALLOW_HUGE_VMAP flag, it will set the alignment
> to PMD_SIZE internally, if it deems huge mappings to be eligible.
> Therefore, setting the alignment in execmem_vmalloc is redundant. Apart
> from this, it also reduces the probability of allocation in case vmalloc
> fails to allocate hugepages - in the fallback case, vmalloc tries to use
> the original alignment and allocate basepages, which unfortunately will
> again be PMD_SIZE passed over from execmem_vmalloc, thus constraining
> the search for a free space in vmalloc region.
> 
> Therefore, remove this constraint.
> 
> Signed-off-by: Dev Jain <dev.jain@arm.com>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
> mm-selftests pass, but I am not sure if they touch execmem code, and I
> have no experience with this code.
> 
>  mm/execmem.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 0822305413ec..810a4ba9c924 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -38,9 +38,6 @@ static void *execmem_vmalloc(struct execmem_range *range, size_t size,
>  	if (kasan)
>  		vm_flags |= VM_DEFER_KMEMLEAK;
>  
> -	if (vm_flags & VM_ALLOW_HUGE_VMAP)
> -		align = PMD_SIZE;
> -
>  	p = __vmalloc_node_range(size, align, start, end, gfp_flags,
>  				 pgprot, vm_flags, NUMA_NO_NODE,
>  				 __builtin_return_address(0));
> -- 
> 2.30.2
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc()
  2025-09-18 10:33 ` Mike Rapoport
@ 2025-09-18 13:06   ` Ryan Roberts
  2025-09-21  7:39     ` Mike Rapoport
  0 siblings, 1 reply; 6+ messages in thread
From: Ryan Roberts @ 2025-09-18 13:06 UTC (permalink / raw)
  To: Mike Rapoport, Dev Jain; +Cc: akpm, linux-mm, linux-kernel

On 18/09/2025 11:33, Mike Rapoport wrote:
> On Thu, Sep 18, 2025 at 03:04:53PM +0530, Dev Jain wrote:
>> When using vmalloc with VM_ALLOW_HUGE_VMAP flag, it will set the alignment
>> to PMD_SIZE internally, if it deems huge mappings to be eligible.
>> Therefore, setting the alignment in execmem_vmalloc is redundant. Apart
>> from this, it also reduces the probability of allocation in case vmalloc
>> fails to allocate hugepages - in the fallback case, vmalloc tries to use
>> the original alignment and allocate basepages, which unfortunately will
>> again be PMD_SIZE passed over from execmem_vmalloc, thus constraining
>> the search for a free space in vmalloc region.
>>
>> Therefore, remove this constraint.
>>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
> 
> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Hijacking this thread to ask Mike a related question, which I noticed during
code review...

execmem_alloc() determines the pgprot from the descriptor set by the
architecture, and for !CONFIG_ARCH_HAS_EXECMEM_ROX (a la arm64), passes the
value into __vmalloc_node_range() (via execmem_vmalloc()). vmalloc then uses the
pgprot to set the permissions of the vmap. But the linear map is left as is
(i.e. RW).

arm64 requests PAGE_KERNEL_ROX for the EXECMEM_KPROBES type. So by my reckoning,
the memory ends up ROX in vmap and RW in linear map. Naively that sounds like
something we should be avoiding? Is this intentional?

For arches using the ROX cache, execmem_cache_populate() calls set_memory_rox()
which sets both vmap and linear map to ROX.

Thanks,
Ryan

> 
>> ---
>> mm-selftests pass, but I am not sure if they touch execmem code, and I
>> have no experience with this code.
>>
>>  mm/execmem.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/mm/execmem.c b/mm/execmem.c
>> index 0822305413ec..810a4ba9c924 100644
>> --- a/mm/execmem.c
>> +++ b/mm/execmem.c
>> @@ -38,9 +38,6 @@ static void *execmem_vmalloc(struct execmem_range *range, size_t size,
>>  	if (kasan)
>>  		vm_flags |= VM_DEFER_KMEMLEAK;
>>  
>> -	if (vm_flags & VM_ALLOW_HUGE_VMAP)
>> -		align = PMD_SIZE;
>> -
>>  	p = __vmalloc_node_range(size, align, start, end, gfp_flags,
>>  				 pgprot, vm_flags, NUMA_NO_NODE,
>>  				 __builtin_return_address(0));
>> -- 
>> 2.30.2
>>
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc()
  2025-09-18 13:06   ` Ryan Roberts
@ 2025-09-21  7:39     ` Mike Rapoport
  0 siblings, 0 replies; 6+ messages in thread
From: Mike Rapoport @ 2025-09-21  7:39 UTC (permalink / raw)
  To: Ryan Roberts; +Cc: Dev Jain, akpm, linux-mm, linux-kernel

Hi Ryan,

On Thu, Sep 18, 2025 at 02:06:25PM +0100, Ryan Roberts wrote:
> On 18/09/2025 11:33, Mike Rapoport wrote:
> > On Thu, Sep 18, 2025 at 03:04:53PM +0530, Dev Jain wrote:
> >> When using vmalloc with VM_ALLOW_HUGE_VMAP flag, it will set the alignment
> >> to PMD_SIZE internally, if it deems huge mappings to be eligible.
> >> Therefore, setting the alignment in execmem_vmalloc is redundant. Apart
> >> from this, it also reduces the probability of allocation in case vmalloc
> >> fails to allocate hugepages - in the fallback case, vmalloc tries to use
> >> the original alignment and allocate basepages, which unfortunately will
> >> again be PMD_SIZE passed over from execmem_vmalloc, thus constraining
> >> the search for a free space in vmalloc region.
> >>
> >> Therefore, remove this constraint.
> >>
> >> Signed-off-by: Dev Jain <dev.jain@arm.com>
> > 
> > Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> Hijacking this thread to ask Mike a related question, which I noticed during
> code review...

Replied on the related thread :)
https://lore.kernel.org/all/aM-rDD-TRqmtr6Nb@kernel.org/

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc()
  2025-09-18  9:34 [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc() Dev Jain
  2025-09-18 10:33 ` Mike Rapoport
@ 2025-09-22  6:02 ` Anshuman Khandual
  2025-09-22  6:50   ` Dev Jain
  1 sibling, 1 reply; 6+ messages in thread
From: Anshuman Khandual @ 2025-09-22  6:02 UTC (permalink / raw)
  To: Dev Jain, akpm; +Cc: rppt, ryan.roberts, linux-mm, linux-kernel



On 18/09/25 3:04 PM, Dev Jain wrote:
> When using vmalloc with VM_ALLOW_HUGE_VMAP flag, it will set the alignment
> to PMD_SIZE internally, if it deems huge mappings to be eligible.
> Therefore, setting the alignment in execmem_vmalloc is redundant. Apart
> from this, it also reduces the probability of allocation in case vmalloc
> fails to allocate hugepages - in the fallback case, vmalloc tries to use
> the original alignment and allocate basepages, which unfortunately will
> again be PMD_SIZE passed over from execmem_vmalloc, thus constraining
> the search for a free space in vmalloc region.
> 
> Therefore, remove this constraint.
> 
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> mm-selftests pass, but I am not sure if they touch execmem code, and I
> have no experience with this code.
> 
>  mm/execmem.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 0822305413ec..810a4ba9c924 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -38,9 +38,6 @@ static void *execmem_vmalloc(struct execmem_range *range, size_t size,
>  	if (kasan)
>  		vm_flags |= VM_DEFER_KMEMLEAK;
>  
> -	if (vm_flags & VM_ALLOW_HUGE_VMAP)
> -		align = PMD_SIZE;
> -

So if the above assignment is getting dropped here, probably the local
variable 'align' could be dropped as well and range->alignment be used
directly instead ?
>  	p = __vmalloc_node_range(size, align, start, end, gfp_flags,
>  				 pgprot, vm_flags, NUMA_NO_NODE,
>  				 __builtin_return_address(0));



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc()
  2025-09-22  6:02 ` Anshuman Khandual
@ 2025-09-22  6:50   ` Dev Jain
  0 siblings, 0 replies; 6+ messages in thread
From: Dev Jain @ 2025-09-22  6:50 UTC (permalink / raw)
  To: Anshuman Khandual, akpm; +Cc: rppt, ryan.roberts, linux-mm, linux-kernel


On 22/09/25 11:32 am, Anshuman Khandual wrote:
>
> On 18/09/25 3:04 PM, Dev Jain wrote:
>> When using vmalloc with VM_ALLOW_HUGE_VMAP flag, it will set the alignment
>> to PMD_SIZE internally, if it deems huge mappings to be eligible.
>> Therefore, setting the alignment in execmem_vmalloc is redundant. Apart
>> from this, it also reduces the probability of allocation in case vmalloc
>> fails to allocate hugepages - in the fallback case, vmalloc tries to use
>> the original alignment and allocate basepages, which unfortunately will
>> again be PMD_SIZE passed over from execmem_vmalloc, thus constraining
>> the search for a free space in vmalloc region.
>>
>> Therefore, remove this constraint.
>>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
>> mm-selftests pass, but I am not sure if they touch execmem code, and I
>> have no experience with this code.
>>
>>   mm/execmem.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/mm/execmem.c b/mm/execmem.c
>> index 0822305413ec..810a4ba9c924 100644
>> --- a/mm/execmem.c
>> +++ b/mm/execmem.c
>> @@ -38,9 +38,6 @@ static void *execmem_vmalloc(struct execmem_range *range, size_t size,
>>   	if (kasan)
>>   		vm_flags |= VM_DEFER_KMEMLEAK;
>>   
>> -	if (vm_flags & VM_ALLOW_HUGE_VMAP)
>> -		align = PMD_SIZE;
>> -
> So if the above assignment is getting dropped here, probably the local
> variable 'align' could be dropped as well and range->alignment be used
> directly instead ?

Sure, but that isn't a big deal. Replacing with range->aligment will
push the arguments declaration into an extra line in __vmalloc_node_range.
So will prefer not respinning for this triviality, this has already
been pulled into mm-new :)

>>   	p = __vmalloc_node_range(size, align, start, end, gfp_flags,
>>   				 pgprot, vm_flags, NUMA_NO_NODE,
>>   				 __builtin_return_address(0));


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-09-22  6:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-18  9:34 [PATCH] mm: Remove PMD alignment constraint in execmem_vmalloc() Dev Jain
2025-09-18 10:33 ` Mike Rapoport
2025-09-18 13:06   ` Ryan Roberts
2025-09-21  7:39     ` Mike Rapoport
2025-09-22  6:02 ` Anshuman Khandual
2025-09-22  6:50   ` Dev Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox