|WARNING| kernel/freebsd: add NUMA domain selection for contigmem

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-24

# Review of DPDK Patch: kernel/freebsd NUMA domain selection for contigmem

## ERRORS

None identified.

## WARNINGS

**Warning 1: Missing release notes context**

The release notes entry in `doc/guides/rel_notes/release_26_11.rst` appears before the "Added division round up macro" entry, suggesting this may be targeting an older release or the ordering is incorrect. Release notes should only update the current release file. Verify this is the correct release notes file for the target release.

**Warning 2: Documentation completeness**

File: `doc/guides/freebsd_gsg/build_dpdk.rst`, added section starting at line ~110.

The documentation states "applications should keep requesting memory from socket 0 or `SOCKET_ID_ANY`" but does not explain what happens if an application explicitly requests memory from a different socket ID. Consider clarifying whether socket-specific allocation requests are ignored, remapped, or produce an error when this tunable is set.

**Warning 3: Loader configuration example**

File: `doc/guides/freebsd_gsg/build_dpdk.rst`, line 134.

The documentation mentions that kernel environment variables "can also be specified during boot by placing the following in `/boot/loader.conf`" but does not provide an example showing the `hw.contigmem.numa_domain` setting in that context. For consistency with the existing documentation pattern, consider adding an example:

```
hw.contigmem.numa_domain="1"
```

## INFO

**Info 1: Consider bounds check optimization**

File: `kernel/freebsd/contigmem/contigmem.c`, lines 161-167.

The bounds check `contigmem_numa_domain >= vm_ndomains` is correct. However, when `vm_ndomains` is 1 (single-socket system), domain 0 is the only valid choice and the tunable provides no benefit. Consider logging an informational message when the tunable is set on a single-socket system, as users may not realize their configuration has no effect.

**Info 2: Error message consistency**

File: `kernel/freebsd/contigmem/contigmem.c`, lines 180-185.

The error messages use different prefixes: "contigmem:" vs. no prefix. The NUMA-specific path uses "contigmem:" while the existing path does not. For consistency, consider using "contigmem:" prefix for all error messages in this function, though this is a minor style issue in error reporting.

**Info 3: Pre-existing: memory leak on partial allocation failure**

File: `kernel/freebsd/contigmem/contigmem.c`, lines 169-190 (context around the new code).

This is NOT introduced by this patch, but the existing code has a resource leak on error path. If `contigmalloc()` fails on buffer i, the function jumps to the `error:` label (line 189). The error handler should free buffers 0 through i-1 that were successfully allocated before returning. This applies to both the existing code path and the new NUMA-aware path. The patch correctly preserves this behavior without making it worse.

---

This patch adds NUMA domain selection for FreeBSD contigmem allocation, correctly using `contigmalloc_domainset()` with `DOMAINSET_FIXED()` for strict placement and falling back to the original round-robin behavior when the tunable is -1. The bounds checking and error reporting are appropriate.

Review-Result: WARNING