docs: refresh NUMA_GPU_REVIEW to match implemented state

Several items in the note had landed or were inaccurate. Update it to:
- mark DONE: G2 pin_gpu, ImageBuffer first-touch, acquisition_device
  de-NUMA, CUDAWrapper sysfs node lookup;
- add the previously-missing FPGA DMA buffer section - placement is a
  kernel concern (dma_alloc_coherent, device-local), not the userspace
  mbind, which was only the simulator;
- record that libnuma is now down to a single file (NUMAHWPolicy.cpp);
- note that dependency removal (G3) and pinning behaviour (G4) are
  separable axes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

NUMA_GPU_REVIEW.md

@@ -1,115 +1,145 @@
-# NUMA / GPU usage — current state, goals, proposed changes (for review)
+# NUMA / GPU usage — current state, goals, remaining work
 
-Working note to agree on direction **before** modifying `ImageBuffer` / `NUMAHWPolicy` and the
-GPU-dispatch logic. Nothing here is implemented yet. File:line anchors are from branch
+Working note on the NUMA/GPU direction. Several items are now **implemented and committed** on
+branch `2606-pixel-refine` (see §3/§5); this note tracks what landed, what's left, and — importantly
+— the corrected mental model of *where NUMA actually matters*. File:line anchors are from
 `2606-pixel-refine`.
 
+**Headline:** after the committed work, **libnuma is used in exactly one file**
+(`common/NUMAHWPolicy.cpp`). Everything else (GPU node lookup, the big RAM buffer, the FPGA DMA
+buffers, the simulator) is libnuma-free.
+
 ## 1. Current state (the mind map)
 
 ### 1a. `ImageBuffer` — the big RAM ring buffer
 - **One instance**, a member of the receiver: `JFJochReceiverService::image_buffer`
   (`receiver/JFJochReceiverService.h:21`), sized `image_buffer_MiB` from broker config
-  (`broker/jfjoch_broker.cpp:104` → `JFJochReceiverService` ctor
-  `receiver/JFJochReceiverService.cpp:15`, `image_buffer(send_buffer_size_MiB*1024*1024)`).
-  This is the 150–200 GB allocation.
-- **Allocated + zeroed in the ctor** (`common/ImageBuffer.cpp`): `numa_alloc_interleaved` if libnuma,
-  else `std::malloc`; then a **single-threaded `memset`** to pre-fault every page (deliberate — kills
-  first-use page-fault latency in the hot path). Happens once at broker startup.
+  (`broker/jfjoch_broker.cpp:104` → ctor `receiver/JFJochReceiverService.cpp:15`). The 150–200 GB
+  allocation.
+- **Allocation (DONE, commit `d373ba04`):** plain `std::malloc` + a **parallel first-touch `memset`**
+  (`hardware_concurrency()` threads, unpinned) in `common/ImageBuffer.cpp`. Each page is
+  first-touched — and thus NUMA-placed — by whichever node the scheduler ran the zeroing thread on:
+  approximates the old `numa_alloc_interleaved` placement for the random-access buffer, *and*
+  pre-faults every page (no first-use fault in the hot path), *and* speeds up startup, *and* drops
+  libnuma here. (Was `numa_alloc_interleaved` + single-threaded `memset`.)
 - **Producers/consumers**: receiver/decompression threads write frames into slots; consumers are
   preview/TIFF/JPEG/HTTP retrieval (`GetImage`) and the ZMQ/file sender. Access is random and
-  unpinned (any thread → any slot).
+  unpinned (any thread → any slot) — which is *why* interleave/first-touch placement (not per-node
+  binding) is the right model.
 - **Not used by `jfjoch_process` or `jfjoch_viewer`** — they read HDF5 through the reader, never
-  instantiate `ImageBuffer`. So this buffer is **broker/receiver-only** (it only needs to *compile*
-  for the viewer).
+  instantiate `ImageBuffer`. Broker/receiver-only (it only needs to *compile* for the viewer).
 
