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

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Sat Oct 24 20:26:06 CEST 2026


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

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

**Missing domain value verification after successful allocation:**

In the allocation loop, when `contigmem_numa_domain >= 0` and `contigmalloc_domainset()` succeeds, there is no verification that the buffer was actually allocated from the requested domain. While `DOMAINSET_FIXED()` should guarantee this, adding a post-allocation check with `vm_phys_domain()` would provide defense in depth and catch potential kernel bugs or configuration issues.

Suggested addition after successful allocation:

```c
if (contigmem_numa_domain >= 0) {
    int actual_domain = vm_phys_domain(pmap_kextract((vm_offset_t)addr));
    if (actual_domain != contigmem_numa_domain) {
        printf("contigmem: buffer %d allocated from wrong domain %d (expected %d)\n",
               i, actual_domain, contigmem_numa_domain);
        contigfree(addr, contigmem_buffer_size, M_CONTIGMEM);
        error = ENXIO;
        goto error;
    }
}
```

## INFO

**Pre-existing: Error message inconsistency**

The new error messages use "contigmem:" prefix (lines 163, 182-183), while the pre-existing error message at line 185 uses bare "contigmalloc failed for buffer %d". For consistency, the pre-existing message should also use the "contigmem:" prefix, but this is not introduced by this patch.

**Documentation clarity**

The documentation states "applications should keep requesting memory from socket 0 or `SOCKET_ID_ANY`" (line 129-130). This is correct but might benefit from explaining *why* -- that the EAL's socket reporting is unchanged and still reports everything as socket 0, so requesting socket 1 would fail even though the physical memory is in domain 1. The current wording is adequate but could be clearer about the reason.

**Variable initialization location**

The `contigmem_numa_domain` variable is initialized at declaration (line 63), which is appropriate for a module parameter with a meaningful default. This is good practice.

Review-Result: WARNING


More information about the test-report mailing list