[PATCH v2 0/1] Encapsulate the populate and alloc as one atomic

[PATCH v2 0/1] Encapsulate the populate and alloc as one atomic

[Hubert Mazur]

Hello,
thanks for the review of the v1 patchset. I tried to make v2 diff as
small as possible and without a modification of the core logic.

When a block of memory is requested from the execmem manager the
free_areas tree is traversed to find area of given size. If it is not
found then a new fragment, aligned to a PAGE_SIZE, is allocated and
added to free_areas. Afterwards, the free_areas tree is being traversed
again to fullfil the request.

The above operations of allocation and tree traversal are not atomic
hence another request may consume this newly allocated memory
block dedicated to the original request. As a result - the first
request fails to get the memory. Such occurence can be spotted on
evices running the 6.18 kernel during the paralell modules loading.

Regards
Hubert

Changes in v2:
The __execmem_cache_alloc_locked function (lockless version of
__execmem_cache_alloc) is introduced and called after
execmem_cache_add_locked from the __execmem_cache_populate_alloc
function (renamed from execmem_cache_populate). Both calls are
guarded now with a single mutex.

Changes in v1:
Allocate new memory fragment and assign it directly to the busy_areas
inside execmem_cache_populate function.

Link to v1:
https://lore.kernel.org/all/20260312131438.361746-1-hmazur@google.com/T/#t

Hubert Mazur (1):
  mm/execmem: Make the populate and alloc atomic

 mm/execmem.c | 61 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

--
2.53.0.851.ga537e3e6e9-goog

[PATCH v2 1/1] mm/execmem: Make the populate and alloc atomic

[Hubert Mazur]

When a memory block is requested from the execmem manager, it tries
to find a suitable fragment in the free_areas. In case there is no
such block, a new memory area is added to free_areas and then
allocated to the caller. Those two operations must be atomic
to ensure that no other memory request consumes it.

Signed-off-by: Hubert Mazur <hmazur@google.com>
---
 mm/execmem.c | 61 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/mm/execmem.c b/mm/execmem.c
index 810a4ba9c924..9043f0f8f61a 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -203,13 +203,6 @@ static int execmem_cache_add_locked(void *ptr, size_t size, gfp_t gfp_mask)
 	return mas_store_gfp(&mas, (void *)lower, gfp_mask);
 }
 
-static int execmem_cache_add(void *ptr, size_t size, gfp_t gfp_mask)
-{
-	guard(mutex)(&execmem_cache.mutex);
-
-	return execmem_cache_add_locked(ptr, size, gfp_mask);
-}
-
 static bool within_range(struct execmem_range *range, struct ma_state *mas,
 			 size_t size)
 {
@@ -225,18 +218,16 @@ static bool within_range(struct execmem_range *range, struct ma_state *mas,
 	return false;
 }
 
-static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
+static void *__execmem_cache_alloc_locked(struct execmem_range *range, size_t size)
 {
 	struct maple_tree *free_areas = &execmem_cache.free_areas;
 	struct maple_tree *busy_areas = &execmem_cache.busy_areas;
 	MA_STATE(mas_free, free_areas, 0, ULONG_MAX);
 	MA_STATE(mas_busy, busy_areas, 0, ULONG_MAX);
-	struct mutex *mutex = &execmem_cache.mutex;
 	unsigned long addr, last, area_size = 0;
 	void *area, *ptr = NULL;
 	int err;
 
-	mutex_lock(mutex);
 	mas_for_each(&mas_free, area, ULONG_MAX) {
 		area_size = mas_range_len(&mas_free);
 
@@ -245,7 +236,7 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
 	}
 
 	if (area_size < size)
-		goto out_unlock;
+		return NULL;
 
 	addr = mas_free.index;
 	last = mas_free.last;
@@ -254,7 +245,7 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
 	mas_set_range(&mas_busy, addr, addr + size - 1);
 	err = mas_store_gfp(&mas_busy, (void *)addr, GFP_KERNEL);
 	if (err)
-		goto out_unlock;
+		return NULL;
 
 	mas_store_gfp(&mas_free, NULL, GFP_KERNEL);
 	if (area_size > size) {
@@ -268,19 +259,25 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
 		err = mas_store_gfp(&mas_free, ptr, GFP_KERNEL);
 		if (err) {
 			mas_store_gfp(&mas_busy, NULL, GFP_KERNEL);
-			goto out_unlock;
+			return NULL;
 		}
 	}
 	ptr = (void *)addr;
 
-out_unlock:
-	mutex_unlock(mutex);
 	return ptr;
 }
 
-static int execmem_cache_populate(struct execmem_range *range, size_t size)
+static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
+{
+	guard(mutex)(&execmem_cache.mutex);
+
+	return __execmem_cache_alloc_locked(range, size);
+}
+
+static void *__execmem_cache_populate_alloc(struct execmem_range *range, size_t size)
 {
 	unsigned long vm_flags = VM_ALLOW_HUGE_VMAP;
+	struct mutex *mutex = &execmem_cache.mutex;
 	struct vm_struct *vm;
 	size_t alloc_size;
 	int err = -ENOMEM;
@@ -294,7 +291,7 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
 	}
 
 	if (!p)
-		return err;
+		return NULL;
 
 	vm = find_vm_area(p);
 	if (!vm)
@@ -307,33 +304,43 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
 	if (err)
 		goto err_free_mem;
 
-	err = execmem_cache_add(p, alloc_size, GFP_KERNEL);
-	if (err)
+	/*
+	 * New memory blocks must be propagated and allocated as an atomic operation,
+	 * otherwise it may be consumed by a parallel call to the execmem_cache_alloc
+	 * function.
+	 */
+	mutex_lock(mutex);
+	err = execmem_cache_add_locked(p, alloc_size, GFP_KERNEL);
+	if (err) {
+		mutex_unlock(mutex);
 		goto err_reset_direct_map;
+	}
 
-	return 0;
+	p = __execmem_cache_alloc_locked(range, size);
+	if (!p) {
+		mutex_unlock(mutex);
+		return NULL;
+	}
+	mutex_unlock(mutex);
+
+	return p;
 
 err_reset_direct_map:
 	execmem_set_direct_map_valid(vm, true);
 err_free_mem:
 	vfree(p);
-	return err;
+	return NULL;
 }
 
 static void *execmem_cache_alloc(struct execmem_range *range, size_t size)
 {
 	void *p;
-	int err;
 
 	p = __execmem_cache_alloc(range, size);
 	if (p)
 		return p;
 
-	err = execmem_cache_populate(range, size);
-	if (err)
-		return NULL;
-
-	return __execmem_cache_alloc(range, size);
+	return __execmem_cache_populate_alloc(range, size);
 }
 
 static inline bool is_pending_free(void *ptr)
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH v2 0/1] Encapsulate the populate and alloc as one atomic