-### 1b. `NUMAHWPolicy` — bundles three concerns per worker thread
+### 1b. FPGA DMA buffers — placement is a *kernel* concern, not libnuma  *(this section was missing)*
+The real per-frame DMA buffers are **allocated and NUMA-placed by the kernel driver**, with zero
+userspace/libnuma involvement. The earlier draft framed the userspace `mbind` as "the real
+hardware-locality win" — that was wrong; the win is in the kernel:
+- `fpga/pcie_driver/jfjoch_memory.c:28` — `dma_alloc_coherent(&pdev->dev, FPGA_BUFFER_LOCATION_SIZE,
+  …)` × `nbuffer` (512). Gives **physically contiguous, DMA-coherent** pages (required: each buffer's
+  bus address is written into the FPGA address table at `:37-38`). Placement is **device-local by
+  construction** — the kernel DMA/page allocator uses `dev_to_node(&pdev->dev)`. This is exactly why
+  it can't be a userspace `malloc`+`mbind`: userspace virtual memory is neither physically contiguous
+  nor a valid DMA target.
+- `jfjoch_memory.c:99` — `dma_mmap_coherent` maps those *same physical pages* into userspace;
+  `JungfraujochDevice::MapKernelBuffer` (`fpga/host_library/JungfraujochDevice.cpp:166`) is a plain
+  `mmap` of the char dev. No second allocation, no first-touch, no migration.
+- `IOCTL_JFJOCH_NUMA` (`fpga/pcie_driver/jfjoch_ioctl.c:133`) just **reports**
+  `drvdata->pdev->dev.numa_node`. Userspace's only NUMA action on the real path is to pin the
+  *acquire thread's CPU* to that node (the `RunOnNode` calls in §1c).
+- **Simulated path (DONE, commit `042df308`):** `HLSSimulatedDevice` used to *emulate* device locality
+  with a userspace `mmap`+`mbind` (`<numaif.h>`) — the only libnuma user in `acquisition_device/`. It
+  now uses **plain zeroed heap buffers** (`std::vector`, not performance-critical). Buffer ownership
+  was refactored so each device owns its lifecycle: `PCIExpressDevice` `mmap`s/`munmap`s the kernel
+  DMA buffers; `HLSSimulatedDevice` owns the heap buffers; the base `AcquisitionDevice` is just a
+  non-owning address view. No libnuma, no mmap on the sim path.
+
+### 1c. `NUMAHWPolicy` — bundles three concerns per worker thread (the last libnuma user)
 Built from the broker config `numa_policy` string (e.g. `n2g2`, `n8g4`, `n8g4_hbm`) into a table of
 `NUMABinding{cpu_node, mem_node, gpu}`; `GetBinding(thread) = bindings[thread % nbindings]`
-(round-robin, `common/NUMAHWPolicy.cpp:54`). `Bind(thread)` does **all three** at once
-(`common/NUMAHWPolicy.cpp:61`):
-1. **CPU pin** — `RunOnNode` / `numa_run_on_node`
-2. **Memory bind** — `MemOnNode` / `numa_set_membind` (on `n8g4_hbm`, `mem_node = i+8` → binds to HBM nodes)
+(round-robin, `common/NUMAHWPolicy.cpp:50`). `Bind(thread)` does **all three** at once:
+1. **CPU pin** — `RunOnNode` / `numa_run_on_node`  ← libnuma
+2. **Memory bind** — `MemOnNode` / `numa_set_membind` (`n8g4_hbm` → HBM nodes)  ← libnuma
 3. **GPU select** — `SelectGPU` / `set_gpu` (= `cudaSetDevice`, **not** libnuma)
 
 Call sites:
-- `receiver/JFJochReceiverFPGA.cpp:180/217/264` — data-stream threads `RunOnNode(FPGA's NUMA node)`
-  (device-locality pin, the NIC-era idea applied to the FPGA card).
+- `receiver/JFJochReceiverFPGA.cpp:180/217/264` — acquire (data-stream) threads
+  `RunOnNode(acquisition_device.GetNUMANode())` → the kernel-reported device node (§1b). CPU pin only.
 - `receiver/JFJochReceiverFPGA.cpp:299`, `receiver/JFJochReceiverLite.cpp:234` — analysis worker
