[PATCH v2 0/3] EAL memory fixes

[PATCH v2 0/3] EAL memory fixes

[Jake Freeland]

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

[PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection

[Jake Freeland]

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

[PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes

[Jake Freeland]

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

[PATCH v2 3/3] eal/linux: Check hugepage access permissions

[Jake Freeland]

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

Re: [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection

[Burakov, Anatoly]

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

Re: [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes

[Burakov, Anatoly]

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

Re: [PATCH v2 3/3] eal/linux: Check hugepage access permissions

[Burakov, Anatoly]

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

Re: [PATCH v2 2/3] eal/freebsd: Avoid claiming memseg holes

[Burakov, Anatoly]

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

Re: [PATCH v2 1/3] eal/freebsd: Do not use prev_ms_idx for hole detection

[Burakov, Anatoly]

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

Re: [PATCH v2 0/3] EAL memory fixes

[Thomas Monjalon]

Hello,

14/08/2025 23:32, Jake Freeland:
> 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.

Your patches have been reviewed, please could you reply to the comments?

Thank you

Re: [PATCH v2 0/3] EAL memory fixes

[Stephen Hemminger]

On Thu, 14 Aug 2025 16:32:42 -0500
Jake Freeland <jfree@FreeBSD.org> wrote:

> 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(-)
> 

Makes sense. I noticed that AI spotted same dead code as Anatoly.
You might want to fix that before Coverity complains.



## DPDK Patch Review: FreeBSD/Linux Memory Setup Fixes (v2)

**Series:** [PATCH v2 1-3/3] Memory segment and hugepage fixes  
**Author:** Jake Freeland <jfree@FreeBSD.org>  
**Acked-by:** Anatoly Burakov <anatoly.burakov@intel.com>

---

### Patch 1/3: eal/freebsd: Do not use prev_ms_idx for hole detection

**Subject Analysis:**
| Check | Status | Notes |
|-------|--------|-------|
| Length ≤60 chars | ✅ Pass | 55 characters |
| Format | ✅ Pass | `component: description` |
| Case after colon | ⚠️ Warning | "Do" should be "do" — lowercase required |
| No trailing period | ✅ Pass | |
| Signed-off-by | ✅ Pass | Present with real name |

**Commit Message Body:**
- ✅ Lines within 75 characters
- ✅ Does not start with "It"
- ✅ Clear explanation of the bug and fix

**Code Review:**
```c
-			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;
```

- ✅ Correctly uses `rte_fbarray_is_used()` API instead of fragile index tracking
- ✅ Fixes cross-memseg-list logic error
- ✅ Clean removal of now-unnecessary variable

**Verdict:** ✅ Acceptable — minor subject case warning

---

### Patch 2/3: eal/freebsd: Avoid claiming memseg holes

**Subject Analysis:**
| Check | Status | Notes |
|-------|--------|-------|
| Length ≤60 chars | ✅ Pass | 42 characters |
| Format | ✅ Pass | `component: description` |
| Case after colon | ⚠️ Warning | "Avoid" should be "avoid" |
| No trailing period | ✅ Pass | |
| Signed-off-by | ✅ Pass | |

**Commit Message Body:**
- ✅ Lines within 75 characters
- ✅ Does not start with "It"
- ✅ Clear problem/solution description

**Code Review:**

```c
start_idx = rte_fbarray_find_prev_n_used(
        arr, arr->len - 1, 1) + 1;
if (start_idx < 0)
    start_idx = 0;
```

⚠️ **Warning — Dead Code:** The condition `if (start_idx < 0)` can never be true. When `rte_fbarray_find_prev_n_used()` returns -1 (not found), adding 1 yields 0, which is not < 0. The check is ineffective. Consider either:
- Removing the dead check (since starting at 0 when nothing found is correct behavior), or
- Checking the return value before adding 1

```c
/* Multi-line comment style is correct */
/* begin our search after the last used
 * element in the list, skipping over
 * any previously placed holes
 */
```
- ✅ Comment formatting follows DPDK style

**Verdict:** ⚠️ Minor issues — dead code check, subject case

---

### Patch 3/3: eal/linux: Check hugepage access permissions

**Subject Analysis:**
| Check | Status | Notes |
|-------|--------|-------|
| Length ≤60 chars | ✅ Pass | 43 characters |
| Format | ✅ Pass | `component: description` |
| Case after colon | ⚠️ Warning | "Check" should be "check" |
| No trailing period | ✅ Pass | |
| Signed-off-by | ✅ Pass | |

**Commit Message Body:**
- ✅ Lines within 75 characters
- ✅ Does not start with "It"
- ✅ Clear description of the permission issue and fix

**Code Review:**
```c
+	if (access(splitstr[MOUNTPT], R_OK | W_OK) < 0) {
+		EAL_LOG(NOTICE,
+			"Skipping hugepage dir '%s': missing r/w perms",
+			splitstr[MOUNTPT]);
+		continue;
+	}
```

- ✅ Correct use of `access()` with R_OK | W_OK
- ✅ Appropriate NOTICE log level for skipping mountpoints
- ✅ Good defensive programming — fail gracefully rather than EACCES later
- ✅ Explicit comparison with `< 0` (not implicit boolean)

**Verdict:** ✅ Acceptable — minor subject case warning

---

## Summary

| Patch | Errors | Warnings | Info |
|-------|--------|----------|------|
| 1/3 | 0 | 1 (subject case) | 0 |
| 2/3 | 0 | 2 (subject case, dead code) | 0 |
| 3/3 | 0 | 1 (subject case) | 0 |

### Recommended Actions

**Should fix (Warnings):**

1. **All patches:** Fix subject line capitalization. Per DPDK guidelines, text after the colon should be lowercase except for acronyms:
   - `eal/freebsd: do not use prev_ms_idx for hole detection`
   - `eal/freebsd: avoid claiming memseg holes`
   - `eal/linux: check hugepage access permissions`

2. **Patch 2/3:** Remove or fix the dead `if (start_idx < 0)` check — it serves no purpose as written.

**Consider (Info):**

3. These appear to be bug fixes. If they fix regressions introduced by specific commits, consider adding `Fixes:` tags with the 12-character SHA and original subject.

4. If these are candidates for stable backport, consider adding `Cc: stable@dpdk.org`.

---

**Overall Assessment:** The series is technically sound and addresses real bugs in memory segment management. The fixes are well-designed and the commit messages explain the issues clearly. With the minor subject line case fixes and the dead code cleanup in patch 2, this series should be ready for merge.

[PATCH v3 0/3] EAL memory fixes

[Jake Freeland]

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.

v3:
* Check if ms_idx is zero to accommodate static analysis tools.
* Remove redundant start_idx check.
* Reword hugepage skip message.

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      | 26 +++++++++++++++++++++-----
 lib/eal/linux/eal_hugepage_info.c |  7 +++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

-- 
2.51.0

[PATCH v3 1/3] eal/freebsd: do not use prev_ms_idx for hole detection

[Jake Freeland]

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 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index 6d3d46a390..8c315d97d5 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,13 @@ rte_eal_hugepage_init(void)
 				if (ms_idx < 0)
 					continue;
 
-				if (need_hole && prev_ms_idx == ms_idx - 1)
+				/* ms_idx should never be 0 if we need a hole,
+				 * but include a check for static analysis.
+				 */
+				if (need_hole &&
+				    ms_idx > 0 &&
+				    rte_fbarray_is_used(arr, ms_idx - 1))
 					ms_idx++;
-				prev_ms_idx = ms_idx;
 
 				break;
 			}
-- 
2.51.0

[PATCH v3 2/3] eal/freebsd: avoid claiming memseg holes

[Jake Freeland]

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 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index 8c315d97d5..5790f095c9 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,22 @@ 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;
+					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.51.0

[PATCH v3 3/3] eal/linux: check hugepage access permissions

[Jake Freeland]

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..350c7d8cf6 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 directory '%s': missing R/W permissions"
+				splitstr[MOUNTPT]);
+			continue;
+		}
+
 		/*
 		 * If no --huge-dir option has been given, we're done.
 		 */
-- 
2.51.0

[PATCH v4] eal/linux: check hugepage access permissions

[Jake Freeland]

Added comma missing in the v3 patch.

Thanks,
Jake

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..219a95dd9f 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 directory '%s': missing R/W permissions",
+				splitstr[MOUNTPT]);
+			continue;
+		}
+
 		/*
 		 * If no --huge-dir option has been given, we're done.
 		 */
