dev.dpdk.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] EAL memory fixes
@ 2025-08-14 21:32 Jake Freeland
  2025-08-14 21:32 ` [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection Jake Freeland
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jake Freeland @ 2025-08-14 21:32 UTC (permalink / raw)
  To: Anatoly Burakov, Bruce Richardson; +Cc: Jake Freeland, dev

Hi there,

This patchset contains a number of EAL-specific memory fixes that I've
made over the last year. Two pertain to FreeBSD, one pertains to Linux.

v2:
* Log messages are no longer split across multiple lines.
* The patch titled "Do not index out of bounds in memseg list" has been
  removed since there was no indexing out of bounds happening.
* A new patch has been added per Anatoly's recommendation that starts
  searching for memseg entries after the last used entry.

Jake Freeland (3):
  eal/freebsd: Do not use prev_ms_idx for hole detection
  eal/freebsd: Avoid claiming memseg holes
  eal/linux: Check hugepage access permissions

 lib/eal/freebsd/eal_memory.c      | 24 +++++++++++++++++++-----
 lib/eal/linux/eal_hugepage_info.c |  7 +++++++
 2 files changed, 26 insertions(+), 5 deletions(-)

--
2.47.2


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

* [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection
  2025-08-14 21:32 [PATCH v2 0/3] EAL memory fixes Jake Freeland
@ 2025-08-14 21:32 ` Jake Freeland
  2025-10-13 12:23   ` Burakov, Anatoly
  2025-08-14 21:32 ` [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes Jake Freeland
  2025-08-14 21:32 ` [PATCH v2 3/3] eal/linux: Check hugepage access permissions Jake Freeland
  2 siblings, 1 reply; 8+ messages in thread
From: Jake Freeland @ 2025-08-14 21:32 UTC (permalink / raw)
  To: Anatoly Burakov, Bruce Richardson; +Cc: Jake Freeland, dev

Use rte_fbarray_is_used() to check if the previous fbarray entry is
already empty.

Using prev_ms_idx to do this is flawed in cases where we loop through
multiple memseg lists. Each memseg list has its own count and length,
so using a prev_ms_idx from one memseg list to check for used entries
in another non-empty memseg list can lead to incorrect hole placement.

Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
---
 lib/eal/freebsd/eal_memory.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index 6d3d46a390..be3bde2cb9 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -103,7 +103,6 @@ rte_eal_hugepage_init(void)
 	for (i = 0; i < internal_conf->num_hugepage_sizes; i++) {
 		struct hugepage_info *hpi;
 		rte_iova_t prev_end = 0;
-		int prev_ms_idx = -1;
 		uint64_t page_sz, mem_needed;
 		unsigned int n_pages, max_pages;
 
@@ -167,9 +166,9 @@ rte_eal_hugepage_init(void)
 				if (ms_idx < 0)
 					continue;
 
-				if (need_hole && prev_ms_idx == ms_idx - 1)
+				if (need_hole &&
+				    rte_fbarray_is_used(arr, ms_idx - 1))
 					ms_idx++;
-				prev_ms_idx = ms_idx;
 
 				break;
 			}
-- 
2.47.2


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

* [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes
  2025-08-14 21:32 [PATCH v2 0/3] EAL memory fixes Jake Freeland
  2025-08-14 21:32 ` [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection Jake Freeland
@ 2025-08-14 21:32 ` Jake Freeland
  2025-10-13 12:36   ` Burakov, Anatoly
  2025-10-13 13:12   ` Burakov, Anatoly
  2025-08-14 21:32 ` [PATCH v2 3/3] eal/linux: Check hugepage access permissions Jake Freeland
  2 siblings, 2 replies; 8+ messages in thread
From: Jake Freeland @ 2025-08-14 21:32 UTC (permalink / raw)
  To: Anatoly Burakov, Bruce Richardson; +Cc: Jake Freeland, dev

When need_hole is false, memseg searches will only be done for a single
element. If the search starts at beginning of the list, an element that
was previously reserved as a hole may be wrongly claimed.

To avoid this, begin the search following the last used entry. This way,
we ignore all pre-existing holes.

Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
---
 lib/eal/freebsd/eal_memory.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index be3bde2cb9..b159e9ef4e 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -143,6 +143,7 @@ rte_eal_hugepage_init(void)
 
 			for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS;
 					msl_idx++) {
+				int start_idx, num_elems;
 				bool empty, need_hole;
 				msl = &mcfg->memsegs[msl_idx];
 				arr = &msl->memseg_arr;
@@ -157,10 +158,24 @@ rte_eal_hugepage_init(void)
 				 * adjacent to current one.
 				 */
 				need_hole = !empty && !is_adjacent;
+				if (need_hole) {
+					start_idx = 0;
+					/* we need 1, plus hole */
+					num_elems = 2;
+				} else {
+					/* begin our search after the last used
+					 * element in the list, skipping over
+					 * any previously placed holes
+					 */
+					start_idx = rte_fbarray_find_prev_n_used(
+							arr, arr->len - 1, 1) + 1;
+					if (start_idx < 0)
+						start_idx = 0;
+					num_elems = 1;
+				}
 
-				/* we need 1, plus hole if not adjacent */
 				ms_idx = rte_fbarray_find_next_n_free(arr,
-						0, 1 + (need_hole ? 1 : 0));
+						start_idx, num_elems);
 
 				/* memseg list is full? */
 				if (ms_idx < 0)
-- 
2.47.2


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

* [PATCH v2 3/3] eal/linux: Check hugepage access permissions
  2025-08-14 21:32 [PATCH v2 0/3] EAL memory fixes Jake Freeland
  2025-08-14 21:32 ` [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection Jake Freeland
  2025-08-14 21:32 ` [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes Jake Freeland
@ 2025-08-14 21:32 ` Jake Freeland
  2025-10-13 12:43   ` Burakov, Anatoly
  2 siblings, 1 reply; 8+ messages in thread
From: Jake Freeland @ 2025-08-14 21:32 UTC (permalink / raw)
  To: Anatoly Burakov, Bruce Richardson; +Cc: Jake Freeland, dev

Currently, hugepage mountpoints will be used irrespective of permissions,
leading to potential EACCES errors during memory allocation. Fix this by
not using a mountpoint if we do not have read/write permissions on it.

Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
---
 lib/eal/linux/eal_hugepage_info.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/eal/linux/eal_hugepage_info.c b/lib/eal/linux/eal_hugepage_info.c
index d47a19c56a..e2ddd6218b 100644
--- a/lib/eal/linux/eal_hugepage_info.c
+++ b/lib/eal/linux/eal_hugepage_info.c
@@ -260,6 +260,13 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int len)
 				continue;
 		}
 