-  threads `numa_policy.Bind(threadid)` → cpu + mem + **GPU** per the policy table.
+  threads `numa_policy.Bind(threadid)` → cpu + mem + GPU per the policy table.
 - `image_analysis/indexing/IndexerThreadPool.cpp:34` — each indexer thread
-  `SelectGPUAndItsNUMA(threadid % gpu_count)` (GPU round-robin + that GPU's own NUMA node;
+  `SelectGPUAndItsNUMA(threadid % gpu_count)` (GPU round-robin + that GPU's NUMA node via sysfs;
   independent of the `numa_policy` table).
 
-libnuma is used in exactly three files: `NUMAHWPolicy.cpp` (the above), `ImageBuffer.cpp`
-(`numa_alloc_interleaved`/`numa_free`), and `CUDAWrapper.cpp` (one `numa_node` lookup).
+The GPU NUMA node is now read from sysfs (`/sys/bus/pci/devices/<bdf>/numa_node`,
+`common/CUDAWrapper.cu:75`), **not** libnuma. So the only remaining libnuma calls are `RunOnNode` and
+`MemOnNode` in this file.
 
-### 1c. GPU dispatch — and why `jfjoch_process` underuses GPUs
+### 1d. GPU dispatch — `jfjoch_process` now uses all visible GPUs (was the root cause)
 - `get_gpu_count()` = `cudaGetDeviceCount()` (`common/CUDAWrapper.*`), so it already honours
   **`CUDA_VISIBLE_DEVICES`**. All dispatch is `% gpu_count`.
 - **Broker/receiver**: worker threads spread over GPUs via `numa_policy.Bind` (the `gpu` field) +
-  the indexer pool via `threadid % gpu_count`. → uses all visible GPUs.
-- **`jfjoch_process`**: its worker lambda (`tools/jfjoch_process.cpp:849`, launched `nthreads` times)
-  constructs `MXAnalysisWithoutFPGA` but **never calls `Bind`/`SelectGPU`**, and
-  `MXAnalysisWithoutFPGA` itself does not select a device (`image_analysis/MXAnalysisWithoutFPGA.cpp:38`
-  just builds GPU engines on the *current* device). → all per-image preprocessing / spot-finding /
-  azimuthal integration run on **GPU 0**. Only the indexer pool spreads. **This is the root cause of
-  "`jfjoch_process` doesn't use all GPUs."**
+  indexer pool via `threadid % gpu_count`. → uses all visible GPUs.
+- **`jfjoch_process` (DONE, G2):** its worker (`tools/jfjoch_process.cpp:854`) now calls `pin_gpu()`
+  before building `MXAnalysisWithoutFPGA`, so each worker's CUDA streams/engines land on a distinct
+  device. Previously everything ran on GPU 0 (only the indexer pool spread).
 
 ## 2. Goals
 
 - **G1 — multiple brokers, disjoint GPUs.** Run >1 `jfjoch_broker` on one machine, each confined to a
-  subset of GPUs, with the code transparently using "all it can see" (no hard-coded indices). Pure
-  workload control, no security requirement.
-- **G2 — `jfjoch_process` should use all visible GPUs**, not just GPU 0.
-- **G3 — drop the libnuma dependency** if it doesn't cost real performance (annoying dep; also a
-  blocker for the long-term Windows/MSVC viewer).
-- **G4 — reassess whether NUMA CPU/mem pinning is still worth it** given the FPGA pipeline (DMA into
-  kernel-mmap'd buffers, negligible IRQ traffic) rather than the old network-RX model.
+  subset of GPUs, code transparently using "all it can see" (no hard-coded indices). Workload
+  control, no security requirement.
+- **G2 — `jfjoch_process` should use all visible GPUs**, not just GPU 0.  **DONE** (§1d).
+- **G3 — drop the libnuma dependency.** Annoying dep; also a blocker for the long-term Windows/MSVC
+  viewer. Mostly done — one file (`NUMAHWPolicy.cpp`) left.
+- **G4 — reassess whether NUMA CPU/mem pinning is still worth it.** Note the corrected model: FPGA DMA
+  buffer *placement* is the kernel's job (§1b), so the only behavioural NUMA op left on real data flow
+  is the **CPU pin** of the acquire/analysis threads to the kernel-reported device node. That's the
+  thing to A/B.
 
-## 3. Proposed changes
+**Key separability insight:** dependency removal (G3) and behaviour removal (G4) are *independent
+axes*. Dropping libnuma does **not** require dropping any NUMA behaviour — `numa_run_on_node` →
+`sched_setaffinity`, `mbind`/`set_mempolicy` are raw syscalls. So G3 can complete regardless of how
+the G4 measurement turns out.
 
-- **G1 (zero code):** launch each broker under `CUDA_DEVICE_ORDER=PCI_BUS_ID
-  CUDA_VISIBLE_DEVICES=<subset> jfjoch_broker …`. `get_gpu_count()`/`% gpu_count` already do the rest.
-  Action item: **document this** (deployment note) and set `CUDA_DEVICE_ORDER=PCI_BUS_ID` so indices
-  are stable across boots.
+## 3. Status of changes
 
-- **G2 (DONE):** added `pin_gpu()` to `CUDAWrapper` — a process-wide round-robin counter
-  (`counter++ % get_gpu_count()`, no thread id needed, no-op when no GPU). The `jfjoch_process` worker
-  calls it once before building `MXAnalysisWithoutFPGA`, so each worker's CUDA streams/engines land on
-  a distinct device. Caller-agnostic and reusable by other thread pools later.
+- **G1 (zero code) — TODO (docs).** Launch each broker under `CUDA_DEVICE_ORDER=PCI_BUS_ID
+  CUDA_VISIBLE_DEVICES=<subset> jfjoch_broker …`. `get_gpu_count()`/`% gpu_count` do the rest. Set
+  `CUDA_DEVICE_ORDER=PCI_BUS_ID` so indices are stable across boots. Action: write the deployment note.
+- **G2 — DONE.** `pin_gpu()` in `CUDAWrapper` (process-wide round-robin `counter++ % get_gpu_count()`,
+  no-op when no GPU); `jfjoch_process` worker calls it once. Caller-agnostic, reusable.
+- **`ImageBuffer` — DONE (`d373ba04`).** malloc + parallel first-touch (§1a).
+- **`acquisition_device` — DONE (`042df308`).** Simulator off NUMA/mmap → plain heap; per-device
+  buffer ownership; libnuma gone from `acquisition_device/` (§1b).
+- **`CUDAWrapper` `numa_node` — DONE.** sysfs lookup, not libnuma (§1c).
+- **`NUMAHWPolicy` — REMAINING.** Split the bundled concerns:
+  - **Memory bind** (`MemOnNode`) — **drop.** HBM out of scope (the one Xeon MAX box didn't pay off;
+    treat every host as plain multi-socket).
+  - **CPU pin** (`RunOnNode`) — gated on **G4**. If kept, reimplement with `sched_setaffinity` +
+    node→cpulist from `/sys/devices/system/node/nodeN/cpulist` (no libnuma). Note it pins to the
+    *kernel-reported device node* (§1b) — well-founded if any pinning is kept.
+  - **GPU select** (`SelectGPU`) — **keep** (`cudaSetDevice`). `SelectGPUAndItsNUMA` then collapses to
+    `set_gpu` (+ optional CPU pin); `Bind` collapses to `SelectGPU` (+ optional CPU pin).
+  - Result: `NUMA_LIBRARY` leaves the CMake (`CMakeLists.txt:78-80`,
+    `common/CMakeLists.txt:155-161`).
 
-- **G3 / `ImageBuffer`:** replace `numa_alloc_interleaved` + single-threaded `memset` with **plain
-  `malloc` + a parallel first-touch `memset`** (N threads, unpinned). Threads spread by the scheduler
-  → balanced placement ≈ interleave for random access, *and* faster startup, *and* no libnuma. Safe
-  here because the buffer is 30–40 % of RAM (first-touch spills to the other node if one fills; no OOM;
-  just check `vm.zone_reclaim_mode == 0`). Deterministic per-page interleave (if ever needed under
-  tight RAM) is a raw `mbind(MPOL_INTERLEAVE)` syscall — still libnuma-free.
+## 4. Open questions / to validate before deleting `NUMAHWPolicy` pinning
 
-- **G3 / G4 / `NUMAHWPolicy`:** split the bundled concerns:
-  - **CPU pin** (`RunOnNode`) — likely **drop** for the FPGA path (G4). If ever wanted back, use
-    `sched_setaffinity` (no libnuma).
-  - **GPU select** (`SelectGPU`/`SelectGPUAndItsNUMA`) — **keep**; already `cudaSetDevice`, no libnuma.
-  - **Memory bind** (`MemOnNode`) — **drop.** (HBM is out of scope: the only Xeon MAX box didn't pay
-    off and isn't worth special-casing, so no `mbind` path is needed — treat every host as plain
-    multi-socket.)
-  - **`CUDAWrapper` `numa_node`** — read the GPU PCIe device's `/sys/.../numa_node` instead of libnuma.
-  - Result: `NUMA_LIBRARY` leaves the CMake entirely.
-
-## 4. Open questions / to validate before deleting anything
-
-- **G4 is empirical.** A/B at production frame rate (pinning on vs off): sustained throughput, dropped
-  frames, latency jitter. The reasoning predicts "no regression," but measure on one real box first —
+- **G4 is empirical.** A/B at production frame rate (CPU pin on vs off): sustained throughput, dropped
+  frames, latency jitter. Reasoning predicts "no regression," but measure on one real box first —
   production systems are currently tuned around this.
-- Confirm `vm.zone_reclaim_mode` is `0` on the broker hosts (else first-touch reclaims locally before
-  spilling → latency stalls).
-- Parallel first-touch placement is *approximate* (depends on the scheduler spreading the zeroing
-  threads); fine with RAM headroom, but note it's not the guaranteed 50/50 of `mbind` interleave.
+- Confirm `vm.zone_reclaim_mode` is `0` on the broker hosts (`cat /proc/sys/vm/zone_reclaim_mode`).
+  Non-zero turns `ImageBuffer`'s first-touch into synchronous local reclaim (page-cache eviction /
+  writeback) → startup latency stalls; `0` just spills the page to a remote node.
+- `ImageBuffer` first-touch placement is *approximate* (depends on the scheduler spreading the zeroing
+  threads); fine with RAM headroom, but not the guaranteed 50/50 of `mbind` interleave. Deterministic
+  per-page interleave, if ever needed, is a raw `mbind(MPOL_INTERLEAVE)` syscall — still libnuma-free.
 
