Issues / #2134

#2134 Foresight swap: a slot copied before an adaptive exchange can hold another expert's bytes (FS_VERIFY: 19 BAD on clean v0.1.43)

open · @radoslawsz · 0 comentarios · En GitHub

BenchmarksServer & APIAMD / HIPNVIDIA / CUDAModels & quantsWindowsLinux

Descripción

**Summary**
When the Foresight swap space is on (`STRATA_FS_SLOTS > 0`) and the adaptive cache swaps experts between VRAM and the page-locked RAM copy, a slot copy can read the wrong expert. `note_miss` / `predict` issue the copy from an expert's address in the RAM copy before `commit_exchanges()` / `commit_copies()` / `commit_flip()` run. The copy can land after the exchange has rewritten that RAM place with the outgoing expert's bytes. The slot is then ready for expert X but holds expert Y, and `take()` serves it when X misses the cache. Nothing in the exchange path touches the Foresight slots.

The default configuration is not affected, because Foresight is opt-in.

**Reproduction (clean v0.1.43, 7695859a, no other patches)**
- Hardware: RTX PRO 6000 (CUDA, sm_89 build), Linux, Docker.
- Model: Qwen3.8-Flash-Next GSQ IQ3_XXS.
- Configuration:
  - resident RAM mode: 35.8 GiB of experts page-locked, expert cache 4698;
  - adaptive swaps on (defaults);
  - `STRATA_FS_SLOTS=4 STRATA_FS_VERIFY=1`; other FS switches at their defaults, so the router look-ahead is on.
- Workload: HumanEval 50, temperature 1.0, one request at a time.

| build | verified ok | BAD | dropped by exchanges | decode tok/s (whole run / median per request) |
|---|---|---|---|---|
| v0.1.43 | 425,973 | **19** | — | 75.3 / 71.8 |
| v0.1.43 + patch below | 763,665 | **0** | 263,727 | 74.7 / 71.8 |

The patch costs no speed: the median per request is 71.8 tok/s in both runs, and the 0.8% difference over the whole run is run-to-run noise. We first saw it on our fork: 1 BAD in 611,201, then 0 BAD in 617,808 with the same fix.

**Where (v0.1.43)**
- `src/core/expert_source.cpp:3320` `FileExpertSource::commit_exchanges()`
- `src/core/expert_source.cpp:3349` `FileExpertSource::commit_copies()`
- `src/core/expert_source.cpp:3360` `FileExpertSource::commit_flip()`
- `src/core/expert_source.cpp:4876` / `4881` `ForesightSwap::note_miss()` / `predict()`

**Fix**
In all three commit functions, every exchange invalidates the Foresight slots of both experts it moves (`x.in`, `x.out`). Under `smu_`, the slot is freed and the next miss or prediction copies again. A copy still in flight finishes first, because it is on the same stream. The patch is 37 lines and applies cleanly to v0.1.43:

<details><summary>patch</summary>

