* [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; 9+ 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] 9+ 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-10-14 9:06 ` 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, 2 replies; 9+ 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] 9+ 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 2025-10-14 9:06 ` Burakov, Anatoly 1 sibling, 0 replies; 9+ 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] 9+ 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 @ 2025-10-14 9:06 ` Burakov, Anatoly 1 sibling, 0 replies; 9+ messages in thread From: Burakov, Anatoly @ 2025-10-14 9:06 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> > --- > 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; > This is not a bug as the logic won't allow for this to happen, but some static analysis tools might flag this: Earlier we check for ms_idx < 0, so assuming ms_idx == 0, we will pass (ms_idx - 1) to rte_fbarray_is_used. This won't actually happen because ms_idx will never be 0 if `need_hole` is true, but *technically* it is not impossible, and so should probably be addressed somehow to avoid false positives from static analysis. -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2025-10-14 9:06 UTC | newest] Thread overview: 9+ 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-10-14 9:06 ` 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).