|WARNING| eal/linux: fix dirty flag for reused hugepages in legacy mem

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172650

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-06

# DPDK Patch Review

## Analysis

This patch fixes a bug where hugepage files reused with `--huge-unlink=never` in legacy memory mode are not marked as dirty, causing `rte_zmalloc_socket()` to skip the memset and return stale data. The fix sets `RTE_MEMSEG_FLAG_DIRTY` on remapped segments when existing hugepage files are reused.

### Correctness Review

**No correctness bugs identified.** The patch correctly addresses a real bug:
- Without this fix, `rte_zmalloc_socket()` returns unzeroed memory when legacy mem reuses hugepages
- The fix matches the behavior of the dynamic memory path (`alloc_seg()`)
- The `dirty` flag is set only when files are reused (when `unlink_existing` is false)
- The flag is correctly written to `ms->flags` alongside other memseg initialization

### Code Style

**ERROR: Implicit boolean comparison**

Line 697 in `eal_memory.c`:
```c
const bool dirty = !internal_conf->hugepage_file.unlink_existing;
```

DPDK style requires explicit comparison for boolean struct members. This should be:
```c
const bool dirty = internal_conf->hugepage_file.unlink_existing == false;
```

Or equivalently:
```c
const bool dirty = (internal_conf->hugepage_file.unlink_existing == false);
```

Note: The assignment itself (storing to a `bool` variable) is correct. The issue is the implicit boolean test of the struct member on the right-hand side. Per AGENTS.md: "write `if (a != 0)` not `if (a)`, and `if (p == NULL)` not `if (!p)`. Direct truthiness is acceptable only on actual `bool` types." This applies to any expression that tests truthiness, including negation with `!`.

### Documentation

**WARNING: Documentation incomplete**

The comment added to `eal_internal_cfg.h` explains why reused pages are dirty, which is good. However, it should also explain the *consequence* for clarity:

```c
/**
 * Unlink existing files at startup and re-create them before mapping.
 * If false, the files are kept and mapped again. The kernel zeroes a
 * hugepage only the first time it is mapped from a given file, so
 * reused pages still hold the previous content and must be treated
 * as dirty (callers of rte_zmalloc must zero them explicitly).
 */
```

Adding "(callers of rte_zmalloc must zero them explicitly)" or similar makes the contract clearer.

### Process Compliance

-  Fixes tag present with correct commit reference
-  Cc: stable at dpdk.org present (appropriate for a bug fix)
-  Suggested-by tag present
-  Changes are minimal and focused on the bug
-  **Release notes missing**: This is a user-visible bug fix for legacy memory mode. Should add a line to `doc/guides/rel_notes/release_26_11.rst` under "Fixed Issues" section describing the fix.

---

## ERRORS

1. **Implicit boolean comparison** (line 697):
   ```c
   /* Current */
   const bool dirty = !internal_conf->hugepage_file.unlink_existing;
   
   /* Required */
   const bool dirty = internal_conf->hugepage_file.unlink_existing == false;
   ```

## WARNINGS

1. **Missing release notes**: Bug fix affecting legacy memory users with `--huge-unlink=never` should be documented in release notes.

2. **Documentation could be clearer**: The comment in `eal_internal_cfg.h` should mention the consequence (explicit zeroing required) in addition to the mechanism.

Review-Result: ERROR