```diff
diff --git a/include/strata/core/foresight_swap.hpp b/include/strata/core/foresight_swap.hpp
index 379f4079..a0402d85 100644
--- a/include/strata/core/foresight_swap.hpp
+++ b/include/strata/core/foresight_swap.hpp
@@ -64,6 +64,7 @@ struct ForesightSwap {
     uint64_t hits = 0, copies = 0, busy = 0, over_budget = 0, pending = 0, predicted = 0;   // (under smu_)
     bool verify = false;                     ///< STRATA_FS_VERIFY=1: read every landed slot back and compare (slow, tests)
     uint64_t verify_ok = 0, verify_bad = 0;
+    uint64_t invalidated = 0;                ///< slots dropped because an exchange moved their expert's bytes (under smu_)
 
     /// Allocates the slots on each card and starts the filler.  `card_of_layer[l]` indexes `devices` (CUDA ids).
     bool init(int slots_per_layer, int64_t layers, int64_t experts, const std::vector<int>& card_of_layer,
@@ -72,6 +73,9 @@ struct ForesightSwap {
 
     /// The pool, for a routed expert that is not in the cache: true and its device address when a landed slot holds it.
     bool take(int64_t layer, int32_t expert, unsigned long long& ptr);
+    /// The adaptation moved this expert's bytes in the RAM copy (an exchange): a copy issued from the old place
+    /// may hold another expert's bytes, so the slot is dropped (never served); the next miss or prediction copies again
+    void invalidate(int64_t layer, int32_t expert);
     /// The pool, for an expert the CPU computes now: maybe copy it into a slot of its layer for the next windows.
     void note_miss(ExpertSource* src, int64_t layer, int32_t expert);
     /// The router look-ahead (any thread), for an expert the next layer's router predicts and no cache holds: maybe copy
@@ -92,6 +96,9 @@ private:
     void fill_loop();
 };
 
+/// The swap space the exchanges invalidate (set by the serve path, null when off)
+void set_fs_invalidate(ForesightSwap* fs);
+void fs_invalidate_index(size_t index, int64_t n_expert);
 ForesightSwap* foresight_swap_from_env(int64_t layers, int64_t experts, const std::vector<int>& card_of_layer,
                                        const std::vector<int>& devices);
 
diff --git a/src/core/expert_source.cpp b/src/core/expert_source.cpp
index ad28f56f..1cbdae40 100644
--- a/src/core/expert_source.cpp
+++ b/src/core/expert_source.cpp
@@ -3322,6 +3322,8 @@ int64_t FileExpertSource::commit_exchanges() {
     for (const Exchange& x : staged_)
         if (!soft_in_.empty()) soft_in_[x.out] = SIZE_MAX;
     for (const Exchange& x : staged_) {
+        fs_invalidate_index(x.in, n_expert_);    // both experts' bytes move in the RAM copy
+        fs_invalidate_index(x.out, n_expert_);
         const uint8_t* src = override_[x.out];
         if (exchange_storage_.active()) {
             if (!exchange_storage_.commit(x.in, x.out, (size_t)x.q, src, (size_t)x.bytes)) {
@@ -3349,6 +3351,8 @@ int64_t FileExpertSource::commit_exchanges() {
 void FileExpertSource::commit_copies() {
     if (exchange_storage_.active()) return;   // STRATA_EXCHANGE_ROTATE: ownership moves in commit_flip, nothing to copy
     for (const Exchange& x : staged_) {
+        fs_invalidate_index(x.in, n_expert_);    // (as in commit_exchanges)
+        fs_invalidate_index(x.out, n_expert_);
         const uint8_t* src = override_.empty() ? nullptr : override_[x.out];
         const uint64_t at = complement_offsets_[x.in];
         if (src != nullptr && at != kNoComplement && at <= complement_bytes_ && x.bytes <= complement_bytes_ - at &&
@@ -3360,6 +3364,8 @@ void FileExpertSource::commit_copies() {
 int64_t FileExpertSource::commit_flip() {
     int64_t n = 0;
     for (const Exchange& x : staged_) {
+        fs_invalidate_index(x.in, n_expert_);    // a copy requested between commit_copies and here read old bytes
+        fs_invalidate_index(x.out, n_expert_);
         const uint8_t* src = override_[x.out];
         if (exchange_storage_.active()) {   // STRATA_EXCHANGE_ROTATE: the same ownership transfer as commit_exchanges
             if (!exchange_storage_.commit(x.in, x.out, (size_t) x.q, src, (size_t) x.bytes)) {
@@ -4873,6 +4879,27 @@ bool ForesightSwap::take(int64_t layer, int32_t expert, unsigned long long& ptr)
     return true;
 }
 
+static std::atomic<ForesightSwap*> g_fs_inval{nullptr};
+void set_fs_invalidate(ForesightSwap* fs) { g_fs_inval.store(fs, std::memory_order_release); }
+void fs_invalidate_index(size_t index, int64_t n_expert) {
+    ForesightSwap* fs = g_fs_inval.load(std::memory_order_acquire);
+    if (fs == nullptr || n_expert <= 0) return;
+    fs->invalidate((int64_t) (index / (size_t) n_expert), (int32_t) (index % (size_t) n_expert));
+}
+
+void ForesightSwap::invalidate(int64_t layer, int32_t expert) {
+    if (layer < 0 || layer >= n_layers || expert < 0 || expert >= n_expert) return;
+    std::lock_guard<std::mutex> smu_lk(smu_);
+    const size_t key = (size_t) (layer * n_expert + expert);
+    const int s = where[key];
+    if (s < 0) return;
+    where[key] = -1;
+    Slot& sl = slot[(size_t) (layer * slots + s)];
+    sl.expert = -1;     // free for the next request; a copy still in flight finishes first (same stream)
+    sl.ready = false;
+    ++invalidated;
+}
+
 void ForesightSwap::note_miss(ExpertSource* src, int64_t layer, int32_t expert) {
     std::lock_guard<std::mutex> smu_lk(smu_);
     request_locked(src, layer, expert, false);
@@ -4944,6 +4971,7 @@ std::string ForesightSwap::report() const {
                   (unsigned long long) hits, (unsigned long long) copies, (unsigned long long) predicted,
                   (unsigned long long) pending, (unsigned long long) busy, (unsigned long long) over_budget, verify ? "" : "");
     std::string r = buf;
+    if (invalidated > 0) r += ", " + std::to_string(invalidated) + " dropped by exchanges";
     if (verify) r += ", verified " + std::to_string(verify_ok) + " ok / " + std::to_string(verify_bad) + " BAD";
     return r;
 }
diff --git a/src/program/generate.cpp b/src/program/generate.cpp
index 4aa057db..71d8e824 100644
--- a/src/program/generate.cpp
+++ b/src/program/generate.cpp
@@ -8191,11 +8191,12 @@ int main(int argc, char** argv) {
             fs_swap.reset(strata::core::foresight_swap_from_env(g.n_layers, g.n_expert, card, devs));
             drive.d.fs = fs_swap.get();
             if (fs_swap && drive.d.lookahead != nullptr) lookahead.set_foresight(fs_swap.get());
+            strata::core::set_fs_invalidate(fs_swap.get());   // exchanges drop the slots of the experts they move
         }
         // the look-ahead thread must not touch the swap space once it is gone (declared after it: destroyed first)
         struct FsDetach {
             strata::core::RouterLookahead* la;
-            ~FsDetach() { la->set_foresight(nullptr); }
+            ~FsDetach() { la->set_foresight(nullptr); strata::core::set_fs_invalidate(nullptr); }
         } fs_detach{&lookahead};
         drive.d.pcie_num = std::max(0, std::min(256, (int) (o.pcie_frac * 256.0 + 0.5)));
         const bool all_experts_resident = !host_res.empty() &&
```
</details>

En el sitio

Enlaces a install, modelos, releases.