[Mike Rapoport]

Hi Hubert,

On Tue, Mar 17, 2026 at 12:50:19PM +0000, Hubert Mazur wrote:
> Hello,
> thanks for the review of the v1 patchset. I tried to make v2 diff as
> small as possible and without a modification of the core logic.
> 
> When a block of memory is requested from the execmem manager the
> free_areas tree is traversed to find area of given size. If it is not
> found then a new fragment, aligned to a PAGE_SIZE, is allocated and
> added to free_areas. Afterwards, the free_areas tree is being traversed
> again to fullfil the request.
> 
> The above operations of allocation and tree traversal are not atomic
> hence another request may consume this newly allocated memory
> block dedicated to the original request. As a result - the first
> request fails to get the memory. Such occurence can be spotted on
> evices running the 6.18 kernel during the paralell modules loading.

typo: devices

In general, a single patch does not require cover letter, but the details
about why the patch is needed should be a part of the commit message.
 
> Regards
> Hubert
> 
> Changes in v2:
> The __execmem_cache_alloc_locked function (lockless version of
> __execmem_cache_alloc) is introduced and called after
> execmem_cache_add_locked from the __execmem_cache_populate_alloc
> function (renamed from execmem_cache_populate). Both calls are
> guarded now with a single mutex.
> 
> Changes in v1:
> Allocate new memory fragment and assign it directly to the busy_areas
> inside execmem_cache_populate function.
> 
> Link to v1:
> https://lore.kernel.org/all/20260312131438.361746-1-hmazur@google.com/T/#t
> 
> Hubert Mazur (1):
>   mm/execmem: Make the populate and alloc atomic
> 
>  mm/execmem.c | 61 +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 27 deletions(-)
> 
> --
> 2.53.0.851.ga537e3e6e9-goog
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 1/1] mm/execmem: Make the populate and alloc atomic

[Mike Rapoport]

Hi Hubert,

