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 commentaires · Sur GitHub
BenchmarksServer & APIAMD / HIPNVIDIA / CUDAModels & quantsWindowsLinux
Description
**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>Sur le site
Liens install, modèles, releases.