-- 
2.51.0

Re: [PATCH v2 0/3] EAL memory fixes

[Stephen Hemminger]

On Thu, 14 Aug 2025 16:32:42 -0500
Jake Freeland <jfree@FreeBSD.org> wrote:

> 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
> 

AI review of this series is mostly positive

=== Patch Review: bundle-1679-eal-freebsd.mbox (via Claude) ===
# DPDK Patch Review: eal/freebsd Memory Management Fixes

## Summary
This patch series contains 3 commits addressing memory management issues in FreeBSD EAL and adding permission checks for Linux hugepage directories.

---

## Patch 1/3: eal/freebsd: Do not use prev_ms_idx for hole detection

### Errors
None found.

### Warnings

1. **Subject line capitalization issue**
   - Subject: `eal/freebsd: Do not use prev_ms_idx for hole detection`
   - "Do" should be lowercase after the colon per guidelines
   - Should be: `eal/freebsd: do not use prev_ms_idx for hole detection`

### Info
- Commit message is clear and explains the problem well
- `Signed-off-by` and `Acked-by` tags present and in correct order
- Code change is minimal and focused
- Line lengths are within limits

---

## Patch 2/3: eal/freebsd: Avoid claiming memseg holes

### Errors
None found.

### Warnings

1. **Subject line capitalization issue**
   - Subject: `eal/freebsd: Avoid claiming memseg holes`
   - "Avoid" should be lowercase after the colon
   - Should be: `eal/freebsd: avoid claiming memseg holes`

