diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index f0170dba..36351044 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -1095,6 +1095,18 @@ namespace MobileGL::MG_Remote::Server { // mid-drain is acted on within the stride - the frame was being applied either way. constexpr Uint32 kFaultCheckRecordStride = 256; Uint32 recordsSinceFaultCheck = 0; + // THE FAULT POLL ALSO RUNS BEFORE THE FIRST RECORD THE LAST POLL COULD NOT HAVE SEEN. The + // loop's per-wake poll covers what was in the ring when it woke, and the stride and Present + // cover a long drain; a record published while this drain was still running - after the + // previous record was applied and acknowledged, so after a reset the driver may already + // report - used to be popped and applied unpolled, and a GUILTY context ran one more record + // (ServerLoopEglLatchTest under CPU load). `polledHead` is the command ring's head as loaded + // just BEFORE the last poll: every record below it was visible when that poll ran. A record + // at or past it re-loads the head and polls first. While the client stays ahead this is one + // poll per batch the drain catches up to, never more than one per record. + Transport::LinkSignals signals{}; + if (auto* link = session.DataLink()) signals = link->Signals(); + std::uint64_t polledHead = 0; for (;;) { if (SessionLatched()) break; if (m_windowRequests.load(std::memory_order_acquire) != 0) { @@ -1103,14 +1115,18 @@ namespace MobileGL::MG_Remote::Server { } bool corrupt = false; const bool popped = consumer.ApplyOne( - [this, &applier, timed, &recordsSinceFaultCheck](const Transport::RingRecordView& record) { + [this, &applier, timed, &recordsSinceFaultCheck, &signals, + &polledHead](const Transport::RingRecordView& record) { // Checked inside the drain too: a producer may publish more work while this - // drain is running, after the outer-loop check (stride above). - if (m_backend != nullptr && - (++recordsSinceFaultCheck >= kFaultCheckRecordStride || - record.kind == static_cast(MG_Pipe::MGPWireOp::Present))) { - recordsSinceFaultCheck = 0; - if (m_backend->LatchIfGpuFaulted()) return false; + // drain is running, after the outer-loop check (stride above, and `polledHead`). + if (m_backend != nullptr) { + const bool unseen = signals.CmdHead != nullptr && record.cursor >= polledHead; + if (unseen || ++recordsSinceFaultCheck >= kFaultCheckRecordStride || + record.kind == static_cast(MG_Pipe::MGPWireOp::Present)) { + if (unseen) polledHead = signals.CmdHead->load(std::memory_order_acquire); + recordsSinceFaultCheck = 0; + if (m_backend->LatchIfGpuFaulted()) return false; + } } if (SessionLatched()) return false; // P65ServerFrame: timed around the ONE call that is the server's work, and diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index 6ed9600f..da016ad3 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -2021,6 +2021,20 @@ namespace { std::atomic g_fakeResetStatus{GL_NO_ERROR}; GLenum FakeGetGraphicsResetStatus() { return g_fakeResetStatus.exchange(GL_NO_ERROR); } + // THE WINDOW, HELD OPEN. The server acknowledges a record before it looks for the next one, so + // a record the client publishes right after that acknowledgement can be popped by the SAME + // drain, without passing the apply loop's per-wake fault poll. That used to happen only under + // CPU load (one CI failure in this case); the between-records hook makes it happen every time: + // armed, it holds the drain after the record it follows until the GUILTY record is in the ring. + std::atomic g_holdDrainForGuilty{0}; + std::atomic g_guiltyPublished{false}; + void HoldDrainUntilTheGuiltyRecordIsPublished() { + if (g_holdDrainForGuilty.exchange(0) != 1) return; + const auto until = std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (!g_guiltyPublished.load(std::memory_order_acquire) && std::chrono::steady_clock::now() < until) + std::this_thread::yield(); + } + enum : int { kResetInnocentEnded = 1, // an INNOCENT report latched the session (the compositor would die with a client) kResetGuiltyApplied = 2, // a record published after the GUILTY report was applied @@ -2041,21 +2055,28 @@ namespace { gles.glGetGraphicsResetStatus = &FakeGetGraphicsResetStatus; MG_Backend::DirectGLES::SetGLESFuncsTable(gles); int failed = 0; - const auto applyOne = [&] { + const auto applyOne = [&](std::atomic* publishedFlag) { MG_Pipe::MGPMemoryBarrier barrier{}; barrier.Bits = 0x2000u; const Uint64 seq = fixture.encoder.EncodeRecord(MG_Pipe::MGPWireOp::MemoryBarrier, &barrier, sizeof(barrier)); fixture.encoder.Publish(); fixture.producer.PublishAndNotify(seq); + if (publishedFlag != nullptr) publishedFlag->store(true, std::memory_order_release); return fixture.producer.WaitForApplied(seq, 2000) == Transport::SessionWait::Reached; }; // Another context hung the GPU; this one only waited behind it. g_fakeResetStatus = GL_INNOCENT_CONTEXT_RESET; - if (!applyOne()) failed |= kResetHealthyNotApplied; - if (!applyOne() || MG_Remote::SessionLatched()) failed |= kResetInnocentEnded; + if (!applyOne(nullptr)) failed |= kResetHealthyNotApplied; + if (!applyOne(nullptr) || MG_Remote::SessionLatched()) failed |= kResetInnocentEnded; + // The drain that applies this healthy record is held, after acknowledging it, until the + // GUILTY record below is published, so that drain pops the GUILTY record itself. + Server::ServerLoopInstance().SetBetweenRecordsHookForTesting(&HoldDrainUntilTheGuiltyRecordIsPublished); + g_holdDrainForGuilty = 1; + if (!applyOne(nullptr)) failed |= kResetHealthyNotApplied; // This context's own work hung the GPU. g_fakeResetStatus = GL_GUILTY_CONTEXT_RESET; - if (applyOne()) failed |= kResetGuiltyApplied; + if (applyOne(&g_guiltyPublished)) failed |= kResetGuiltyApplied; + Server::ServerLoopInstance().SetBetweenRecordsHookForTesting(nullptr); if (!MG_Remote::SessionLatched() || MG_Remote::SessionLatchedFamily() != MG_Remote::MGFatalFamily::BackendDeviceLost || std::strstr(MG_Remote::SessionLatchedLine(), "Espryt:apply") == nullptr) failed |= kResetGuiltyNotLatched;