-## 5. Suggested order (low-risk first)
+## 5. Remaining order (low-risk first)
 
-1. **G1** — document `CUDA_VISIBLE_DEVICES` launch (no code).
-2. ~~**G2** — per-worker GPU pin in `jfjoch_process`.~~ **DONE** (`pin_gpu()`).
-3. **ImageBuffer** parallel first-touch (drops one libnuma user, helps startup; stands alone).
-4. **G4 A/B** on a real broker; if clean, drop CPU pinning.
-5. **NUMAHWPolicy** simplify (keep GPU select, drop CPU pin + mem bind) + `CUDAWrapper` sysfs
-   → remove `NUMA_LIBRARY` from CMake.
+1. **G1** — document the `CUDA_VISIBLE_DEVICES` launch (no code).
+2. **G4 A/B** on a real broker — the gate for dropping the `RunOnNode` CPU pin.
+3. **`NUMAHWPolicy`** — drop `MemOnNode`; drop/replace `RunOnNode` per G4; keep `SelectGPU`. Then
+   remove `NUMA_LIBRARY` from CMake → G3 complete.
+
+*(DONE so far: G2 `pin_gpu`, `ImageBuffer` first-touch, `acquisition_device` de-NUMA, `CUDAWrapper`
+sysfs node lookup.)*