2. **Variable declaration style inconsistency**
   ```c
   for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS;
   				msl_idx++) {
   	int start_idx, num_elems;
   	bool empty, need_hole;
   ```
   - The new variables `start_idx` and `num_elems` are declared at the start of the block, which is acceptable. However, they could be declared closer to their point of use or initialized when meaningful values exist.

3. **Potential logic issue with start_idx check**
   ```c
   start_idx = rte_fbarray_find_prev_n_used(
   		arr, arr->len - 1, 1) + 1;
   if (start_idx < 0)
   	start_idx = 0;
   ```
   - Since `rte_fbarray_find_prev_n_used()` returns -1 on failure, and then +1 is added, `start_idx` would be 0 when the function fails, making the `if (start_idx < 0)` check unreachable. This appears to be dead code. Consider:
   ```c
   start_idx = rte_fbarray_find_prev_n_used(arr, arr->len - 1, 1);
   if (start_idx < 0)
   	start_idx = 0;
   else
   	start_idx++;
   ```

### Info
- Commit message explains the fix adequately
- Tags are present and in correct order

---

## Patch 3/3: eal/linux: Check hugepage access permissions

### Errors
None found.

### Warnings

1. **Subject line capitalization issue**
   - Subject: `eal/linux: Check hugepage access permissions`
   - "Check" should be lowercase after the colon
   - Should be: `eal/linux: check hugepage access permissions`

2. **Use of lowercase log message**
   - The log message uses lowercase which is inconsistent with some other EAL log messages, though this is a minor style preference.

### Info

1. **Consider using `faccessat()` for race-free checks**
   - The `access()` function has TOCTOU (time-of-check-time-of-use) race conditions. While this is likely acceptable for this use case (startup configuration), it's worth noting.

2. **Good defensive programming**
   - Adding permission checks before attempting to use mountpoints improves error handling and user feedback.

