|SUCCESS| eal/linux: fix dirty flag for reused hugepages in legacy mem
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Nov 6 03:06:19 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172650
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-06
# DPDK Patch Review
## Patch Summary
This patch fixes a bug in the legacy memory mode where reused hugepage files are not marked as dirty, causing `rte_zmalloc_socket()` to skip the memset and return stale data. The fix sets `RTE_MEMSEG_FLAG_DIRTY` when remapping segments that come from preserved hugepage files.
## ERRORS
None.
## WARNINGS
None.
## INFO
**1. Variable scope could be tightened**
Line (diff context):
```c
const struct internal_config *internal_conf =
eal_get_internal_configuration();
const bool dirty = !internal_conf->hugepage_file.unlink_existing;
```
The `internal_conf` pointer is now used only to read one field. Consider directly dereferencing in the `dirty` initialization to reduce the number of local variables:
```c
const bool dirty = !eal_get_internal_configuration()
->hugepage_file.unlink_existing;
```
This is a minor readability suggestion, not a defect.
---
**2. Comment placement and clarity**
The new comment in `eal_internal_cfg.h` is clear and documents why reused pages are dirty. No issues.
---
**3. Pre-existing: `#ifdef RTE_ARCH_64` removal scope**
The patch removes the `#ifndef RTE_ARCH_64` guard around `internal_conf`, which is correct since the variable is now used unconditionally. This is not introduced by the patch but is a good cleanup.
---
## Analysis
### Correctness
- **Resource leak check**: No new allocations or resource acquisitions.
- **Error path check**: No new error paths introduced. The `dirty` flag assignment on line `ms->flags = dirty ? RTE_MEMSEG_FLAG_DIRTY : 0;` is unconditional and straightforward.
- **Logic**: The fix correctly identifies that when `unlink_existing` is false, hugepage files are reused and contain stale data, matching the behavior in the dynamic memory path (`alloc_seg()`).
### Style and Conventions
- No forbidden tokens.
- No deprecated API usage.
- Boolean logic is explicit (`dirty ? ... : 0`).
- Comment adheres to DPDK style.
### Documentation
- Release notes are not required for this fix (it corrects a bug in existing functionality).
- The added comment in `eal_internal_cfg.h` explains the semantics of `unlink_existing` and why reused pages are dirty, which aids future maintainers.
### Commit Message
The commit message clearly describes the bug, impact (crash in mlx5 probe), root cause, and fix. The `Fixes:` tag correctly references the commit that introduced `--huge-unlink=never`. `Cc: stable at dpdk.org` is appropriate for a correctness fix.
---
Review-Result: CLEAN
More information about the test-report
mailing list