+		if (access(splitstr[MOUNTPT], R_OK | W_OK) < 0) {
+			EAL_LOG(NOTICE,
+				"Skipping hugepage dir '%s': missing r/w perms",
+				splitstr[MOUNTPT]);
+			continue;
+		}
+
 		/*
 		 * If no --huge-dir option has been given, we're done.
 		 */
-- 
2.47.2


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

* Re: [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection
  2025-08-14 21:32 ` [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection Jake Freeland
@ 2025-10-13 12:23   ` Burakov, Anatoly
  0 siblings, 0 replies; 8+ messages in thread
From: Burakov, Anatoly @ 2025-10-13 12:23 UTC (permalink / raw)
  To: Jake Freeland, Bruce Richardson; +Cc: dev

On 8/14/2025 11:32 PM, Jake Freeland wrote:
> Use rte_fbarray_is_used() to check if the previous fbarray entry is
> already empty.
> 
> Using prev_ms_idx to do this is flawed in cases where we loop through
> multiple memseg lists. Each memseg list has its own count and length,
> so using a prev_ms_idx from one memseg list to check for used entries
> in another non-empty memseg list can lead to incorrect hole placement.
> 
> Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

* Re: [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes
  2025-08-14 21:32 ` [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes Jake Freeland
@ 2025-10-13 12:36   ` Burakov, Anatoly
  2025-10-13 13:12   ` Burakov, Anatoly
  1 sibling, 0 replies; 8+ messages in thread
From: Burakov, Anatoly @ 2025-10-13 12:36 UTC (permalink / raw)
  To: Jake Freeland, Bruce Richardson; +Cc: dev

On 8/14/2025 11:32 PM, Jake Freeland wrote:
> When need_hole is false, memseg searches will only be done for a single
> element. If the search starts at beginning of the list, an element that
> was previously reserved as a hole may be wrongly claimed.
> 
> To avoid this, begin the search following the last used entry. This way,
> we ignore all pre-existing holes.
> 
> Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
> ---
>   lib/eal/freebsd/eal_memory.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
> index be3bde2cb9..b159e9ef4e 100644
> --- a/lib/eal/freebsd/eal_memory.c
> +++ b/lib/eal/freebsd/eal_memory.c
> @@ -143,6 +143,7 @@ rte_eal_hugepage_init(void)
>   
>   			for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS;
>   					msl_idx++) {
> +				int start_idx, num_elems;
>   				bool empty, need_hole;
>   				msl = &mcfg->memsegs[msl_idx];
>   				arr = &msl->memseg_arr;
> @@ -157,10 +158,24 @@ rte_eal_hugepage_init(void)
>   				 * adjacent to current one.
>   				 */
>   				need_hole = !empty && !is_adjacent;
> +				if (need_hole) {
> +					start_idx = 0;
> +					/* we need 1, plus hole */
> +					num_elems = 2;
> +				} else {
> +					/* begin our search after the last used
> +					 * element in the list, skipping over
> +					 * any previously placed holes
> +					 */
> +					start_idx = rte_fbarray_find_prev_n_used(
> +							arr, arr->len - 1, 1) + 1;
> +					if (start_idx < 0)
> +						start_idx = 0;
> +					num_elems = 1;
> +				}
>   
> -				/* we need 1, plus hole if not adjacent */
>   				ms_idx = rte_fbarray_find_next_n_free(arr,
> -						0, 1 + (need_hole ? 1 : 0));
> +						start_idx, num_elems);
>   
>   				/* memseg list is full? */
>   				if (ms_idx < 0)

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

* Re: [PATCH v2 3/3] eal/linux: Check hugepage access permissions
  2025-08-14 21:32 ` [PATCH v2 3/3] eal/linux: Check hugepage access permissions Jake Freeland
@ 2025-10-13 12:43   ` Burakov, Anatoly
  0 siblings, 0 replies; 8+ messages in thread
From: Burakov, Anatoly @ 2025-10-13 12:43 UTC (permalink / raw)
  To: Jake Freeland, Bruce Richardson; +Cc: dev

On 8/14/2025 11:32 PM, Jake Freeland wrote:
> Currently, hugepage mountpoints will be used irrespective of permissions,
> leading to potential EACCES errors during memory allocation. Fix this by
> not using a mountpoint if we do not have read/write permissions on it.
> 
> Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
> ---
>   lib/eal/linux/eal_hugepage_info.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/lib/eal/linux/eal_hugepage_info.c b/lib/eal/linux/eal_hugepage_info.c
> index d47a19c56a..e2ddd6218b 100644
> --- a/lib/eal/linux/eal_hugepage_info.c
> +++ b/lib/eal/linux/eal_hugepage_info.c
> @@ -260,6 +260,13 @@ get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int len)
>   				continue;
>   		}
>   
> +		if (access(splitstr[MOUNTPT], R_OK | W_OK) < 0) {
> +			EAL_LOG(NOTICE,
> +				"Skipping hugepage dir '%s': missing r/w perms",

Perhaps should be reworded as:

Skipping hugepage directory '%s': missing R/W permissions"

Otherwise,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

> +				splitstr[MOUNTPT]);
> +			continue;
> +		}
> +
>   		/*
>   		 * If no --huge-dir option has been given, we're done.
>   		 */


-- 
Thanks,
Anatoly

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

* Re: [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes
  2025-08-14 21:32 ` [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes Jake Freeland
  2025-10-13 12:36   ` Burakov, Anatoly
@ 2025-10-13 13:12   ` Burakov, Anatoly
  1 sibling, 0 replies; 8+ messages in thread
From: Burakov, Anatoly @ 2025-10-13 13:12 UTC (permalink / raw)
  To: Jake Freeland, Bruce Richardson; +Cc: dev

On 8/14/2025 11:32 PM, Jake Freeland wrote:
> When need_hole is false, memseg searches will only be done for a single
> element. If the search starts at beginning of the list, an element that
> was previously reserved as a hole may be wrongly claimed.
> 
> To avoid this, begin the search following the last used entry. This way,
> we ignore all pre-existing holes.
> 
> Signed-off-by: Jake Freeland <jfree@FreeBSD.org>
> ---
>   lib/eal/freebsd/eal_memory.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
> index be3bde2cb9..b159e9ef4e 100644
> --- a/lib/eal/freebsd/eal_memory.c
> +++ b/lib/eal/freebsd/eal_memory.c
> @@ -143,6 +143,7 @@ rte_eal_hugepage_init(void)
>   
>   			for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS;
>   					msl_idx++) {
> +				int start_idx, num_elems;
>   				bool empty, need_hole;
>   				msl = &mcfg->memsegs[msl_idx];
>   				arr = &msl->memseg_arr;
> @@ -157,10 +158,24 @@ rte_eal_hugepage_init(void)
>   				 * adjacent to current one.
>   				 */
>   				need_hole = !empty && !is_adjacent;
> +				if (need_hole) {
> +					start_idx = 0;
> +					/* we need 1, plus hole */
> +					num_elems = 2;
> +				} else {
> +					/* begin our search after the last used
> +					 * element in the list, skipping over
> +					 * any previously placed holes
> +					 */
> +					start_idx = rte_fbarray_find_prev_n_used(
> +							arr, arr->len - 1, 1) + 1;
> +					if (start_idx < 0)
> +						start_idx = 0;

Actually, I think this check is largely redundant, because even if 
find_prev_n_used returns -1 (empty array case), we immediately +1 the 
result, so it's always 0 at a minimum.

> +					num_elems = 1;
> +				}
>   
> -				/* we need 1, plus hole if not adjacent */
>   				ms_idx = rte_fbarray_find_next_n_free(arr,
> -						0, 1 + (need_hole ? 1 : 0));
> +						start_idx, num_elems);
>   
>   				/* memseg list is full? */
>   				if (ms_idx < 0)


-- 
Thanks,
Anatoly

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

end of thread, other threads:[~2025-10-13 13:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14 21:32 [PATCH v2 0/3] EAL memory fixes Jake Freeland
2025-08-14 21:32 ` [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection Jake Freeland
2025-10-13 12:23   ` Burakov, Anatoly
2025-08-14 21:32 ` [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes Jake Freeland
2025-10-13 12:36   ` Burakov, Anatoly
2025-10-13 13:12   ` Burakov, Anatoly
2025-08-14 21:32 ` [PATCH v2 3/3] eal/linux: Check hugepage access permissions Jake Freeland
2025-10-13 12:43   ` Burakov, Anatoly

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).