3. **Commit message is clear and concise**

---

## General Observations

### All Patches
- All patches have proper `Signed-off-by` tags with real names and valid email addresses
- All patches have `Acked-by` from the maintainer
- Tag order is correct (Signed-off-by before Acked-by)
- No trailing whitespace detected
- Line lengths appear to be within 100 character limit for code
- Commit body lines are within 75 character limit
- Files have appropriate licenses (existing files being modified)

### Missing Items (Info level)
- No `Fixes:` tag on patches 1 and 2, which appear to be bug fixes. If these fix regressions from specific commits, a `Fixes:` tag should be added.
- No `Cc: stable@dpdk.org` tag if these are candidates for backporting to stable releases.

---

## Summary of Required Changes

| Severity | Count | Items |
|----------|-------|-------|
| Error | 0 | - |
| Warning | 4 | 3x subject line capitalization, 1x potential dead code |
| Info | 4 | Missing Fixes tags, TOCTOU note, style preferences |

### Recommended Actions

1. **Fix subject lines** (all 3 patches): Change to lowercase after colon
   - `eal/freebsd: do not use prev_ms_idx for hole detection`
   - `eal/freebsd: avoid claiming memseg holes`
   - `eal/linux: check hugepage access permissions`

2. **Review the start_idx logic** in patch 2 to ensure the `< 0` check is reachable

3. **Consider adding `Fixes:` tags** if these address specific regressions

Re: [PATCH v2 0/3] EAL memory fixes

[Jake Freeland]