On Tue, Mar 17, 2026 at 12:50:20PM +0000, Hubert Mazur wrote:
> Subject: mm/execmem: Make the populate and alloc atomic
>
> When a memory block is requested from the execmem manager, it tries
> to find a suitable fragment in the free_areas. In case there is no
> such block, a new memory area is added to free_areas and then
> allocated to the caller. Those two operations must be atomic
> to ensure that no other memory request consumes it.
> 
> Signed-off-by: Hubert Mazur <hmazur@google.com>
> ---
>  mm/execmem.c | 61 +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 27 deletions(-)
> 
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 810a4ba9c924..9043f0f8f61a 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -203,13 +203,6 @@ static int execmem_cache_add_locked(void *ptr, size_t size, gfp_t gfp_mask)
>  	return mas_store_gfp(&mas, (void *)lower, gfp_mask);
>  }
>  
> -static int execmem_cache_add(void *ptr, size_t size, gfp_t gfp_mask)
> -{
> -	guard(mutex)(&execmem_cache.mutex);
> -
> -	return execmem_cache_add_locked(ptr, size, gfp_mask);
> -}
> -
>  static bool within_range(struct execmem_range *range, struct ma_state *mas,
>  			 size_t size)
>  {
> @@ -225,18 +218,16 @@ static bool within_range(struct execmem_range *range, struct ma_state *mas,
>  	return false;
>  }
>  
> -static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
> +static void *__execmem_cache_alloc_locked(struct execmem_range *range, size_t size)

No need to keep prefix underscores here.

>  {
>  	struct maple_tree *free_areas = &execmem_cache.free_areas;
>  	struct maple_tree *busy_areas = &execmem_cache.busy_areas;
>  	MA_STATE(mas_free, free_areas, 0, ULONG_MAX);
>  	MA_STATE(mas_busy, busy_areas, 0, ULONG_MAX);
> -	struct mutex *mutex = &execmem_cache.mutex;
>  	unsigned long addr, last, area_size = 0;
>  	void *area, *ptr = NULL;
>  	int err;
>  
> -	mutex_lock(mutex);
>  	mas_for_each(&mas_free, area, ULONG_MAX) {
>  		area_size = mas_range_len(&mas_free);
>  
> @@ -245,7 +236,7 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
>  	}
>  
>  	if (area_size < size)
> -		goto out_unlock;
> +		return NULL;
>  
>  	addr = mas_free.index;
>  	last = mas_free.last;
> @@ -254,7 +245,7 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
>  	mas_set_range(&mas_busy, addr, addr + size - 1);
>  	err = mas_store_gfp(&mas_busy, (void *)addr, GFP_KERNEL);
>  	if (err)
> -		goto out_unlock;
> +		return NULL;
>  
>  	mas_store_gfp(&mas_free, NULL, GFP_KERNEL);
>  	if (area_size > size) {
> @@ -268,19 +259,25 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
>  		err = mas_store_gfp(&mas_free, ptr, GFP_KERNEL);
>  		if (err) {
>  			mas_store_gfp(&mas_busy, NULL, GFP_KERNEL);
> -			goto out_unlock;
> +			return NULL;
>  		}
>  	}
>  	ptr = (void *)addr;
>  
> -out_unlock:
> -	mutex_unlock(mutex);
>  	return ptr;
>  }
>  
> -static int execmem_cache_populate(struct execmem_range *range, size_t size)
> +static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
> +{
> +	guard(mutex)(&execmem_cache.mutex);
> +
> +	return __execmem_cache_alloc_locked(range, size);
> +}
> +
> +static void *__execmem_cache_populate_alloc(struct execmem_range *range, size_t size)

No need for leading underscores here as well.

>  {
>  	unsigned long vm_flags = VM_ALLOW_HUGE_VMAP;
> +	struct mutex *mutex = &execmem_cache.mutex;
>  	struct vm_struct *vm;
>  	size_t alloc_size;
>  	int err = -ENOMEM;
> @@ -294,7 +291,7 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
>  	}
>  
>  	if (!p)
> -		return err;
> +		return NULL;
>  
>  	vm = find_vm_area(p);
>  	if (!vm)
> @@ -307,33 +304,43 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
>  	if (err)
>  		goto err_free_mem;
>  
> -	err = execmem_cache_add(p, alloc_size, GFP_KERNEL);
> -	if (err)
> +	/*
> +	 * New memory blocks must be propagated and allocated as an atomic operation,
> +	 * otherwise it may be consumed by a parallel call to the execmem_cache_alloc
> +	 * function.
> +	 */

Please keep the comment lines under 80 characters.

> +	mutex_lock(mutex);
> +	err = execmem_cache_add_locked(p, alloc_size, GFP_KERNEL);
> +	if (err) {
> +		mutex_unlock(mutex);
>  		goto err_reset_direct_map;

Please add a new label that will unlock the mutex and goto to the new
label.

> +	}
>  
> -	return 0;
> +	p = __execmem_cache_alloc_locked(range, size);
> +	if (!p) {
> +		mutex_unlock(mutex);
> +		return NULL;

This if is not needed, we anyway unlock the mutex and return p, if
__execmem_cache_alloc_locked() failed p would be NULL anyway.

> +	}
> +	mutex_unlock(mutex);
> +
> +	return p;
>  
>  err_reset_direct_map:
>  	execmem_set_direct_map_valid(vm, true);
>  err_free_mem:
>  	vfree(p);
> -	return err;
> +	return NULL;
>  }
>  
>  static void *execmem_cache_alloc(struct execmem_range *range, size_t size)
>  {
>  	void *p;
> -	int err;
>  
>  	p = __execmem_cache_alloc(range, size);
>  	if (p)
>  		return p;
>  
> -	err = execmem_cache_populate(range, size);
> -	if (err)
> -		return NULL;
> -
> -	return __execmem_cache_alloc(range, size);
> +	return __execmem_cache_populate_alloc(range, size);
>  }
>  
>  static inline bool is_pending_free(void *ptr)
> -- 
> 2.53.0.851.ga537e3e6e9-goog
> 

-- 
Sincerely yours,
Mike.