On Thu Jan 22, 2026 at 11:09 PM CST, Stephen Hemminger wrote:
> On Thu, 14 Aug 2025 16:32:42 -0500
> Jake Freeland <jfree@FreeBSD.org> wrote:
>
>> 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
>> 
>
> AI review of this series is mostly positive
>
> === Patch Review: bundle-1679-eal-freebsd.mbox (via Claude) ===
> # DPDK Patch Review: eal/freebsd Memory Management Fixes
>
> ## Summary
> This patch series contains 3 commits addressing memory management issues in FreeBSD EAL and adding permission checks for Linux hugepage directories.
>
> ---
>
> ## Patch 1/3: eal/freebsd: Do not use prev_ms_idx for hole detection
>
> ### Errors
> None found.
>
> ### Warnings
>
> 1. **Subject line capitalization issue**
>    - Subject: `eal/freebsd: Do not use prev_ms_idx for hole detection`
>    - "Do" should be lowercase after the colon per guidelines
>    - Should be: `eal/freebsd: do not use prev_ms_idx for hole detection`
>
> ### Info
> - Commit message is clear and explains the problem well
> - `Signed-off-by` and `Acked-by` tags present and in correct order
> - Code change is minimal and focused
> - Line lengths are within limits
>
> ---
>
> ## Patch 2/3: eal/freebsd: Avoid claiming memseg holes
>
> ### Errors
> None found.
>
> ### Warnings
>
> 1. **Subject line capitalization issue**
>    - Subject: `eal/freebsd: Avoid claiming memseg holes`
>    - "Avoid" should be lowercase after the colon
>    - Should be: `eal/freebsd: avoid claiming memseg holes`
>
> 2. **Variable declaration style inconsistency**
>    ```c
>    for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS;
>    				msl_idx++) {
>    	int start_idx, num_elems;
>    	bool empty, need_hole;
>    ```
>    - The new variables `start_idx` and `num_elems` are declared at the start of the block, which is acceptable. However, they could be declared closer to their point of use or initialized when meaningful values exist.
>
> 3. **Potential logic issue with start_idx check**
>    ```c
>    start_idx = rte_fbarray_find_prev_n_used(
>    		arr, arr->len - 1, 1) + 1;
>    if (start_idx < 0)
>    	start_idx = 0;
>    ```
>    - Since `rte_fbarray_find_prev_n_used()` returns -1 on failure, and then +1 is added, `start_idx` would be 0 when the function fails, making the `if (start_idx < 0)` check unreachable. This appears to be dead code. Consider:
>    ```c
>    start_idx = rte_fbarray_find_prev_n_used(arr, arr->len - 1, 1);
>    if (start_idx < 0)
>    	start_idx = 0;
>    else
>    	start_idx++;
>    ```
>
> ### Info
> - Commit message explains the fix adequately
> - Tags are present and in correct order
>
> ---
>
> ## Patch 3/3: eal/linux: Check hugepage access permissions
>
> ### Errors
> None found.
>
> ### Warnings
>
> 1. **Subject line capitalization issue**
>    - Subject: `eal/linux: Check hugepage access permissions`
>    - "Check" should be lowercase after the colon
>    - Should be: `eal/linux: check hugepage access permissions`
>
> 2. **Use of lowercase log message**
>    - The log message uses lowercase which is inconsistent with some other EAL log messages, though this is a minor style preference.
>
> ### Info
>
> 1. **Consider using `faccessat()` for race-free checks**
>    - The `access()` function has TOCTOU (time-of-check-time-of-use) race conditions. While this is likely acceptable for this use case (startup configuration), it's worth noting.
>
> 2. **Good defensive programming**
>    - Adding permission checks before attempting to use mountpoints improves error handling and user feedback.
>
> 3. **Commit message is clear and concise**
>
> ---
>
> ## General Observations
>
> ### All Patches
> - All patches have proper `Signed-off-by` tags with real names and valid email addresses
> - All patches have `Acked-by` from the maintainer
> - Tag order is correct (Signed-off-by before Acked-by)
> - No trailing whitespace detected
> - Line lengths appear to be within 100 character limit for code
> - Commit body lines are within 75 character limit
> - Files have appropriate licenses (existing files being modified)
>
> ### Missing Items (Info level)
> - No `Fixes:` tag on patches 1 and 2, which appear to be bug fixes. If these fix regressions from specific commits, a `Fixes:` tag should be added.
> - No `Cc: stable@dpdk.org` tag if these are candidates for backporting to stable releases.
>
> ---
>
> ## Summary of Required Changes
>
> | Severity | Count | Items |
> |----------|-------|-------|
> | Error | 0 | - |
> | Warning | 4 | 3x subject line capitalization, 1x potential dead code |
> | Info | 4 | Missing Fixes tags, TOCTOU note, style preferences |
>
> ### Recommended Actions
>
> 1. **Fix subject lines** (all 3 patches): Change to lowercase after colon
>    - `eal/freebsd: do not use prev_ms_idx for hole detection`
>    - `eal/freebsd: avoid claiming memseg holes`
>    - `eal/linux: check hugepage access permissions`
>
> 2. **Review the start_idx logic** in patch 2 to ensure the `< 0` check is reachable
>
> 3. **Consider adding `Fixes:` tags** if these address specific regressions

Hi Stephen,

This AI analysis seems to have been run on the previous revision on the
patchset.

Jake Freeland

Re: [PATCH v3 3/3] eal/linux: check hugepage access permissions

[David Marchand]

Hello,

On Wed, 14 Jan 2026 at 10:33, Jake Freeland <jfree@freebsd.org> 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>

There was some cleanup in this area recently.
Please check whether the change is still necessary and rebase.


-- 
David Marchand

Re: [PATCH v3 0/3] EAL memory fixes

[David Marchand]

On Wed, 14 Jan 2026 at 10:33, Jake Freeland <jfree@freebsd.org> wrote:
>
> 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.
>
> v3:
> * Check if ms_idx is zero to accommodate static analysis tools.
> * Remove redundant start_idx check.
> * Reword hugepage skip message.
>
> 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      | 26 +++++++++++++++++++++-----
>  lib/eal/linux/eal_hugepage_info.c |  7 +++++++
>  2 files changed, 28 insertions(+), 5 deletions(-)

Applied the first two fixes for FreeBSD.
Thanks.


-- 
David Marchand