From a3466e6f4ca31da4d85e0d34445eb7eccc732939 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 07:57:33 -0400 Subject: [PATCH 1/5] [MG_Remote] (Disaggregated): P5e ra - SEG_EVENT stops aborting and starts flow-controlling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONTRACT-P5E §2.6. Fatal{EventRingOverflow}'s premise was "the client drains at every verb, so a full 256 KiB ring is a burst no workload has". A run-ahead client drains at its waits, which on the steady path are one present apart, so a full ring is an ordinary backlog and aborting on it would turn the reverse channel's capacity into a workload limit. - The producer blocks instead (ReserveEventOrBlock, one entry point for all four event kinds): publish what is in the ring, ring the client, park on this server's own bell until the latch clears, retry. A server that does not publish kCapRunAheadApply keeps P5C's Fatal byte for byte. - Two named refusals survive, and they are the impossible cases rather than the busy ones: a single event larger than half the ring (no drain can ever make room) and a 30 s wait for a client that is not draining because it is not running. - The deadlock argument needs both halves, so both are here. EventRingConsumer::Drained() now CLEARS AND RINGS - a cleared flag with no bell is the lost wakeup the forward direction's publish-then-ring order exists to prevent - and SessionProducer gains WaitForAppliedOrEventBacklog / WaitForPresentAckOrEventBacklog so a parked client is always wakeable by the server that is waiting for it. The old two waits keep their callers: teardown has no ring to drain and must not learn about one. - ServerLoop's park condition gains a fourth clause, and it is the one that says "not now" rather than "wake up": with the ring full a waiting record is not work yet, because applying it would only park the producer half way through. Stop and Control still win, so a teardown is never held up by a client that stopped draining. - Inert this phase: nothing publishes kCapRunAheadApply until the integration commit, so every arm above answers as it did before. --- MobileGL/MG_Remote/Server/ServerLoop.cpp | 18 +++- MobileGL/MG_Remote/Server/ServerSession.cpp | 105 +++++++++++++++++--- MobileGL/MG_Remote/Transport/EventRing.h | 20 +++- MobileGL/MG_Remote/Transport/Ring.cpp | 34 +++++++ MobileGL/MG_Remote/Transport/SessionRings.h | 16 +++ 5 files changed, 173 insertions(+), 20 deletions(-) diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 1d56898d..114a9a08 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -377,15 +377,27 @@ namespace MobileGL::MG_Remote::Server { // CondVarDoorbell::Kill() breaks that, and Kill is the CLIENT's teardown call, not // something an EGL make-current request may perform. So the predicate carries all // three arming conditions and the doorbell wakes the thread for any of them. + // P5e (ra, CONTRACT-P5E §2.6) ADDS A FOURTH, AND IT IS THE ONE THAT SAYS "NOT NOW" + // RATHER THAN "WAKE UP". A full SEG_EVENT means the apply thread has nowhere to put + // the events the next record's apply would produce, so a waiting record is NOT work + // yet: applying it would only park the producer half way through. Stop and Control + // still win - a teardown may not be held up by a client that stopped draining - which + // is why they are tested first and the ring test sits between them and the queue. + // + // The wake comes from EventRingConsumer::Drained(), which clears the latch AND rings + // this bell (that pairing is the whole of the flow-control protocol; a cleared flag + // with no bell is the lost wakeup the forward direction's publish-then-ring order + // exists to prevent). const auto ready = [this, &control, &ring] { - return control.cmdHead.load(std::memory_order_acquire) != ring.LocalTail() || - m_stopRequested.load(std::memory_order_acquire) || ControlIsPending(); + if (m_stopRequested.load(std::memory_order_acquire) || ControlIsPending()) return true; + if (control.eventRingFull.load(std::memory_order_acquire) != 0) return false; + return control.cmdHead.load(std::memory_order_acquire) != ring.LocalTail(); }; for (;;) { PumpControlRequest(); if (m_stopRequested.load(std::memory_order_acquire)) break; - DrainRing(); + if (control.eventRingFull.load(std::memory_order_acquire) == 0) DrainRing(); if (m_stopRequested.load(std::memory_order_acquire)) break; if (ready()) continue; diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 8401dbec..f8734b58 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -180,6 +180,87 @@ namespace MobileGL::MG_Remote::Server { std::abort(); } + // ---- P5e (ra), CONTRACT-P5E §2.6: the ring stops being a Fatal and becomes a queue -- + // + // THE FATAL'S PREMISE DIES WITH THE LOCKSTEP. "A full ring is a producer burst no + // workload has" was true because the client drained at EVERY verb: the ring was empty + // whenever the server started a record, so filling 256 KiB inside one apply meant a + // defect. A run-ahead client drains at its waits, which on the steady path are one + // present apart, so a full ring is now an ordinary backlog and aborting on it would + // turn the reverse channel's capacity into a workload limit. + // + // SO THE PRODUCER BLOCKS. It publishes what is already in the ring and rings the + // client (a client that has not yet noticed cannot drain), then parks on its own bell + // until the client's Drained() clears the latch and rings back. The deadlock argument + // is the contract's, and it needs BOTH halves: the server blocks only here, the client + // blocks only on watermarks the server advances, and every client park breaks on + // `eventRingFull` as well (SessionProducer::WaitFor*OrEventBacklog) - so a parked + // client is always wakeable by the server that is waiting for it. + // + // TWO NAMED REFUSALS SURVIVE, and they are the impossible cases rather than the busy + // ones: a single event larger than the ring can EVER hold (no amount of draining + // helps), and a wait that ran out of patience (the peer is gone or wedged). A server + // that does not publish kCapRunAheadApply keeps P5C's Fatal unchanged. + Bool ServerPublishesRunAhead(const ServerSession& session) { + if (!session.CallMaskIsSet()) return false; + return (session.CallMask() & static_cast(MG_Pipe::kCapRunAheadApply)) != 0; + } + + constexpr Uint32 kEventBacklogWaitMs = 30000; + + void* ReserveEventOrBlock(ServerSession& session, Transport::EventKind kind, + const char* eventName, Uint64 payloadBytes) { + void* slot = session.Events().Reserve(kind, payloadBytes); + if (slot != nullptr) return slot; + if (!ServerPublishesRunAhead(session)) FatalEventRingOverflow(eventName, payloadBytes); + + // Impossible case one: the record does not fit an EMPTY ring. Reserve caps one + // record at capacity/2 (Ring.h), so no drain can ever make room and blocking would + // be a hang with a comment on it. + const Uint64 cap = session.Events().Ring().MaxRecordBytes(); + if (payloadBytes + sizeof(Transport::RingRecordHeader) > cap) { + MGLOG_F("MGPipe: Fatal{EventRingOverflow} - %s needs %llu bytes and SEG_EVENT " + "caps ONE record at %llu (half its capacity). Flow control cannot help: " + "no amount of draining makes a record fit a ring that is too small for " + "it. Raise MOBILEGL_IPC_EVENT_KB or slice the event at its producer, as " + "the writeback path already does", + eventName, static_cast(payloadBytes), + static_cast(cap)); + std::abort(); + } + + Transport::RingControl& control = session.Control(); + Transport::Doorbell& bell = session.ConsumerDoorbell(); + const auto drained = [&control] { + return control.eventRingFull.load(std::memory_order_acquire) == 0; + }; + for (Uint32 round = 0; round < 2; ++round) { + // PUBLISH AND RING FIRST. The client cannot drain a ring whose head it has not + // been shown, and the latch Reserve just set is only useful to a client that + // gets a bell with it. + session.PublishEvents(); + if (!drained() && !bell.Wait(control.consumerParked, drained, /*spinUs=*/0, + kEventBacklogWaitMs)) { + MGLOG_F("MGPipe: Fatal{EventRingOverflow} - %s waited %u ms for the client " + "to drain SEG_EVENT and it never did. Under run-ahead the apply " + "thread parks on this ring (CONTRACT-P5E §2.6); a client that does " + "not drain is one that is not running", + eventName, kEventBacklogWaitMs); + std::abort(); + } + slot = session.Events().Reserve(kind, payloadBytes); + if (slot != nullptr) return slot; + } + // Two full rounds against a ring the record provably fits: something else is + // producing into it, which on this side is impossible (one producer, the apply + // thread, by construction). + MGLOG_F("MGPipe: Fatal{EventRingOverflow} - %s could not reserve %llu bytes on an " + "emptied SEG_EVENT. The apply thread is SEG_EVENT's only producer, so a " + "drained ring that still refuses a record it fits is a corrupt cursor set", + eventName, static_cast(payloadBytes)); + std::abort(); + } + void ServerOnBufferWriteback(MG_Pipe::MGPipeHandle res, Uint64 offset, MG_Pipe::MGPBlobRef bytes) { if (bytes.Seg != MG_Pipe::kMGHostSpanSegNone) { @@ -195,10 +276,8 @@ namespace MobileGL::MG_Remote::Server { } ServerSession& session = ServerSessionInstance(); const Uint64 payloadBytes = sizeof(Transport::EventBufferWritebackHead) + bytes.Size; - void* slot = session.Events().Reserve(Transport::kEventBufferWriteback, payloadBytes); - if (slot == nullptr) { - FatalEventRingOverflow("kEventBufferWriteback", payloadBytes); - } + void* slot = ReserveEventOrBlock(session, Transport::kEventBufferWriteback, + "kEventBufferWriteback", payloadBytes); Transport::EventBufferWritebackHead head{}; head.Resource = Transport::EventHandle{res.Slot, res.Gen}; head.Offset = offset; @@ -217,10 +296,8 @@ namespace MobileGL::MG_Remote::Server { ServerSession& session = ServerSessionInstance(); const Uint64 tailBytes = static_cast(rangeCount) * sizeof(Transport::EventRange); const Uint64 payloadBytes = sizeof(Transport::EventGpuWrittenHead) + tailBytes; - void* slot = session.Events().Reserve(Transport::kEventGpuWritten, payloadBytes); - if (slot == nullptr) { - FatalEventRingOverflow("kEventGpuWritten", payloadBytes); - } + void* slot = ReserveEventOrBlock(session, Transport::kEventGpuWritten, + "kEventGpuWritten", payloadBytes); Transport::EventGpuWrittenHead head{}; head.Resource = Transport::EventHandle{res.Slot, res.Gen}; head.RangeCount = static_cast(rangeCount); @@ -237,10 +314,8 @@ namespace MobileGL::MG_Remote::Server { void ServerOnSurfaceChanged(const MG_Pipe::MGPSurfaceInfo* info) { ServerSession& session = ServerSessionInstance(); constexpr Uint64 payloadBytes = sizeof(Transport::EventSurfaceChangedHead); - void* slot = session.Events().Reserve(Transport::kEventSurfaceChanged, payloadBytes); - if (slot == nullptr) { - FatalEventRingOverflow("kEventSurfaceChanged", payloadBytes); - } + void* slot = ReserveEventOrBlock(session, Transport::kEventSurfaceChanged, + "kEventSurfaceChanged", payloadBytes); Transport::EventSurfaceChangedHead head{}; if (info != nullptr) { head.Width = info->Width; @@ -649,10 +724,8 @@ namespace MobileGL::MG_Remote::Server { } const Uint32 messageBytes = static_cast(length) + 1; const Uint64 payloadBytes = sizeof(Transport::EventGlErrorHead) + messageBytes; - void* slot = m_events.Reserve(Transport::kEventGlError, payloadBytes); - if (slot == nullptr) { - FatalEventRingOverflow("kEventGlError", payloadBytes); - } + void* slot = ReserveEventOrBlock(*this, Transport::kEventGlError, "kEventGlError", + payloadBytes); Transport::EventGlErrorHead head{}; head.Code = code; head.MessageBytes = messageBytes; diff --git a/MobileGL/MG_Remote/Transport/EventRing.h b/MobileGL/MG_Remote/Transport/EventRing.h index 644664af..3911f592 100644 --- a/MobileGL/MG_Remote/Transport/EventRing.h +++ b/MobileGL/MG_Remote/Transport/EventRing.h @@ -189,10 +189,26 @@ namespace MobileGL::MG_Remote::Transport { void Drained() { m_consumer.PublishRetired(); if (m_cmdControl != nullptr) { - m_cmdControl->eventRingFull.store(0, std::memory_order_release); + // P5e (ra, CONTRACT-P5E §2.6): CLEARING THE LATCH IS ONLY HALF OF IT. Under + // flow control the server is PARKED on this flag - either inside a producer + // that could not Reserve, or at a record boundary refusing to start the next + // record - and a cleared flag with no bell is a wakeup lost exactly the way + // the forward direction's publish-then-ring order exists to prevent. The + // exchange is what makes the ring one-shot: only the drain that actually found + // the flag up rings, so a steady stream of drains costs one relaxed RMW. + const bool wasFull = + m_cmdControl->eventRingFull.exchange(0, std::memory_order_acq_rel) != 0; + if (wasFull && m_serverBell != nullptr) { + NotifyIfParked(*m_serverBell, m_cmdControl->consumerParked); + } } } + // The bell the SERVER parks on, lent by the session so that Drained() can ring it. + // Null in every fixture that consumes a ring without a server behind it, and the + // ring's clear then behaves exactly as it did before P5e. + void SetServerDoorbell(Doorbell* bell) { m_serverBell = bell; } + // Byte offset of `payload` inside SEG_EVENT, which is what an // OnBufferWriteback MGPBlobRef must carry (Seg = kSegEvent, Offset = // this, Size = the head's Size). Never a host address - R-2's rule B. @@ -217,6 +233,8 @@ namespace MobileGL::MG_Remote::Transport { RingControl* m_cmdControl = nullptr; RingConsumer m_consumer; const std::uint8_t* m_segmentBase = nullptr; + // P5e (ra): the server's bell, rung by Drained() when it cleared a full latch. + Doorbell* m_serverBell = nullptr; }; } // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index a1473b50..dec86938 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -501,6 +501,40 @@ namespace MobileGL::MG_Remote::Transport { timeoutMs); } + // P5e (ra), CONTRACT-P5E §2.6. Two predicates, one OR each, and the caller decides which + // of the two happened by reading the watermark it actually cares about. + bool SessionProducer::EventRingIsFull() const { + return m_control != nullptr && m_control->eventRingFull.load(std::memory_order_acquire) != 0; + } + + SessionWait SessionProducer::WaitForAppliedOrEventBacklog(std::uint64_t seq, + std::uint32_t timeoutMs) { + if (!Valid()) { + return SessionWait::TimedOut; + } + RingControl* control = m_control; + return Park( + [control, seq] { + return Watermark::Reached(control->appliedSeq, seq) || + control->eventRingFull.load(std::memory_order_acquire) != 0; + }, + timeoutMs); + } + + SessionWait SessionProducer::WaitForPresentAckOrEventBacklog(std::uint64_t serial, + std::uint32_t timeoutMs) { + if (!Valid()) { + return SessionWait::TimedOut; + } + RingControl* control = m_control; + return Park( + [control, serial] { + return Watermark::Reached(control->presentAckSerial, serial) || + control->eventRingFull.load(std::memory_order_acquire) != 0; + }, + timeoutMs); + } + SessionWait SessionProducer::WaitForCmdSpace(std::uint64_t bytes, std::uint32_t timeoutMs) { if (!Valid()) { return SessionWait::TimedOut; diff --git a/MobileGL/MG_Remote/Transport/SessionRings.h b/MobileGL/MG_Remote/Transport/SessionRings.h index b3fd1c35..bcb445ff 100644 --- a/MobileGL/MG_Remote/Transport/SessionRings.h +++ b/MobileGL/MG_Remote/Transport/SessionRings.h @@ -325,6 +325,22 @@ namespace MobileGL::MG_Remote::Transport { SessionWait WaitForApplied(std::uint64_t seq, std::uint32_t timeoutMs); // Present throttle. SessionWait WaitForPresentAck(std::uint64_t serial, std::uint32_t timeoutMs); + + // ---- P5e (ra), CONTRACT-P5E §2.6: the same two waits, PLUS the event ring --------- + // + // A parked client must be wakeable by a server that ran out of SEG_EVENT, or the + // flow-control deadlock is one burst wide: the server stops producing, so it stops + // applying, so appliedSeq never reaches the seq this waiter is parked on, so the ring + // is never drained. Both predicates therefore break on `eventRingFull` as well, and + // the CALLER re-reads the watermark to find out which of the two woke it - a third + // SessionWait value would have made every existing `== Reached` site a bug. + // + // The old entry points above are unchanged and keep their callers: teardown's + // WaitForApplied has no ring to drain and must not learn about one. + SessionWait WaitForAppliedOrEventBacklog(std::uint64_t seq, std::uint32_t timeoutMs); + SessionWait WaitForPresentAckOrEventBacklog(std::uint64_t serial, std::uint32_t timeoutMs); + // True when the peer latched "SEG_EVENT is full and I stopped producing". + bool EventRingIsFull() const; // Back-pressure when Reserve returned nullptr. NEVER call this when // FreeBytes() is already >= the record: Ring.h:226-233 - a nullptr with // enough free bytes can only mean "too big, chunk", and waiting on it From 6e6b86e592e2aa1c67e3b83d60c7d6a7d3032b42 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 07:58:06 -0400 Subject: [PATCH 2/5] [MG_Remote, MG_Pipe, MG_Impl, MG_Backend] (Disaggregated): P5e ra - the wait rule, the present credit and server-owned gPipeInputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONTRACT-P5E §2.3. EmitAndWaitTails now switches on the generated WaitClass column when RunAheadArmed(): kWaitNone and kWaitPresent return at the publish, kWaitApplied waits and then releases the fill pins, and a reply row is unchanged. It REPLACES the BatchWaits skip rather than layering on it - the batch rule reads the call CLASS and this one reads what an apply READS, and the server's MGPipeBarriered reads the second, so two rules layered is two halves of the wire disagreeing about whether the client is parked. - §1: RunAheadArmed() = barrierArmed && Ipc.RunAhead && Caps().HasCap(kCapRunAheadApply), latched at the first caps adoption after Start and demotable but never promotable - a record already published without a wait cannot be un-published. A knob set against a server that does not publish the bit logs once and runs lockstep. - §2.4: MGPPresent::FrameSerial is minted client-side, 1-based, by AcquirePresentCredit, which pays the credit BEFORE the encode (a client that encoded first would hold a SEG_CMD reservation across the park). ServerVerbSink::OnPresent returns one credit per swap through ServerSession::ReturnPresentCredit, which has had no production caller since v1 wrote it; serial 0 on a run-ahead server is Fatal{ProtocolCorruption, "Present.FrameSerial"}. The stats line and the wire ledger gain credit-waits. - §2.5, the drains and the forced waits: every wait exit drains SEG_EVENT (the applied wait, the present-ack wait, the cmd-space wait and - through a hook the session lends the encoder - the stage bell); glFinish becomes WaitForApplyToCatchUp, since "the commands issued so far have completed" is a promise a queue nobody waits on can break; every Server* EGL forwarder waits first, because the control mailbox is pumped BETWEEN drain batches and a make-current landing between two run-ahead records is not a wrong pixel but a wrong everything. ServerSwapEGLBuffers is not on that list: present is the swap. - §2.5 / ruling 15 (ID-93): resource_subdata's BUFFER half stops waiting. The row keeps kReplySlot, the server still posts, and the slot is never read - which is what makes the 64 KB persistent-map push fire-and-forget instead of a hidden round trip per block. MGPipeSubDataWantsItsReply is the one predicate, read by the route and by the emit table, so the two cannot disagree about whether a wait is owed. The texture half keeps its reply: DrainTextureSubData clears a level's dirty flag on the accepted answer. - §3: gPipeInputs becomes server-role memory for unbarriered records. The tracker walk and the emitters still run - they produce records - but the serial bump, the verb stamp, the server-stamp withdrawal and the whole of step 4 do not, and MGPipeNoteFrontendMutation / MGPipeLeaveVerb return instead of writing. `fillOwed` is a separate name from `barriered` on purpose: the red-once is that one line. - §3.3: MGPipeInputUnfreshRead's counter becomes an UNCONDITIONAL abort under an unbarriered record, knob or no knob, because the value is torn or stale by construction and there is nothing for a counter to count. The marker gains its reason so the lane's allowlist can tell the two apart. - §3.5: RefusePipeInputsTouchWhileApplierOwnsIt is re-derived - under run-ahead the rule is "not at all unless you are the barriered fill", and the BatchWaits early return goes with the argument that justified it. - §2.3's ReleaseFillPins drops the four O-class SharedPtr rows once a barriered apply returns: under lockstep the next fill overwrote them within a verb, under run-ahead a pinned VAO can make the apply thread its last owner a frame later. - §2.7 / ruling 13 (ID-91): the deferred-destroy queue stays live, and an enqueue from an UNBARRIERED apply is now a named finding - loud once, Fatal under strict. - NAMED DEVIATION from §3.1: the identity/liveness pair is still written on every verb. It is not a field of the residual model - no ownership row owns it, no record carries it, no server stamp can answer it - it is "does this process still have a GL context", which only this thread knows and every null-context guard reads. - Inert until the integration commit: RunAheadArmed() is false while kMGPipeP5eRunAheadReady is, so every branch above answers as it did before. --- MobileGL/MG_Backend/MGPipe/PipeInputs.cpp | 38 +- .../MG_Impl/GLImpl/Exporting/Definitions.cpp | 13 +- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 259 ++++++++++++-- MobileGL/MG_Pipe/PipeMutation.h | 30 ++ MobileGL/MG_Pipe/PipeRoute.h | 31 +- .../MG_Remote/Client/BackendObject_Remote.cpp | 26 ++ MobileGL/MG_Remote/Client/ClientSession.cpp | 326 +++++++++++++++++- MobileGL/MG_Remote/Client/ClientSession.h | 89 ++++- MobileGL/MG_Remote/Client/EmitTables.cpp | 18 +- MobileGL/MG_Remote/Client/WireTables.cpp | 30 +- MobileGL/MG_Remote/Server/PipeApplier.cpp | 38 ++ MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 10 +- MobileGL/MG_Remote/Wire/PipeWireCodec.h | 17 + 13 files changed, 867 insertions(+), 58 deletions(-) diff --git a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp index 939c03b5..bc8f9741 100644 --- a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp +++ b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp @@ -58,22 +58,50 @@ namespace MobileGL::MG_Pipe { // existing filter on Fatal{UnmigratedPipeInput still matches, plus the class and the // phase that retires it - a strict abort that did not say which phase owes the answer // would leave the reader exactly where the gate found them. - [[noreturn]] void StrictBarrierPullFatal(MGPipeInputField field, MGPipeVerb verb) { + // P5e (ra): `why` is the second half of the marker, because the lane's allowlist step + // reads this line and the two reasons mean different things to it. "MOBILEGL_IPC_ + // STRICT_ERRORS=1" is the operator asking for a real, ordered value to be loud; + // "UNBARRIERED" is the value not existing (§3.3). The `@` prefix and the + // Fatal{UnmigratedPipeInput tag are unchanged, so every filter written since P5c still + // matches. + [[noreturn]] void StrictBarrierPullFatal(MGPipeInputField field, MGPipeVerb verb, + const char* why) { const SizeT index = static_cast(field); - MGLOG_F("MGPipe: Fatal{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, " - "MOBILEGL_IPC_STRICT_ERRORS=1, retires in %s]", - kMGPipeInputFieldNames[index], MGPipeVerbName(verb), kMGPipeFieldRetiringPhase[index]); + MGLOG_F("MGPipe: Fatal{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, %s, " + "retires in %s]", + kMGPipeInputFieldNames[index], MGPipeVerbName(verb), why, + kMGPipeFieldRetiringPhase[index]); std::abort(); } // One place decides what a BARRIER-PULLED read does, so the field accessors and the // seven sticky forwards cannot drift apart on it. + // + // ---- P5e (ra), CONTRACT-P5E §3.3: THE DETECTOR IS UNCONDITIONAL UNDER AN UNBARRIERED + // RECORD, AND THAT IS WHAT MAKES THE STRICT LANE A GATE ------------------------------- + // + // The knob exists because under LOCKSTEP a pulled row is a real, ordered, fresh value: + // the client filled it and then parked, so "count it and carry on" is an honest + // measurement of remaining debt and MOBILEGL_IPC_STRICT_ERRORS is the operator asking + // for the debt to be loud instead. None of that survives run-ahead. With the client + // running ahead of this apply, the row was either never filled for this verb (§3.1 + // skips the fill) or is being overwritten by a verb two frames later - so the value is + // torn or stale BY CONSTRUCTION and there is nothing for a counter to count. A + // "count it" arm here would be a wrong picture with a number beside it. + // + // rsp is therefore 0 on unbarriered records in every scenario summary (§7's pin): a + // non-zero count would be an abort that did not fire. void CountBarrierPull(MGPipeInputField field, MGPipeVerb verb) { + if (!MGPipeApplierCurrentRecordIsBarriered()) { + StrictBarrierPullFatal(field, verb, "UNBARRIERED, the client did not fill it"); + } ++g_residualPulls; if (MG_Util::PipeStats::Enabled()) { MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::ResidualPulls, 1); } - if (MG_Config::Ipc.StrictErrors) StrictBarrierPullFatal(field, verb); + if (MG_Config::Ipc.StrictErrors) { + StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1"); + } } // The verb's OWN may-read table (FillPoints.def, kMGPipeClassFieldMask). The stamp diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index ed94936a..2275a89d 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -108,7 +108,18 @@ DECLARE_GL_FUNCTION_HEAD(void, DrawArrays, GLenum mode, GLint first, GLsizei cou DECLARE_GL_FUNCTION_HEAD(void, DrawElements, GLenum mode, GLsizei count, GLenum type, const void* indices) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawElements, mode, count, type, indices) DECLARE_GL_FUNCTION_HEAD(void, Enable, GLenum cap) DECLARE_GL_FUNCTION_END_NO_RETURN(void, Enable, cap) DECLARE_GL_FUNCTION_HEAD(void, EnableVertexAttribArray, GLuint index) DECLARE_GL_FUNCTION_END_NO_RETURN(void, EnableVertexAttribArray, index) -MOBILEGL_GL_API void glFinish() { MGLOG_D("Implementing function: %s(...)", __FUNCTION__); } +// P5e (ra), CONTRACT-P5E §2.5. STILL NOTHING ON THE WIRE - Flush and Finish have no record +// (ARCHITECTURE §11) and this stays a no-op on every monolith build. What it is not any more +// is a no-op on a RUN-AHEAD client: "the commands issued so far have completed" is a promise a +// queue the client never waits on can break, and this is the only place an application can ask +// for it. Under lockstep the client had already waited out every command it issued, so the +// call really did have nothing to do; MGPipeClientFinish answers false-cheap there too. +MOBILEGL_GL_API void glFinish() { + MGLOG_D("Implementing function: %s(...)", __FUNCTION__); +#if MOBILEGL_BUILD_DISAGGREGATED + MobileGL::MG_Pipe::MGPipeClientFinish(); +#endif +} MOBILEGL_GL_API void glFlush() { MGLOG_D("Implementing function: %s(...)", __FUNCTION__); } DECLARE_GL_FUNCTION_HEAD(void, FramebufferRenderbuffer, GLenum target, GLenum attachment, GLenum renderbuffertarget, GLuint renderbuffer) DECLARE_GL_FUNCTION_END_NO_RETURN(void, FramebufferRenderbuffer, target, attachment, renderbuffertarget, renderbuffer) DECLARE_GL_FUNCTION_HEAD(void, FramebufferTexture2D, GLenum target, GLenum attachment, GLenum textarget, GLuint texture, GLint level) DECLARE_GL_FUNCTION_END_NO_RETURN(void, FramebufferTexture2D, target, attachment, textarget, texture, level) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index fe0999ef..34d23700 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -85,6 +85,20 @@ namespace MobileGL::MG_Pipe { return inputs.m_currentVertexAttribute; } +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (ra, CONTRACT-P5E §2.3). THE FOUR O-CLASS ROWS, AND ONLY THEY: these are the only + // members of this block that own a frontend object rather than point into one, so they + // are the only ones through which the apply thread can become a last owner. The raw + // pointer rows (the binding-slot bases) are borrowed from a live GLContext and own + // nothing, so releasing them would buy no lifetime and lose the monolith's arms. + static void ReleaseObjectPins(PipeInputs& inputs) { + inputs.m_boundVertexArray.reset(); + inputs.m_programForDispatch.reset(); + inputs.m_programForDraw.reset(); + inputs.m_transformFeedbackProgram.reset(); + } +#endif + static void CopyField(PipeInputs& dst, GLContext& ctx, MGPipeInputField field) { using F = MGPipeInputField; using MG_State::GLState::BufferBindPointTargets; @@ -330,6 +344,64 @@ namespace MobileGL::MG_Pipe { namespace { GLContext* LiveContext() { return MG_State::pGLContext.get(); } +#if MOBILEGL_BUILD_DISAGGREGATED + // ---- P5e (ra): the fill decision (CONTRACT-P5E §3.1) ----------------------------- + // + // Is this process a run-ahead CLIENT right now? Three questions, cheapest first, and + // the last one is the latch ClientSession took at its first caps adoption - so this is + // one pointer test and one bool load on the verb path once the first two are constant. + Bool ClientRunsAhead() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; + if (MG_Remote::Client::RunsAsTheServerRole()) return false; + const MG_Remote::Client::ClientSession* session = MG_Remote::Client::ClientSession::Active(); + return session != nullptr && session->RunAheadArmed(); + } + + // THE WIRE OP A VERB BOUNDARY BECOMES, which is the join MGPipeVerbForWireOp draws in + // the other direction. Built by walking the op space once at compile time rather than + // written out, because a second hand-written table is a second thing to forget a row + // in - and the generator already refuses a verb-shaped op with no row. + constexpr MGPWireOp WireOpForVerb(MGPipeVerb verb) { + for (SizeT i = 0; i < static_cast(MGPWireOp::kOpCount); ++i) { + const auto op = static_cast(i); + if (MGPipeVerbForWireOp(op) == verb) return op; + } + return MGPWireOp::kOpCount; + } + + // CONTRACT-P5E §2.1's predicate, ASKED AT THE VALIDATE POINT - which is before the + // record exists, so it is asked of the verb and the live context rather than of the + // payload. The two halves must agree with MGPipeBarriered(op, payload, applierState), + // which is what the server computes, so each clause is the same clause: + // + // 1. the static WaitClass column - the same generated table, same op; + // 2. kCtxVerb inside an open XFB span - ctx.IsTransformFeedbackActive() here, + // the applied MGPContextValues mirror there, and set_context_values precedes the + // verb on the ring, so the two read the same value (§2.1); + // 3. a draw carrying kDrawClientArrays - NOT asked here, because under run-ahead + // such a draw never reaches a record at all: vi refuses it on this very thread in + // EmitDrawRecord's array arm (§5.1, ruling 2). Asking it here would be a second + // spelling of vi's "does any enabled attribute lack a buffer" walk, and two + // spellings of an escalation is precisely what ruling 3 replaced. + // + // AN OP WITH NO VERB ROW ANSWERS BARRIERED. A verb the join does not know is one this + // file cannot reason about, and the safe answer - fill it, wait for it - is also the + // pre-P5e answer. + Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext& ctx) { + const MGPWireOp op = WireOpForVerb(verb); + if (op == MGPWireOp::kOpCount) return true; + if (MGPipeWaitClassFor(op) != kWaitNone) return true; + if (MGPipeCallClassFor(op) == kCtxVerb && ctx.IsTransformFeedbackActive()) return true; + return false; + } + + // Whether the LAST fill actually happened, i.e. whether the rows in the block describe + // the verb in flight. Read by MGPipeNoteFrontendMutation, which refreshes one field of + // that fill: with no fill behind it there is nothing to refresh and the write would be + // the role violation §3.5 names. + Bool g_lastFillWasBarriered = true; +#endif + template const SharedPtr& NullShared() { static const SharedPtr null; @@ -546,10 +618,18 @@ namespace MobileGL::MG_Pipe { void MGPipeNoteFrontendMutation(MGPipeInputField field) { PipeInputs& inputs = gPipeInputs; #if MOBILEGL_BUILD_DISAGGREGATED + // P5e (ra, CONTRACT-P5E §3.1): THIS REFRESHES ONE FIELD OF THE LAST FILL, so with no + // fill behind it there is nothing to refresh. Under run-ahead the last verb may have + // been unbarriered, in which case the block describes an older verb the server is no + // longer being asked about and a write here would be a GL-thread touch of server-role + // memory. It returns instead - the same answer, one line earlier, as the class-mask + // test below gives for a field the verb never pushed. + if (!g_lastFillWasBarriered) return; // P5c (gt, layer 2): the single-field refresh is a client write into gPipeInputs too - - // same gate as the fill. + // same gate as the fill, and it makes the same claim the fill made: the rows it is + // touching belong to a record this thread is parked behind (or will park behind). MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( - "MGPipeNoteFrontendMutation"); + "MGPipeNoteFrontendMutation", /*isBarrieredFill=*/true); #endif auto* ctx = LiveContext(); if (ctx == nullptr) return; @@ -999,6 +1079,29 @@ namespace MobileGL::MG_Pipe { Bool MGPipeDeferDestroyAndFreeIfOnApplyThread(MGPipeKind kind, Uint64 lifetimeId) { if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; if (!MG_Remote::Client::RunsAsTheServerRole()) return false; + // P5e (ra), CONTRACT-P5E §2.7 / ruling 13. The queue stays - a BARRIERED record's apply + // may still pin a frontend object (its fill's O-class rows, XFB's targets), so the + // apply thread can still be a last owner and this is still the belt that keeps that + // death off the client's allocator. But an enqueue from an UNBARRIERED record is a + // FINDING, not a service: rule F says such an apply names no client memory at all, so + // a SharedPtr it could be the last owner of means some site is still pinning a + // frontend object across a record and the migration this phase believes it finished is + // not finished. Named once with the kind so the site is findable, and Fatal under + // strict so the lane owns the red rather than a log nobody reads. + if (!MG_Pipe::MGPipeApplierCurrentRecordIsBarriered()) { + if (MG_Config::Ipc.StrictErrors) { + MGLOG_F("MGPipe: Fatal{RoleViolation, \"deferred-destroy\"} - an UNBARRIERED " + "apply was the last owner of a frontend object of kind %u (lifetime " + "%llu). CONTRACT-P5E rule F says an unbarriered apply reads no client " + "memory, so nothing it touched should have been a SharedPtr at all", + static_cast(kind), static_cast(lifetimeId)); + std::abort(); + } + MGLOG_E_ONCE("MGPipe: an UNBARRIERED apply deferred the destruction of a frontend " + "object of kind %u - CONTRACT-P5E §2.7's finding: some apply-thread " + "site still holds a frontend SharedPtr across a record", + static_cast(kind)); + } { std::lock_guard lock(g_deferredDestroyMutex); g_deferredDestroys.push_back(MGPipeDeferredDestroy{kind, lifetimeId}); @@ -1007,6 +1110,30 @@ namespace MobileGL::MG_Pipe { return true; } + // P5e (ra), CONTRACT-P5E §2.3. Declared in PipeMutation.h, where its WHY is argued. + // + // NO GUARD ON THE CALLER'S BEHALF: ClientSession calls this only after a barriered apply + // has returned on a run-ahead session, and adding a second "is run-ahead armed" test here + // would be a copy of a decision that belongs on the other side. What this side owns is + // WHICH rows go, and that answer is ReleaseObjectPins' four. + // + // THE STAMPS ARE LEFT ALONE, deliberately. A released row reads back as null, and a + // BARRIERED verb's fill re-copies it before that verb's apply can pull it (§3.1 skips the + // fill only for UNBARRIERED records); an unbarriered apply may not read it at all, and + // §3.3's detector is what says so by name. Clearing the stamps as well would trade that + // named abort for the poison Fatal, which names the field but not the rule it broke. + void MGPipeReleaseResidualFillPins() { MGPipeFillAccess::ReleaseObjectPins(gPipeInputs); } + + // P5e (ra), CONTRACT-P5E §2.5. Declared in PipeMutation.h; see there for why it is not a + // ClientSession call at the GL entry point. + void MGPipeClientFinish() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; + if (MG_Remote::Client::RunsAsTheServerRole()) return; + if (MG_Remote::Client::ClientSession* session = MG_Remote::Client::ClientSession::Active()) { + session->Finish(); + } + } + void MGPipeDrainDeferredDestroys() { if (g_deferredDestroyCount.load(std::memory_order_acquire) == 0) return; Vector drained; @@ -1955,9 +2082,20 @@ namespace MobileGL::MG_Pipe { void MGPipeLeaveVerb() { PipeInputs& inputs = gPipeInputs; #if MOBILEGL_BUILD_DISAGGREGATED - // Same layer-2 gate as the fill: the serial bump and the verb reset below are writes - // into gPipeInputs (gt). - MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("MGPipeLeaveVerb"); + // P5e (ra, §3.1): a verb whose fill was skipped has no stamps of this thread's to + // retire, and the bump below would move a serial the SERVER's stamp owns. Leave + // returns, and the tracker's base-instance clear - which is frontend state, not block + // state - still runs at the bottom. + if (g_lastFillWasBarriered) { + // Same layer-2 gate as the fill: the serial bump and the verb reset below are + // writes into gPipeInputs (gt). + MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( + "MGPipeLeaveVerb", /*isBarrieredFill=*/true); + } + if (!g_lastFillWasBarriered) { + MGPipeTrackerInstance().ClearPendingBaseInstance(); + return; + } #endif #if MOBILEGL_PIPE_POISON // Same bump the next fill would make, without a verb to fill from: no field is @@ -2686,6 +2824,18 @@ namespace MobileGL::MG_Pipe { // Did the applier reproduce it? Byte for byte, over the attributes this call // named - anything less would be a mirror that disagrees with the frontend in a // window no gate looks at. + // + // P5e (ra, CONTRACT-P5E §3.4): NOT UNDER RUN-AHEAD. The mirror is the APPLIER's + // copy of this record, and set_vertex_attrib_defaults is a kWaitNone row - so + // under run-ahead this thread has not waited for the apply and the read races it, + // and the repair below (a CopyField straight into the block) is precisely the + // GL-thread write §3.5 refuses. The client's own authority is `resolved` and + // `g_attribDefaultLastHeader`, which is what it just published; there is nothing + // the mirror could add that the wire does not already carry. A build that ever + // needs the repair arm again has to earn it with a barriered row. +#if MOBILEGL_BUILD_DISAGGREGATED + if (ClientRunsAhead()) return sizeof(MGPVertexAttribDefaults) + header.Count * sizeof(MGPAttribValue); +#endif const auto* mirror = MGPipeFillAccess::VertexAttribDefaultsOf(gPipeInputs); Bool reproduced = true; for (SizeT i = 0; i < kAttribs && reproduced; ++i) { @@ -3004,13 +3154,20 @@ namespace MobileGL::MG_Pipe { void MGPipeValidateForVerb(MGPipeVerb verb) { PipeInputs& inputs = gPipeInputs; #if MOBILEGL_BUILD_DISAGGREGATED - // P5c (gt, CONTRACT-P5C §6 layer 2): the residual fill is THE client-side write into - // gPipeInputs, and it is legal only because it runs before the record is published - - // under an armed barrier the apply thread's in-applier flag is provably down here. If - // that ever stops being true this is the check that says so, rather than the applier - // reading a half-written fill. - MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( - "MGPipeValidateForVerb"); + // ---- P5e (ra), CONTRACT-P5E §3: WHO OWNS gPipeInputs FOR THIS VERB ---------------- + // + // Under run-ahead the block is SERVER-ROLE MEMORY for an unbarriered record: the + // client does not fill it, does not stamp it and does not withdraw the server's stamp, + // because it will not be parked while the apply runs and every one of those writes + // would race the applier's own reads. What still runs, unchanged, is the tracker walk + // and the emitters (steps 2 and 3): those PRODUCE RECORDS, which is the whole of what + // an unbarriered verb is allowed to hand the server. + // + // `fillOwed` is deliberately a separate name from `barriered`, and that is what makes + // the red-once one line: setting it to `true` restores the old behaviour (a fill for + // every verb) and the guard below then fires Fatal{RoleViolation, "gPipeInputs"} on + // the first unbarriered verb, by name. + const Bool runAhead = ClientRunsAhead(); #endif ParsePoisonOmissionKnob(); #if MOBILEGL_PIPE_VERIFY @@ -3023,24 +3180,64 @@ namespace MobileGL::MG_Pipe { "(configure with -DMOBILEGL_PIPE_VERIFY=ON)"); } #endif + auto* ctx = LiveContext(); + // THE IDENTITY / LIVENESS PAIR IS WRITTEN ON EVERY VERB, run-ahead or not, and that is + // a NAMED DEVIATION from CONTRACT-P5E §3.1's list (see the report): `m_live` and + // `m_contextIdentity` are not FIELDS of the residual model - no FieldOwnership.def row + // owns them, no applier record carries them, and no server stamp can answer them - + // they are "does this process still have a GL context", which every null-context guard + // in the backend reads and which only this thread can know. Two stores, no freshness + // and no stamp, so an unbarriered apply reading them reads a fact about the client's + // process rather than a value the wire owes it. + MGPipeFillAccess::SetIdentity(inputs, ctx); +#if MOBILEGL_BUILD_DISAGGREGATED + // §2.1's predicate, client-side (see ClientVerbIsBarriered). A null context makes the + // verb a no-op below anyway, so it answers barriered and the old path runs whole. + const Bool barriered = !runAhead || ctx == nullptr || ClientVerbIsBarriered(verb, *ctx); + // THE ONE LINE THE RED-ONCE FLIPS: `= true` here is "fill for every verb", the + // pre-P5e behaviour, and it turns the guard below into the abort §3.5 names. + const Bool fillOwed = barriered; + g_lastFillWasBarriered = fillOwed; + if (fillOwed) { + // P5c (gt, CONTRACT-P5C §6 layer 2) / P5e §3.5: the residual fill is THE + // client-side write into gPipeInputs. Under lockstep it is legal because it runs + // before the record is published - the apply thread's in-applier flag is provably + // down here. Under run-ahead it is legal because this record is BARRIERED: this + // thread is about to park behind it. The second argument is which of the two + // claims the caller is making, and a fill that made the second one falsely is the + // named abort. + MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( + "MGPipeValidateForVerb", /*isBarrieredFill=*/barriered); + } +#else + constexpr Bool fillOwed = true; +#endif + if (fillOwed) { #if MOBILEGL_PIPE_POISON - MGPipeFilledState& filled = MGPipeFillAccess::Filled(inputs); - // Starts at 1: FilledGen == 0 is "never filled", and MGPipeInputFieldIsFresh refuses - // it on both branches, so a read before this first bump is - // Fatal{UnmigratedPipeInput, "@"} rather than default storage. - ++filled.CurrentVerbSerial; + // Starts at 1: FilledGen == 0 is "never filled", and MGPipeInputFieldIsFresh + // refuses it on both branches, so a read before this first bump is + // Fatal{UnmigratedPipeInput, "@"} rather than default storage. + ++MGPipeFillAccess::Filled(inputs).CurrentVerbSerial; #endif #if MOBILEGL_BUILD_DISAGGREGATED - // The client is filling, so whatever the server stamped at its last verb boundary is - // withdrawn: the stamps below are the CLIENT's again and a stale read is a defect, not - // a residual pull. Disarming here rather than at the end of the applier's work is what - // makes the arming flag say "the current stamps are the server's" no matter which of - // the two roles ran last. - MGPipeServerClearVerbBoundary(); + // The client is filling, so whatever the server stamped at its last verb boundary + // is withdrawn: the stamps below are the CLIENT's again and a stale read is a + // defect, not a residual pull. Disarming here rather than at the end of the + // applier's work is what makes the arming flag say "the current stamps are the + // server's" no matter which of the two roles ran last. + // + // P5e (§3.2): under run-ahead this runs only inside a barriered fill, which is the + // whole of the E note's hazard (b.4) - a GL thread withdrawing the SERVER's stamp + // while the apply thread is inside a record that depends on it. For an unbarriered + // verb the stamp is not touched at all, and the applier's own LeaveApplier is then + // its only writer. + MGPipeServerClearVerbBoundary(); +#endif + MGPipeFillAccess::SetVerb(inputs, verb); + } +#if MOBILEGL_PIPE_POISON + MGPipeFilledState& filled = MGPipeFillAccess::Filled(inputs); #endif - MGPipeFillAccess::SetVerb(inputs, verb); - auto* ctx = LiveContext(); - MGPipeFillAccess::SetIdentity(inputs, ctx); if (ctx == nullptr) { // The pending base instance belongs to THIS verb, and this exit skips step 3's // clear, so it has to make the same promise here: a base-instanced draw with no @@ -3286,12 +3483,20 @@ namespace MobileGL::MG_Pipe { // does read. The two halves of P5c rv's gate still read the ONE `contextValuesWireLive` // computed at the top of this function - it is the memo's key AND its argument - so they // cannot disagree about who supplies the eight value-class fields. + // + // P5e (ra, §3.1): AND IT DOES NOT RUN AT ALL FOR AN UNBARRIERED RECORD. Every write + // below - the CopyField and the FilledGen stamp alike - is a GL-thread write into a + // block the apply thread is about to read without this thread being parked, which is + // exactly what rule F forbids. The server does not go without an answer: it stamps its + // own boundary and §3.3's detector turns any field it still needs from here into + // Fatal{UnmigratedPipeInput, "@"} by name, which is what makes the strict + // lane a gate rather than a count. const Bool applierDerives = ApplierDerivesRenderStateFields(); // BY VALUE, NOT BY REFERENCE: the memo's storage is rebuilt in place when the key moves, // and the walk below holds this across 63 iterations. const MGPipeFieldMask supplied = SuppliedFieldMask(pushMask, applierDerives, contextValuesWireLive); - for (SizeT i = 0; i < kMGPipeInputFieldCount; ++i) { + for (SizeT i = 0; fillOwed && i < kMGPipeInputFieldCount; ++i) { const auto field = static_cast(i); if (!MGPipeFieldMaskHas(mask, field)) continue; #if MOBILEGL_PIPE_POISON diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index d367a4bf..c9f6fb25 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -231,8 +231,38 @@ namespace MobileGL::MG_Pipe { // BeforeReadOnlyVerb) through MGPipeDrainDeferredDestroys. A delayed free is safe: // slots are plentiful, reuse is delayed rather than corrupted, and the replay runs the // helper itself, so every helper's own three-step order is untouched. + // P5e (ra), CONTRACT-P5E §2.7 / ruling 13 (ID-91): THE QUEUE STAYS LIVE, AND AN + // UNBARRIERED ENQUEUE IS A FINDING. After the family packages land, an unbarriered apply + // holds no frontend SharedPtr at all - every object is resolved from a handle - so the + // only apply thread that can still be a last owner is one inside a BARRIERED record (a + // fill's O-class rows, XFB's pinned targets). An enqueue from an unbarriered record + // therefore means some site still pins a frontend object across a record, which is a + // migration this phase believes it finished: it logs once with the kind, and is Fatal + // under MOBILEGL_IPC_STRICT_ERRORS so the strict lane owns the red. The queue itself is + // NOT removed - it is the belt that keeps the barriered case from corrupting the + // allocator while the finding is being read. Bool MGPipeDeferDestroyAndFreeIfOnApplyThread(MGPipeKind kind, Uint64 lifetimeId); void MGPipeDrainDeferredDestroys(); + + // P5e (ra), CONTRACT-P5E §2.3: drop the four O-class SharedPtr rows the residual fill left + // in gPipeInputs (the bound VAO and the three programs, PipeInputs.h's O block). Called by + // ClientSession on the GL thread once a BARRIERED apply has returned - the one instant + // under run-ahead at which the applier is provably idle and this block has no other + // reader. Under lockstep nothing calls it: the next verb's fill overwrites those rows + // within a verb, and dropping them early would only cost a re-pin. + // + // It is HERE rather than in ClientSession because the rows are PipeInputs' private storage + // and MGPipeFillAccess - the client-side door to it - lives in MG_Impl (PipeFill.cpp). + // MG_Remote says when; MG_Impl says what. + void MGPipeReleaseResidualFillPins(); + + // P5e (ra), CONTRACT-P5E §2.5: glFinish's whole body under a transport. Declared here + // rather than on ClientSession so that MG_Impl's GL entry points - which are BELOW + // MG_Remote - can reach it without learning that a session exists; the implementation in + // PipeFill.cpp is one forward to ClientSession::Finish. A no-op with no live session, on a + // monolith transport, and on a lockstep one (there the client waited out every command it + // issued before it could reach the call). + void MGPipeClientFinish(); #endif void MGPipeEmitResourceCreate(MG_State::GLState::BufferObject& buffer); diff --git a/MobileGL/MG_Pipe/PipeRoute.h b/MobileGL/MG_Pipe/PipeRoute.h index 7e794bf9..63f4b0d5 100644 --- a/MobileGL/MG_Pipe/PipeRoute.h +++ b/MobileGL/MG_Pipe/PipeRoute.h @@ -267,12 +267,41 @@ namespace MobileGL::MG_Pipe { } // ---- resources --------------------------------------------------------------------- + + // P5e (ra), CONTRACT-P5E §2.5 / ruling 15 (ID-93): DOES THIS SUB-DATA RECORD WANT ITS + // ANSWER? One predicate, two callers - this route and the wire emit table - because the + // route cannot pass a flag through the thunk (the call table's signature is the + // catalogue's) and two hand-written copies of "is this the buffer half" is exactly the + // drift that would make the client wait for an answer the emitter told the server not to + // bother with, or read a slot that was never posted. + // + // THE BUFFER HALF DOES NOT: its Bool is discarded at the only call site there is + // (MGPipeEmitResourceSubData), and because the row carries kReplySlot the client used to + // wait for it anyway - one full round trip per 64 KB persistent-map block, whose answer + // nobody looked at. THE TEXTURE HALF DOES: DrainTextureSubData clears the level's dirty + // flag on an ACCEPTED reply, so its answer is load-bearing (D-D5; making that half + // fire-and-forget is a trailing item, not this package's). + // + // The test is MGPSubData::Target == kMGPipeResourceTargetBuffer, whole field, which is the + // invariant MGPipeTypes.h asserts beside the packer and which the applier's own + // SubDataNamesABuffer already reads. + inline Bool MGPipeSubDataWantsItsReply(const MGPSubData& record) { + return record.Target != kMGPipeResourceTargetBuffer; + } + inline Bool MGPipeRouteResourceSubData(const MGPSubData& record, const void* bytes, Uint64 byteCount, const MGPSubRegion* regions = nullptr) { + // THE SLOT IS STILL MINTED AND STILL TAKEN, even for the buffer half. The row keeps + // kReplySlot on the wire, the server still posts, and the mailbox's "every minted slot + // is taken" rule is what the next row's ReplyOverrun Fatal is looking for - so the + // half that does not WAIT still has to collect. What changes is that under run-ahead + // the emit table publishes and returns, and the answer it collects is the emitter's + // accept-by-construction rather than the server's. MGPReplySlot reply = MGPipeMintReplySlot(); MGP_ResourceSubData(&record, bytes, byteCount, regions, record.RegionCount, &reply); - return MGPipeTakeReplyBool(reply, "resource_subdata"); + const Bool accepted = MGPipeTakeReplyBool(reply, "resource_subdata"); + return MGPipeSubDataWantsItsReply(record) ? accepted : true; } inline void MGPipeRouteBufferSubDataResident(const MGPSubData& record, const void* bytes, Uint64 byteCount) { diff --git a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp index bf1193e0..c7a739aa 100644 --- a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp +++ b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp @@ -172,6 +172,27 @@ namespace MobileGL::MG_Remote::Client { return CapsMirrorInstance().Backend(); } + namespace { + // P5e (ra), CONTRACT-P5E §2.5: THE WAIT BEFORE EVERY `Server*` EGL FORWARDER. + // + // These calls do not travel on SEG_CMD. They go through ServerLoop's control mailbox, + // which is pumped BETWEEN drain batches (ServerLoop.cpp's PumpControlRequest, above + // DrainRing) - so a make-current, a surface creation or a resize can land between two + // records the client published and never waited for. Under lockstep that was + // impossible: the client had waited out every record it issued before it could reach + // this line. Under run-ahead it is one queued frame wide, and a context switch applied + // in the middle of another context's draws is not a wrong pixel, it is a wrong + // everything. + // + // ServerSwapEGLBuffers is deliberately NOT on this list, and its own virtual says why: + // present IS the swap and it travels as a record, in order, with its own credit. + void WaitForApplyBeforeEglForwarder(const char* forwarder) { + if (ClientSession* session = ClientSession::Active()) { + session->WaitForApplyToCatchUp(forwarder); + } + } + } // namespace + // ---- the nine EGL lifecycle virtuals --------------------------------------------------- // // FORWARD FIRST, THEN RUN THE BASE. The server has to own the context before the client's @@ -180,6 +201,7 @@ namespace MobileGL::MG_Remote::Client { // InitCapabilities has run. Bool BackendObject_Remote::InitializeEGLDisplay(EGLDisplay dpy, EGLint* major, EGLint* minor) { + WaitForApplyBeforeEglForwarder("InitializeEGLDisplay"); if (!Server::ServerInitializeEGLDisplay(dpy, major, minor)) return false; return MG_Backend::BackendObject::InitializeEGLDisplay(dpy, major, minor); } @@ -188,6 +210,7 @@ namespace MobileGL::MG_Remote::Client { const MG_Backend::WindowHandle& handle) { // The handle first: the server's backend has to know which window it is about to make // a surface for, and ServerSetWindowHandle is the only way to tell it. + WaitForApplyBeforeEglForwarder("CreateEGLWindowSurface"); Server::ServerSetWindowHandle(handle); if (!Server::ServerCreateEGLWindowSurface(surface, handle)) return false; // The server's surface init published the default framebuffer's shape as a @@ -202,6 +225,7 @@ namespace MobileGL::MG_Remote::Client { } Bool BackendObject_Remote::ResizeEGLWindowSurface(EGLSurface surface, Uint32 width, Uint32 height) { + WaitForApplyBeforeEglForwarder("ResizeEGLWindowSurface"); if (!Server::ServerResizeEGLWindowSurface(surface, width, height)) return false; // A resize re-creates the server's swapchain, which re-posts the surface-changed // event - same drain, same reason as CreateEGLWindowSurface. @@ -210,6 +234,7 @@ namespace MobileGL::MG_Remote::Client { } Bool BackendObject_Remote::CreateEGLPbufferSurface(EGLSurface surface, EGLint width, EGLint height) { + WaitForApplyBeforeEglForwarder("CreateEGLPbufferSurface"); if (!Server::ServerCreateEGLPbufferSurface(surface, width, height)) return false; // Same drain as the window surface: InitPbufferSurface publishes the default // framebuffer's depth/stencil format on SEG_EVENT from inside this very RPC. @@ -223,6 +248,7 @@ namespace MobileGL::MG_Remote::Client { // class latches "the surface is initialised" and calls InitCapabilities, because // InitCapabilities' answer comes from a snapshot the server can only publish once its // own InitCapabilities has run - and ServerMakeEGLCurrent is what publishes it. + WaitForApplyBeforeEglForwarder("MakeEGLCurrent"); if (!Server::ServerMakeEGLCurrent(dpy, draw, read, ctx)) return false; if (!MG_Backend::BackendObject::MakeEGLCurrent(dpy, draw, read, ctx)) return false; diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index bd749387..b1332132 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -139,7 +139,12 @@ namespace MobileGL::MG_Remote::Client { return timeoutNs / 1000000 + (timeoutNs % 1000000 != 0) + kBarrierTimeoutMs; } + // Defined below; declared here because the applied wait has to be able to drain + // SEG_EVENT while it is parked (P5e §2.6). + Uint32 DrainEventRing(Transport::EventRingConsumer& events); + Transport::SessionWait WaitForAppliedBudget(Transport::SessionProducer& producer, + Transport::EventRingConsumer& events, Uint64 seq, Uint64 remainingMs) { // The transport takes Uint32 milliseconds with UINT32_MAX meaning forever. // Keep even the largest GL timeout finite, using bounded chunks and preserving @@ -148,7 +153,23 @@ namespace MobileGL::MG_Remote::Client { for (;;) { const Uint32 chunkMs = remainingMs > kMaxFiniteWaitMs ? kMaxFiniteWaitMs : static_cast(remainingMs); - const auto wait = producer.WaitForApplied(seq, chunkMs); + // P5e (ra, CONTRACT-P5E §2.6): THE PARK BREAKS ON TWO THINGS. A server that + // ran out of SEG_EVENT stops producing and therefore stops applying, so a + // waiter that could only be woken by appliedSeq would wait for a watermark + // nothing is going to move - and it is holding the only drain there is. The + // second exit is that case: drain, clear the latch, ring the server, park + // again. `remainingMs` is deliberately NOT charged for it; a wake that did + // the queue's work is not the caller's budget being spent. + const auto wait = producer.WaitForAppliedOrEventBacklog(seq, chunkMs); + if (wait == Transport::SessionWait::Reached) { + Transport::RingControl* control = producer.Control(); + if (control == nullptr || + Transport::Watermark::Reached(control->appliedSeq, seq)) { + return wait; + } + DrainEventRing(events); + continue; + } if (wait != Transport::SessionWait::TimedOut || remainingMs <= chunkMs) return wait; remainingMs -= chunkMs; } @@ -592,6 +613,9 @@ namespace MobileGL::MG_Remote::Client { m_events = Transport::EventRingConsumer(m_shm.EventControl(), control, m_shm.EventRingBase(), m_shm.EventRingCapacity(), m_shm.EventSegmentBase()); + // P5e (ra, §2.6): the drain's other half. A server parked on a full SEG_EVENT is woken + // by the drain that emptied it, not by the next thing this client happens to publish. + m_events.SetServerDoorbell(&m_clientTransport->PeerDoorbell()); if (!m_replies.Valid() || !m_events.Valid()) { Stop(); return MOBILEGL_ERR_INVALID_ARGUMENT; @@ -629,6 +653,10 @@ namespace MobileGL::MG_Remote::Client { // half-wired shape this session exists not to have. m_encoder = Wire::PipeWireEncoder(control, &m_cmd, nullptr, &m_segments); m_encoder.SetStageRetirementDoorbell(m_producer.SelfDoorbell()); + // P5e (ra, §2.6): the stage bell is a client wait and every client wait drains. The + // encoder owns the park; the session owns SEG_EVENT, so the session lends it a drain. + m_encoder.SetStageWaitHook( + +[](void* self) { DrainEventRing(static_cast(self)->Events()); }, this); // ---- 7. the first CapsSnapshot, if the server had a backend to publish one from. // ONE DRAIN, ONE ADOPTER (c1): PumpControlPlane below is the only thing in the client @@ -814,7 +842,8 @@ namespace MobileGL::MG_Remote::Client { Uint64 ClientSession::EmitAndWaitTails(MG_Pipe::MGPWireOp op, const void* payload, Uint64 payloadBytes, const Wire::WireTail* tails, Uint32 tailCount, void* replyOut, Uint64 replyBytes, - Int32* statusOut, Uint64* replySizeOut) { + Int32* statusOut, Uint64* replySizeOut, + Bool wantReply) { Uint64 varTailBytes = 0; for (Uint32 i = 0; i < tailCount; ++i) varTailBytes += tails[i].Size; if (statusOut != nullptr) *statusOut = Wire::ReplySink::kStatusError; @@ -863,6 +892,12 @@ namespace MobileGL::MG_Remote::Client { wait == Transport::SessionWait::ShutDown ? "shutdown" : "timeout"); std::abort(); } + // P5e (ra, §2.6): EVERY WAIT EXIT DRAINS. The cmd-space wait is one of the two + // safety nets under run-ahead (the stage bell is the other), and a client parked + // here without draining is the other half of the flow-control deadlock: the server + // stops applying on a full event ring, so retiredSeq stops moving, so this wait + // never completes. One drain on the way out closes it. + DrainEventRing(m_events); seq = m_encoder.EncodeRecord(op, payload, payloadBytes, tails, tailCount); if (seq == Wire::kInvalidSeq) { MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_CMD\"} - %s refused after sufficient " @@ -880,8 +915,63 @@ namespace MobileGL::MG_Remote::Client { // (R-16 / ID-31) - fourteen rows now, because the four Bool acceptance entry points // gained the flag. Asking the catalogue rather than the caller is what stops a caller // that forgot to pass a buffer from silently turning an answer into a guess. - const Bool ownsReplySlot = + // + // P5e (ra) ADDS ONE CALLER-SIDE CONJUNCT AND NO SECOND SOURCE OF TRUTH: `wantReply`. + // The catalogue still says which rows CARRY a slot; a caller that passes false says it + // will not read this one (resource_subdata's buffer half, §2.5 - the only such caller). + // The server posts either way, so the wire is unchanged and the row keeps its flag. + const Bool rowCarriesReplySlot = (MG_Pipe::MGPipeCallFlagsFor(op) & static_cast(MG_Pipe::kReplySlot)) != 0; + const Bool ownsReplySlot = rowCarriesReplySlot && wantReply; + + // ---- THE P5e WAIT RULE (CONTRACT-P5E §2.3) --------------------------------------- + // + // IT STANDS BEFORE THE BATCH SKIP AND REPLACES IT WHOLE, rather than adding a clause to + // it. The batch skip is a rule about CALL CLASSES ("a set/CSO/object record carries its + // whole value in the payload"); the wait class is a rule about WHAT AN APPLY READS, and + // it is generated from the same PipeCalls.def row the class comes from + // (MGPipeWaitClassFor, c0e). Two rules layered would mean a row could be skipped by the + // class rule and waited by the column, and the column is the one the SERVER's + // MGPipeBarriered reads - so the two halves of the wire would disagree about whether + // the client is parked behind this record, which is the single fact §4.4's exemptions + // and §3.3's detector both rest on. + // + // WITH RUN-AHEAD DISARMED NOTHING BELOW RUNS and the file's original shape continues - + // Magma, a monolith transport, MOBILEGL_IPC_RUN_AHEAD=0 and every build before the + // integration commit take the old path byte for byte, which is what makes RUN_AHEAD=0 + // a pure wait-rule A/B on identical server code (§5.8). + if (RunAheadArmed() && !ownsReplySlot) { + MG_Pipe::MGPipeWaitClass waitClass = MG_Pipe::MGPipeWaitClassFor(op); + // §2.5's named relaxation, and the ONLY place a static column answer is overridden: + // a kWaitReply row whose caller does not want the answer has nothing to wait FOR. + // It is spelled here rather than in the column because the column is per-op and + // this is per-call - the texture half of the same row still waits. + if (!wantReply && waitClass == MG_Pipe::kWaitReply) waitClass = MG_Pipe::kWaitNone; + switch (waitClass) { + case MG_Pipe::kWaitNone: + // Published and gone. Rule F is what makes this legal: the apply of an + // unbarriered record names no client memory at all (§3, §4, §5). + return seq; + case MG_Pipe::kWaitPresent: + // The credit wait already ran, in EmitPresent, BEFORE the encode (§2.4). There + // is nothing left to do here and waiting would turn the credit into a barrier. + return seq; + case MG_Pipe::kWaitApplied: + // §2.2's barriered rows: applier_reset, generate_mipmap (until tx2 lands its + // VerbMipRes arm), set_storage_block_binding, the five XFB span verbs and the + // CopyTex endpoint. Each still pulls a row or probes a registry, and the + // client's wait is exactly what keeps P5C's semantics legal for it (ruling 4). + // Falls through to the wait below. + break; + case MG_Pipe::kWaitReply: + case MG_Pipe::kWaitClassCount: + // Unreachable: `ownsReplySlot` is false here, and gen_pipe refuses a kWaitReply + // row without a slot and a kReplySlot row that does not wait (c0e's three + // negative controls). Treated as a wait rather than as a skip, because the + // wrong answer to "does this hang" is the recoverable one. + break; + } + } // MOBILEGL_IPC_BATCH_WAITS (default 1): a value-class record is published WITHOUT // waiting for its own apply. The barrier's one load-bearing reader is the backend @@ -912,7 +1002,13 @@ namespace MobileGL::MG_Remote::Client { // (set_texture_params, resource_subdata, resource_readback and get_texture_image // own reply slots and wait anyway; the resource_* transfers and object_death resolve // by handle). - if (!ownsReplySlot && MG_Config::Ipc.BatchWaits != 0 && + // + // AND IT IS DISARMED WHOLE UNDER RUN-AHEAD (P5e §2.3): the wait class above is the + // rule there, and a second skip rule reading a different column is how the two halves + // of the wire end up disagreeing about whether the client is parked. Written as a + // conjunct here rather than left to "the classes happen not to overlap", because the + // classes overlapping is one PipeCalls.def row away at any time. + if (!ownsReplySlot && !RunAheadArmed() && MG_Config::Ipc.BatchWaits != 0 && op != MG_Pipe::MGPWireOp::GenerateMipmap) { const MG_Pipe::MGPipeCallClass callClass = MG_Pipe::MGPipeCallClassFor(op); if (callClass == MG_Pipe::kCtxState || callClass == MG_Pipe::kCtxCso || @@ -931,7 +1027,8 @@ namespace MobileGL::MG_Remote::Client { const BarrierWaitScope waiting; const Uint64 waitBudgetMs = AppliedWaitBudgetMs(op, payload); - const Transport::SessionWait wait = WaitForAppliedBudget(m_producer, seq, waitBudgetMs); + const Transport::SessionWait wait = + WaitForAppliedBudget(m_producer, m_events, seq, waitBudgetMs); if (wait == Transport::SessionWait::ShutDown) { // The doorbell died: the server went away. The only thing that returns from a // kWaitForever park, and therefore the only way a client blocked in the barrier @@ -965,7 +1062,17 @@ namespace MobileGL::MG_Remote::Client { // clears when the writeback lands, and the writeback lands on this ring. DrainEventRing(m_events); - if (!ownsReplySlot) return seq; + if (!ownsReplySlot) { + // P5e (ra, §2.3). A barriered record's apply has returned and this thread is the + // only runnable one, so the four O-class SharedPtr rows the last fill left in + // gPipeInputs can go. Under lockstep they were harmless - the next verb's fill + // overwrote them - but under run-ahead the next fill may be a frame away, and a + // frontend VAO or program whose LAST owner is that block is one the apply thread + // can end up destroying (§2.7's deferred-destroy hazard). Released here rather + // than at the fill, because here is where the applier is provably idle. + if (RunAheadArmed()) ReleaseFillPins(); + return seq; + } // THE SAME WAIT, NOT A SECOND ONE. appliedSeq >= seq already means the server wrote // this record's answer, because it writes the slot before it advances the watermark. @@ -1000,6 +1107,166 @@ namespace MobileGL::MG_Remote::Client { Bool ClientSession::BarrierArmed() const { return m_barrierArmed; } + // ---- P5e (ra): the run-ahead latch and the waits it leaves behind --------------------- + + Bool ClientSession::RunAheadArmed() const { return m_runAheadArmed; } + // THE PRODUCER'S NUMBER, NOT A SECOND ONE. SessionProducer records it inside + // PublishAndNotify, which is the only place a record becomes published at all; a copy kept + // here would be a second thing to forget to write on a path that publishes without waiting + // - which, after P5e, is most of them. + Uint64 ClientSession::LastPublishedSeq() const { return m_producer.LastPublishedSeq(); } + Uint64 ClientSession::PresentCreditWaits() const { return m_presentCreditWaits; } + + void ClientSession::LatchRunAheadFromCaps() { +#if MOBILEGL_BUILD_DISAGGREGATED + // THE CONJUNCTION, IN THE CONTRACT'S OWN ORDER (§1). The caps read is last because it + // is the only one that costs a mirror load, and because it is the only one whose answer + // can change: the other two are process configuration. + const Bool armed = m_barrierArmed && MG_Config::Ipc.RunAhead != 0 && + Caps().HasCap(MG_Pipe::kCapRunAheadApply); + if (!m_runAheadLatched) { + m_runAheadLatched = true; + m_runAheadArmed = armed; + if (armed) { + MGLOG_I("MG_Remote client: run-ahead ARMED - the server publishes " + "kCapRunAheadApply, so an unbarriered record is published and not " + "waited for (CONTRACT-P5E §2.3), paced by a present credit of %u", + MG_Config::Ipc.PresentCredit); + } else if (MG_Config::Ipc.RunAhead != 0 && m_barrierArmed) { + // THE KNOB IS NOT A SWITCH ON ITS OWN, and an operator who set it has to be + // told that, or an A/B arm that measured nothing looks like an A/B arm that + // measured no difference (Config.h's own argument for parsing it everywhere). + MGLOG_W_ONCE("MG_Remote client: run-ahead requested, server does not publish " + "kCapRunAheadApply - running lockstep. This is Magma always, and " + "Espryt until the P5e integration commit flips " + "kMGPipeP5eRunAheadReady"); + } + return; + } + // A LATER SNAPSHOT MAY ONLY TURN IT OFF. Promotion would mean the records already + // published without a wait were published against a rule this server never agreed to, + // and nothing can un-publish them; demotion only adds waits, so it is safe and is + // exactly what a server that lost its run-ahead backend needs. + if (!armed && m_runAheadArmed) { + m_runAheadArmed = false; + MGLOG_W("MG_Remote client: run-ahead DISARMED by a later caps snapshot - every " + "record from here waits as it did before P5e"); + } +#endif + } + + void ClientSession::ReleaseFillPins() { +#if MOBILEGL_BUILD_DISAGGREGATED + // The block's own door: PipeInputs' O-class rows are private and MG_Remote has no + // business knowing which four they are. MG_Impl owns the fill, so MG_Impl owns the + // un-fill (PipeFill.cpp), and this side only says WHEN. + MG_Pipe::MGPipeReleaseResidualFillPins(); +#endif + } + + void ClientSession::WaitForApplyToCatchUp(const char* why) { +#if MOBILEGL_BUILD_DISAGGREGATED + // A NO-OP WITHOUT RUN-AHEAD, and that is not an optimisation: under lockstep the + // client already waited for every record it published, so appliedSeq is at + // LastPublishedSeq by construction and a second wait would only add a park on a + // watermark that is already reached. + if (!m_runAheadArmed || !m_started) return; + const Uint64 target = m_producer.LastPublishedSeq(); + if (target == 0) return; + const BarrierWaitScope waiting; + const Transport::SessionWait wait = + WaitForAppliedBudget(m_producer, m_events, target, kBarrierTimeoutMs); + if (wait == Transport::SessionWait::ShutDown) { + // The doorbell died: teardown, not a fault - the same answer EmitAndWaitTails + // gives, and for the same reason. + MGLOG_E("MG_Remote client: the forced wait for %s woke on a dead doorbell; the " + "server is gone", why); + return; + } + if (wait != Transport::SessionWait::Reached) { + MGLOG_F("MGPipe: Fatal{BarrierTimeout, \"%s\"} - a forced run-ahead wait did not " + "reach appliedSeq %llu within %u ms. The forced waits of CONTRACT-P5E §2.5 " + "are the points at which the client must be in step again, so a wedge here " + "is a wedge in the queue and not a slow frame", + why, static_cast(target), kBarrierTimeoutMs); + std::abort(); + } + // EVERY WAIT EXIT DRAINS (§2.6). It is what the flow-control deadlock argument rests + // on: the server blocks only on a full event ring, the client blocks only on + // watermarks the server advances, and every client wait drains - so at most one side + // is parked at any instant. + DrainEventRing(m_events); + ReleaseFillPins(); +#else + (void)why; +#endif + } + + void ClientSession::Finish() { + // glFinish. Nothing goes on the wire - Flush and Finish have no record (ARCHITECTURE + // §11) - and under lockstep there was nothing to do, because the client had already + // waited out every command it issued. Under run-ahead "the commands issued so far have + // completed" is a promise the queue can break, so this is where it is kept. + WaitForApplyToCatchUp("glFinish"); + } + + Uint64 ClientSession::AcquirePresentCredit() { +#if MOBILEGL_BUILD_DISAGGREGATED + // MINTED CLIENT-SIDE AND 1-BASED (§1). Before P5e the client sent 0 and the server + // stamped its own frame count, which was honest while nothing paced on it; a credit + // needs an id space only the payer can advance, and that is this one. + const Uint64 serial = m_presentsSent + 1; + if (!m_runAheadArmed) { + // Lockstep: the present record's own barrier IS the pacing, one frame deep by + // construction. The serial is still minted so that both arms carry the same field + // and the server's ReturnPresentCredit is fed on both - an A/B whose two arms + // disagree about a wire field is not an A/B (Config.h's rule for the knobs). + m_presentsSent = serial; + return serial; + } + const Uint32 credit = MG_Config::Ipc.PresentCredit; + if (m_presentsSent >= credit) { + // Wait for swap number (serial - credit) to have come back. With credit 1 that is + // the previous frame: the client publishes frame N+1's records while the server + // applies and swaps frame N, and blocks here until swap N returned - one frame of + // overlap and at most one frame of added latency (ruling 4 / ID-92). + const Uint64 awaited = serial - credit; + ++m_presentCreditWaits; + const BarrierWaitScope waiting; + Transport::SessionWait wait = Transport::SessionWait::TimedOut; + for (;;) { + // §2.6 again: the credit park breaks on a full SEG_EVENT too, for the same + // reason the applied park does - a server that stopped producing stopped + // swapping, and this waiter holds the only drain. + wait = m_producer.WaitForPresentAckOrEventBacklog(awaited, kBarrierTimeoutMs); + if (wait != Transport::SessionWait::Reached) break; + Transport::RingControl* control = m_producer.Control(); + if (control == nullptr || + Transport::Watermark::Reached(control->presentAckSerial, awaited)) { + break; + } + DrainEventRing(m_events); + } + if (wait == Transport::SessionWait::TimedOut) { + MGLOG_F("MGPipe: Fatal{PresentCreditTimeout} - present %llu waited %u ms for " + "swap %llu to come back with a credit of %u. The credit, never the " + "ring's bytes, is what paces a run-ahead client (CONTRACT-P5E §2.4), " + "so a credit that never returns is a server that stopped swapping", + static_cast(serial), kBarrierTimeoutMs, + static_cast(awaited), credit); + std::abort(); + } + // ShutDown is teardown and returns: the present did not happen, the same answer + // the barrier gives on a dead doorbell. + if (wait == Transport::SessionWait::Reached) DrainEventRing(m_events); + } + m_presentsSent = serial; + return serial; +#else + return ++m_presentsSent; +#endif + } + // R-1's mutual-exclusion invariant, as two probes that answer honestly. // // THE CLIENT'S FLAG IS THREAD-LOCAL AND THE SERVER'S IS NOT, and the asymmetry is the @@ -1012,7 +1279,8 @@ namespace MobileGL::MG_Remote::Client { return g_applyThreadInsideApplier.load(std::memory_order_acquire); } - void ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface) { + void ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface, + Bool isBarrieredFill) { // THE CHAIN IS A CONJUNCTION, SO ITS ORDER IS FREE - AND IT IS ORDERED CHEAPEST-FIRST // (P5d round 3, package D). Every line below is a pure predicate whose only effect is // "no violation, go home"; the Fatal fires on the AND of all of them, so reordering @@ -1028,6 +1296,28 @@ namespace MobileGL::MG_Remote::Client { // both miss do we pay for a role probe, two singleton reads, a shared atomic and the // thread_local at the end. if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; + // ---- P5e (ra), CONTRACT-P5E §3.5: THE RUN-AHEAD RULE IS A DIFFERENT RULE ----------- + // + // It is first, before the BatchWaits return, because BatchWaits' argument does not + // survive run-ahead: "the pull-verbs' own wait keeps their pulled reads fenced" is a + // statement about a client that waits at every verb, and this one does not. Under + // run-ahead the block is SERVER-ROLE memory (§3) and the single legal GL-thread write + // is the residual fill of a BARRIERED record - the one the client is about to park + // behind. Every other touch races the applier by construction, so there is no + // in-applier flag to consult and no "count it" arm: the value would be torn. + // + // This is where a fill left standing for an unbarriered record lands, by name. + if (ClientSessionInstance().RunAheadArmed()) { + if (isBarrieredFill) return; + if (Server::ServerLoop::OnApplyThread()) return; // the applier owns the block + MGLOG_F("MGPipe: Fatal{RoleViolation, \"gPipeInputs\"} - the GL thread touched " + "gPipeInputs (%s) on a RUN-AHEAD session outside a barriered fill. With " + "the client running ahead the block is the server's to read for as long " + "as an unbarriered record is in flight (CONTRACT-P5E §3), and this thread " + "has no wait that would make the touch quiescent", + surface); + std::abort(); + } // MOBILEGL_IPC_BATCH_WAITS=1 makes "the fill runs while the apply thread applies an // earlier value record" the INTENDED shape: the fill writes only fields no record // supplies (the wire-live skips are the same answer the emitters use), the applier @@ -1090,6 +1380,14 @@ namespace MobileGL::MG_Remote::Client { } if (AdoptCapsSnapshot(envelope->msg_as_CapsSnapshot())) ++adopted; } + // P5e (ra, §1): the run-ahead latch rides R-12's invalidation edge, because "the first + // caps adoption after Start" and "the one place a CapsSnapshot becomes a mirror + // generation" are the same instant, and a latch taken anywhere else would be reading a + // mirror whose generation nobody had adopted yet. Called even when `adopted` is 0: the + // first call in Start() is what takes the latch, and a pump that adopted nothing still + // has to leave it taken (a handshake with no snapshot latches "not armed", which is + // the honest answer for a server that never said what it consumes). + if (adopted != 0 || !m_runAheadLatched) LatchRunAheadFromCaps(); return adopted; } @@ -1167,9 +1465,9 @@ namespace MobileGL::MG_Remote::Client { MGLOG_I("MG_Remote client: wire ledger: maxrec=%llu maxrecop=%s cap=%llu (%llu.%llu%% of " "RingProducer::MaxRecordBytes, half of a %llu byte SEG_CMD) cmdbytes=%llu " "ringwraps=%llu ringpads=%llu " - "ringwaits=%llu emitseq=%llu cliwait=%llu clipark=%llu - R-10's proof obligation, " - "R-9's producer readings and P5d round 3's client wait pair, published from the " - "session that produced them", + "ringwaits=%llu emitseq=%llu cliwait=%llu clipark=%llu credit-waits=%llu - " + "R-10's proof obligation, R-9's producer readings, P5d round 3's client wait " + "pair and P5e's present credit, published from the session that produced them", static_cast(maxRecord), m_encoder.MaxRecordOpName(), static_cast(cap), static_cast(permille / 10), @@ -1185,7 +1483,13 @@ namespace MobileGL::MG_Remote::Client { // profile runs did not set MOBILEGL_PIPE_STATS either, and "did the barrier spin // or did it park" is the question their logs have to be able to answer. static_cast(m_producer.Waits()), - static_cast(m_producer.Parks())); + static_cast(m_producer.Parks()), + // P5e (ra, §2.4). The number the device exit reads to say whether the present + // credit is what paces this workload: 0 with credit 1 means the server is + // never the frame's critical path, and a count near the frame count means it + // always is. It is on this line rather than only on the stats line for + // LogWireLedger's own reason - a trace replay never reaches a stats summary. + static_cast(m_presentCreditWaits)); } #undef MGP5_C0_STUB diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index b4ff76df..a0a0f2c7 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -112,15 +112,79 @@ namespace MobileGL::MG_Remote::Client { // or its MGPDrawIndirect block behind the range array). The one-tail form above is this // one with a single WireTail; the barrier policy lives in exactly one body. A distinct // name rather than an overload, because `nullptr, 0` would match both. + // P5e (ra, CONTRACT-P5E §2.5): `wantReply` is the ONE caller-side relaxation of the + // catalogue's reply rule, and it exists for exactly one row - resource_subdata's + // BUFFER half, whose Bool the only caller discards (PipeFill.cpp's + // MGPipeEmitResourceSubData). The row keeps kReplySlot on the wire (the server still + // posts; an unread slot is harmless, ReplySlot.h:16, 105), so nothing about the + // protocol moves - what moves is that under run-ahead this client neither waits for + // the answer nor reads it, which is what makes the 64 KB persistent-map push + // fire-and-forget instead of a hidden round trip per block. It is NOT a licence to + // skip an answer a caller uses: R-5 still forbids re-deriving one locally, and every + // other kReplySlot row passes true (the default) and waits. Uint64 EmitAndWaitTails(MG_Pipe::MGPWireOp op, const void* payload, Uint64 payloadBytes, const Wire::WireTail* tails, Uint32 tailCount, void* replyOut, Uint64 replyBytes, Int32* statusOut, - Uint64* replySizeOut = nullptr); + Uint64* replySizeOut = nullptr, Bool wantReply = true); // MOBILEGL_IPC_VERB_BARRIER. False is the R-1 negative control and is EXPECTED to be // red; it must be run once and the way it goes red recorded. Bool BarrierArmed() const; + // ---- P5e (ra): the wait rule (CONTRACT-P5E §1, §2.3, §2.4, §2.5) ------------------- + + // A CONJUNCTION OF THREE, LATCHED ONCE: `m_barrierArmed && Ipc.RunAhead && + // Caps().HasCap(kCapRunAheadApply)`. The third conjunct is the server's own statement + // that it applies an unbarriered record without reading client memory, so run-ahead is + // never something the client can turn on by itself - on Magma, and on Espryt until the + // integration commit publishes bit 10, this answers false and EmitAndWaitTails runs + // today's lockstep path byte for byte. + // + // LATCHED AT THE FIRST CAPS ADOPTION AFTER Start, AND A LATER SNAPSHOT MAY ONLY TURN IT + // OFF. A caps re-run cannot promote a lockstep server to a run-ahead one mid-frame: + // records already published without a wait would have been published against the wrong + // rule, and there is no way to un-publish them. Demotion is safe in the other + // direction - it only adds waits - so it is allowed. + Bool RunAheadArmed() const; + + // §2.3: the four O-class SharedPtr rows of gPipeInputs (PipeInputs.h:801-809 - the + // bound VAO and the three programs), dropped on the GL thread once a barriered apply + // has returned. Under lockstep the next fill overwrites them within a verb; under + // run-ahead the next fill may be a frame away, and a pinned VAO whose last owner ends + // up being the apply thread is exactly the deferred-destroy hazard §2.7 keeps a queue + // for. Releasing them at the one instant the applier is provably idle costs four + // refcount drops per barriered record and removes the pin entirely. + void ReleaseFillPins(); + + // §2.5's forced wait, as one named entry point: publish nothing, wait for the apply + // thread to reach LastPublishedSeq, drain the reverse channel. `why` names the caller + // in the timeout Fatal, because "which forced wait wedged" is the whole diagnostic. + // A no-op when run-ahead is not armed (the client is already in step by construction). + void WaitForApplyToCatchUp(const char* why); + + // glFinish. Flush and Finish stay no-ops ON THE WIRE (ARCHITECTURE §11, :415) - there + // is no record to emit - but "the GL commands issued so far have completed" cannot be + // true while a run-ahead queue still holds them, so under run-ahead glFinish becomes + // exactly WaitForApplyToCatchUp + the drain that comes with it. + void Finish(); + + // The last seq this session published, whether or not it waited for it. The forced + // waits above are defined against it. + Uint64 LastPublishedSeq() const; + + // §2.4's counter, published on the stats line as `credit-waits`: how often the client + // actually blocked for a swap to come back. Zero with a credit of 1 means the server + // is never the frame's critical path; a number close to the frame count means the + // credit is what paces this workload, which is what the device exit measures. + Uint64 PresentCreditWaits() const; + + // §2.4, called by EmitPresent immediately before it encodes: pay the credit (parking + // if this client already has `MOBILEGL_IPC_PRESENT_CREDIT` presents in flight), then + // mint and return this frame's 1-based MGPPresent::FrameSerial. The credit wait is + // BEFORE the encode on purpose: a client that encoded first would hold a SEG_CMD + // reservation across the park. + Uint64 AcquirePresentCredit(); + // R-1's invariant made checkable rather than only written down: true while this // thread is inside a barrier wait. The apply thread sets its own flag on entry to the // applier; a debug/verify build asserts the two are never both true, and that the @@ -140,7 +204,17 @@ namespace MobileGL::MG_Remote::Client { // split session is live, when the caller IS the apply thread, and when the barrier is // disarmed - MOBILEGL_IPC_VERB_BARRIER=0 is R-1's negative control and its red belongs // to Fatal{BarrierViolation}, not to this check. - static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface); + // + // P5e (ra, CONTRACT-P5E §3.5) RE-DERIVES IT UNDER RUN-AHEAD, and `isBarrieredFill` is + // what the caller says about ITSELF: "this touch is the residual fill of a record this + // thread is about to park behind". With run-ahead armed the block is server-role + // memory for every other touch, so the rule stops being "not while the applier is + // inside" - which is unanswerable when the client never waits - and becomes "not at + // all, unless you are that fill". The MOBILEGL_IPC_BATCH_WAITS early return goes with + // it: batching is about how many round trips a lockstep client pays, and it says + // nothing about a client that pays none. + static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface, + Bool isBarrieredFill = false); // ---- c1's additions --------------------------------------------------------------- @@ -250,9 +324,20 @@ namespace MobileGL::MG_Remote::Client { void LogWireLedger() const; private: + // P5e (ra): the latch, taken from PumpControlPlane after every adoption. Private + // because "when may run-ahead start" is this class's own rule and not a caller's. + void LatchRunAheadFromCaps(); + Wire::PipeWireEncoder m_encoder; Wire::SegmentTable m_segments; Bool m_barrierArmed = true; + // P5e (ra): the latch of RunAheadArmed() and the fact that it has been taken once. + Bool m_runAheadArmed = false; + Bool m_runAheadLatched = false; + // §2.4: presents published (the 1-based FrameSerial space) and how often the credit + // wait really blocked. + Uint64 m_presentsSent = 0; + Uint64 m_presentCreditWaits = 0; std::unique_ptr m_clientTransport; std::unique_ptr m_serverTransport; diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index ac5dc9e4..3cc9451f 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -1034,10 +1034,20 @@ namespace MobileGL::MG_Remote::Client { } MG_Pipe::MGPPresent record{}; - // FrameSerial 0 = "the server stamps its own". P5 has no client-side present credit - // (MOBILEGL_IPC_PRESENT_CREDIT is P6's), so a client-minted serial would be a second - // id space with no consumer. - record.FrameSerial = 0; + // P5e (ra, CONTRACT-P5E §1, §2.4). FrameSerial USED TO BE 0 - "the server stamps + // its own" - and that was honest while nothing paced on it. It is now minted here, + // 1-based, by AcquirePresentCredit, which also PAYS the credit: if this client + // already has MOBILEGL_IPC_PRESENT_CREDIT presents in flight it parks until the + // server's OnPresent returns one, and only then does it mint. + // + // THE CREDIT WAIT IS BEFORE THE ENCODE, WHICH IS NOT A DETAIL: EmitAndWait + // reserves SEG_CMD bytes as its first act, so a client that encoded and then + // parked would hold a ring reservation across a whole frame of server time. It + // also drains SEG_EVENT on its way out, like every other wait (§2.6). + // + // With run-ahead disarmed this is a counter and nothing else, and the record below + // travels exactly as it did - the present row's own barrier is the pacing there. + record.FrameSerial = session.AcquirePresentCredit(); session.EmitAndWait(MG_Pipe::MGPWireOp::Present, &record, sizeof(record), nullptr, 0, nullptr, 0, nullptr); diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 947fb771..ae11348b 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -340,15 +340,33 @@ namespace MobileGL::MG_Remote::Client { ClientSession& session = RequireSession("ResourceSubData"); MG_Pipe::MGPSubData record = *payload; record.Blob = StageOptional(session, blobBytes, blobByteCount); + // P5e (ra, CONTRACT-P5E §2.5 / ruling 15): the BUFFER half does not want its + // answer, and MGPipeSubDataWantsItsReply is the one place that decides - the same + // function the route reads, so the two halves of this call cannot disagree about + // whether a wait is owed. Under run-ahead the record is published and this thread + // returns; the persistent-map push (PersistentMapTracker's 64 KB blocks) stops + // being a hidden round trip per block, which is the single biggest wait left on + // the steady path that is not a draw. + // + // THE ANSWER IS ACCEPT-BY-CONSTRUCTION, AND THAT IS HONEST HERE AND NOWHERE ELSE: + // the only caller discards it (PipeFill.cpp's MGPipeEmitResourceSubData), so + // "accepted" is not a re-derivation of a server decision - it is the absence of a + // question. The server still posts the real answer into the slot; nothing reads + // it, which ReplySlot.h:16, 105 makes legal. A row whose acceptance a caller USES + // may never take this path - R-5 has not moved. + const Bool wantReply = MG_Pipe::MGPipeSubDataWantsItsReply(record); Int32 status = 0; - const Uint64 seq = session.EmitAndWait( - MGPWireOp::ResourceSubData, &record, sizeof(record), varTail, - static_cast(varTailCount) * sizeof(MG_Pipe::MGPSubRegion), nullptr, 0, - &status); + const Wire::WireTail tail{varTail, static_cast(varTailCount) * + sizeof(MG_Pipe::MGPSubRegion)}; + const Uint64 seq = session.EmitAndWaitTails( + MGPWireOp::ResourceSubData, &record, sizeof(record), + varTail != nullptr ? &tail : nullptr, varTail != nullptr ? 1u : 0u, nullptr, 0, + &status, nullptr, wantReply); reply->Id = seq; - MG_Pipe::MGPipePostReply(*reply, status, status == 0 ? 1u : 0u); + MG_Pipe::MGPipePostReply(*reply, wantReply ? status : 0, + (wantReply ? status == 0 : true) ? 1u : 0u); ++g_emitted; - if (status == 1) ++g_declined; + if (wantReply && status == 1) ++g_declined; } void Wire_BufferSubDataResident(const MG_Pipe::MGPSubData* payload, const void* blobBytes, diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index cfd3f735..645e9f22 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -10,6 +10,7 @@ #include "PipeApplier.h" +#include "ServerSession.h" #include "../Transport/ReplySlot.h" #include @@ -29,6 +30,20 @@ namespace MobileGL::MG_Remote::Server { + namespace { + // P5e (ra, CONTRACT-P5E §1 / §6): DOES THIS SERVER PUBLISH kCapRunAheadApply? The + // question is asked of the server's OWN CallMask and not of a build constant, because + // "the client may run ahead" is exactly what that bit says and Magma never sets it. + // A session with no CallMask yet (the bring-up window, a fixture that never called + // SetCapabilityBits) answers false: no client can have latched run-ahead against a + // snapshot that was never published. + Bool ServerPublishesRunAhead() { + const ServerSession* session = ServerSession::Active(); + if (session == nullptr || !session->CallMaskIsSet()) return false; + return (session->CallMask() & static_cast(MG_Pipe::kCapRunAheadApply)) != 0; + } + } // namespace + ReplyPool::ReplyPool(void* base, Uint64 sizeBytes, Uint32 slotCount, Uint32 slotBytes) : m_base(static_cast(base)), m_size(sizeBytes), m_slots(slotCount), m_slotBytes(slotBytes) {} @@ -288,7 +303,30 @@ namespace MobileGL::MG_Remote::Server { ++m_presents; // FrameSerial 0 means "the server stamps its own" (c1-v1 8.3): P5 has no client-side // present credit, so the client sends 0 and the frame count on this side IS the serial. + // + // P5e (ra, CONTRACT-P5E §1, §2.4): IT IS THE CLIENT'S NOW, 1-BASED AND MINTED BY THE + // PAYER. A credit can only be paced in an id space the waiter advances, and the waiter + // is the client (`WaitForPresentAck(m_presentsSent + 1 - credit)`), so a server-stamped + // serial would be the server acknowledging its own count. The 0 arm survives for a + // peer that has not been re-built - and on a server that PUBLISHES the run-ahead cap it + // is a protocol fault, because such a client is pacing on this answer and a 0 would + // acknowledge a frame nobody asked about. + if (present.FrameSerial == 0 && ServerPublishesRunAhead()) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"Present.FrameSerial\"} - a run-ahead " + "server was handed present serial 0. The client mints this 1-based and " + "waits on it for its credit (CONTRACT-P5E §2.4); returning a credit for " + "serial 0 would release a wait that is asking about frame N"); + std::abort(); + } m_lastPresentSerial = present.FrameSerial != 0 ? present.FrameSerial : m_presents; + // §2.4's other half, and the reason ServerSession::ReturnPresentCredit has had no + // production caller since v1 wrote it: ONE CREDIT PER SWAP, returned after Present() + // has returned rather than before it, because what the client is waiting for is the + // swap and not the record's apply. It advances presentAckSerial AND rings the client's + // bell - a client parked in WaitForPresentAck(kWaitForever) needs the pair. + if (ServerSession* session = ServerSession::Active()) { + session->ReturnPresentCredit(m_lastPresentSerial); + } return true; } diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 342b3e6e..30cc44ce 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -885,11 +885,19 @@ namespace MobileGL::MG_Remote::Wire { // The allocation still cannot progress after reclamation. Count this // blocked allocation once, not each watermark poll or each reclaimed mark. if (!waited) { ++m_stageReclaimWaits; waited = true; } - if (!m_stageRetirementBell->Wait(m_control->producerParked, ready, 0, 5000)) { + // P5e (ra, CONTRACT-P5E §2.6): drain BEFORE parking and again after waking. + // Before, because the ring may already be full and the server already stopped + // - in which case retiredSeq will never move and there is nothing to wake us; + // after, because the wake may have come from a server that is about to fill it + // again. The hook is null in every standalone codec, which has no session and + // therefore no reverse channel to drain. + if (m_stageWaitHook != nullptr) m_stageWaitHook(m_stageWaitSelf); + if (!ready() && !m_stageRetirementBell->Wait(m_control->producerParked, ready, 0, 5000)) { MGLOG_F("MGPipe: Fatal{RetirementWaitFailed, \"SEG_STAGE\"} producer wait " "ended before the pending allocation retired (shutdown or timeout)"); std::abort(); } + if (m_stageWaitHook != nullptr) m_stageWaitHook(m_stageWaitSelf); } ReclaimStagedBytes(); } diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index 05d42005..2bf944c3 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -340,6 +340,20 @@ namespace MobileGL::MG_Remote::Wire { // without a consumer cannot wait for retirement and retain the named refusal. void SetStageRetirementDoorbell(Transport::Doorbell* bell) { m_stageRetirementBell = bell; } + // P5e (ra, CONTRACT-P5E §2.6): THE STAGE BELL IS A CLIENT WAIT, so it has to drain the + // reverse channel like every other one. The encoder cannot drain it itself - SEG_EVENT + // and its consumers belong to the session - so the session installs a hook and this + // layer only says WHEN. Without it the flow-control deadlock is one buffer upload + // wide: the server stops applying on a full event ring, retiredSeq stops moving, and a + // client parked here for staged bytes never drains the ring that would release it. + // A raw function pointer rather than std::function: this header sits below MG_Impl and + // the call is on a path that is already about to park. + using StageWaitHook = void (*)(void* self); + void SetStageWaitHook(StageWaitHook hook, void* self) { + m_stageWaitHook = hook; + m_stageWaitSelf = self; + } + // Bytes this encoder has ever written into SEG_CMD, pad fillers included: the // producer's monotonic head cursor. It is the DENOMINATOR the wrap count only means // anything against - "0 wraps" is a defect when the run pushed more bytes than the ring @@ -383,6 +397,9 @@ namespace MobileGL::MG_Remote::Wire { Uint64 m_cmdWrapPads = 0; Uint64 m_stageReclaimWaits = 0; Transport::Doorbell* m_stageRetirementBell = nullptr; + // P5e (ra): the session's event drain, called around the stage-bell park (§2.6). + StageWaitHook m_stageWaitHook = nullptr; + void* m_stageWaitSelf = nullptr; Vector m_stageMarks; SizeT m_stageMarkFront = 0; Uint8* m_stageBase = nullptr; From bbfd4fef81261891561562fc449442be5370c866 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:22:26 -0400 Subject: [PATCH 3/5] [MG_Test, MG_Impl, MG_Pipe] (Disaggregated): P5e ra - the wait rule's cases, and the two constants are one switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fifteen cases, and every one of them arms run-ahead through the production latch (publish kCapRunAheadApply into the caps mirror before Start) rather than through a test-only setter - ID-102's lesson: a red-once whose probe is a branch beside the production path is a red-once that proves the branch. - RemoteRunAhead: the knob alone does not arm it; VERB_BARRIER=0 disarms it even with the cap bit; a credit-1 client pays at every present after the first and a credit-3 client at none of its first three; an unbarriered verb fills nothing and is allowed; a 3 MB event burst behind an unwaited sequence flow-controls instead of aborting and arrives LOSSLESS; only the texture half of resource_subdata wants its reply. - RemoteGuards: a gPipeInputs touch outside a barriered fill under run-ahead is Fatal{RoleViolation, "gPipeInputs"} by name and the barriered fill is allowed; a residual pull under an UNBARRIERED record is Fatal WITHOUT the strict knob while the same pull under a barriered one still only counts (ruling 4, in executable form); a deferred destroy from an unbarriered apply is the finding and from a barriered one is still served (ruling 13). - SessionTest: a full SEG_EVENT wakes a client parked on appliedSeq AND one parked on the present credit, without touching either watermark; the drain clears the latch and RINGS, once, and is silent when the latch was already down. - The event burst's drainer starts 50 ms late on purpose. A drainer that starts at once can keep a 256 KiB ring from ever being full, and a red-once that depends on the scheduler losing a race is not a red-once. - ClientVerbIsBarriered now takes a nullable context: the validate point asks the question before it knows whether there is anything to fill, so the answer is about the RECORD. A null context has no open XFB span, and treating it as "barriered" instead made the fill red-once unreachable in any fixture without a live GLContext. - kMGPipeP5eClientWaitRuleLanded flips WITH kMGPipeP5eRunAheadReady, in the integration commit - an amendment to ID-103's wording ("the same commit that lands the wait rule"). The argument ID-103 makes is about whether the CLIENT is parked: RunAheadArmed() needs the server's caps bit, so between ra landing and the bit being published the client still blocks after every record, and a `false` stamp there would withdraw §4.4's exemptions from probes that wait still makes safe. Two spellings, one switch. --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 17 +- MobileGL/MG_Pipe/PipeApply.h | 16 +- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 372 +++++++++++++++++++++ MobileGL/MG_Test/Wire/SessionTest.cpp | 116 +++++++ 4 files changed, 513 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 34d23700..da761dd3 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -387,11 +387,19 @@ namespace MobileGL::MG_Pipe { // AN OP WITH NO VERB ROW ANSWERS BARRIERED. A verb the join does not know is one this // file cannot reason about, and the safe answer - fill it, wait for it - is also the // pre-P5e answer. - Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext& ctx) { + // `ctx` MAY BE NULL, and that is not a convenience: the validate point asks this + // question BEFORE it has decided whether there is anything to fill, so that the answer + // is about the RECORD and not about the state of the block. A null context has no open + // transform-feedback span, so clause 2 is false for it - and the verb is a no-op below + // either way. + Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext* ctx) { const MGPWireOp op = WireOpForVerb(verb); if (op == MGPWireOp::kOpCount) return true; if (MGPipeWaitClassFor(op) != kWaitNone) return true; - if (MGPipeCallClassFor(op) == kCtxVerb && ctx.IsTransformFeedbackActive()) return true; + if (MGPipeCallClassFor(op) == kCtxVerb && ctx != nullptr && + ctx->IsTransformFeedbackActive()) { + return true; + } return false; } @@ -3191,9 +3199,8 @@ namespace MobileGL::MG_Pipe { // process rather than a value the wire owes it. MGPipeFillAccess::SetIdentity(inputs, ctx); #if MOBILEGL_BUILD_DISAGGREGATED - // §2.1's predicate, client-side (see ClientVerbIsBarriered). A null context makes the - // verb a no-op below anyway, so it answers barriered and the old path runs whole. - const Bool barriered = !runAhead || ctx == nullptr || ClientVerbIsBarriered(verb, *ctx); + // §2.1's predicate, client-side (see ClientVerbIsBarriered). + const Bool barriered = !runAhead || ClientVerbIsBarriered(verb, ctx); // THE ONE LINE THE RED-ONCE FLIPS: `= true` here is "fill for every verb", the // pre-P5e behaviour, and it turns the guard below into the abort §3.5 names. const Bool fillOwed = barriered; diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 232ef337..165851e3 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -917,9 +917,19 @@ namespace MobileGL::MG_Pipe { // EmitAndWaitTails the client still blocks after every record, so the answer the server // must stamp is `true` whatever the wire says - the two named exemption scopes (§4.4) are // legal exactly while the client is parked, and it is parked behind all of them today. - // Package ra flips this to true in the same commit that lands the wait rule; the sink's - // stamp then follows the predicate. Computing the predicate anyway (PipeApplier::ApplyOne) - // is deliberate: it keeps the function exercised on every record for the whole phase. + // IT FLIPS WITH kMGPipeP5eRunAheadReady, IN THE INTEGRATION COMMIT, AND NOT WHEN ra LANDS + // (ra's amendment to ID-103's wording, which said "the same commit that lands the wait + // rule"). The argument ID-103 makes is about whether the CLIENT is parked, not about + // whether ra's code exists: RunAheadArmed() is a conjunction whose third term is the + // server's caps bit, so between ra landing and the caps bit being published the client + // still blocks after every record - and stamping `false` there would withdraw §4.4's + // exemptions from probes that client's own wait still makes safe, which is exactly the + // "refusal with no defect behind it" ID-103 refused. The two constants are therefore ONE + // switch with two spellings, and the integration commit throws both. + // + // Computing the predicate anyway (PipeApplier::ApplyOne) is deliberate: it keeps the + // function exercised on every record for the whole phase rather than first run on the day + // it starts deciding. inline constexpr Bool kMGPipeP5eClientWaitRuleLanded = false; // P5c (rv): the two serials the PipeInputs texture-shutter accessors answer with under a diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index fc6699ea..50961b30 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -2062,8 +2063,379 @@ TEST(RemoteGuards, ClientPipeInputsFillWithTheApplierIdleIsAllowed) { }); ExpectChildSuccess(child); } + +// =========================================================================================== +// P5e (ra) - the wait rule, the present credit and gPipeInputs' ownership +// (MG_Remote/CONTRACT-P5E.md §1, §2.3, §2.4, §3.1, §3.3, §3.5) +// =========================================================================================== +// +// EVERY CASE BELOW ARMS RUN-AHEAD BY PUBLISHING THE CAP BIT, because that is the only thing +// that arms it: RunAheadArmed() is a conjunction whose third term is the SERVER's own +// statement that it applies an unbarriered record without reading client memory, and in a +// tree where kMGPipeP5eRunAheadReady is still false no production session sets it. Driving the +// arm through the caps mirror rather than through a test-only setter is deliberate (ID-102's +// lesson): what these cases exercise is then the production latch and not a branch beside it. +// +// NONE OF THEM DRAWS. The draw path is the family packages' to migrate; until they land, a +// draw under run-ahead would abort inside the backend for a reason that has nothing to do with +// what is being asserted here. +void StartRunAheadSession() { + ::alarm(15); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.RunAhead = 1; + Srv::ServerSessionInstance().SetCapabilityBits( + static_cast(MG_Pipe::kCapRunAheadApply)); + Srv::ServerSessionInstance().SetConsumedSubsystems(kMGPipeSubsystemsMigratedAtP4a); + // THE MIRROR IS ADOPTED BEFORE Start, AND THAT IS THE LATCH'S RULE MADE VISIBLE. This + // process has no server backend, so PublishCapsSnapshot is deferred and the handshake + // carries no snapshot at all - and §1 says a later snapshot may only turn run-ahead OFF, + // never on. So the arm has to be true at the FIRST adoption, which for a fixture means + // before the session takes its latch. A test that adopted afterwards and found the client + // still lockstep would be reading the contract's own rule as a failure. + { + MG_Pipe::MGPCaps caps{}; + caps.CallMask = static_cast(MG_Pipe::kCapRunAheadApply) | + MG_Remote::MGCapsConsumerBits(kMGPipeSubsystemsMigratedAtP4a); + CapsMirrorInstance().Adopt(caps, MG_Backend::FormatCapabilityCache{}, RendererInfo{}, + String{"4.6"}, BackendType::DirectGLES); + } + if (ClientSessionInstance().Start(MG_Config::TransportMode::InProcess, {}) != MOBILEGL_OK) { + ::_exit(81); + } + if (!ClientSessionInstance().RunAheadArmed()) ::_exit(83); // the latch never took +} + +// THE SERVER HALF OF §2.4, SUBSTITUTED - and it is substituted because this fixture has no +// backend object at all, so ServerVerbSink::OnPresent DECLINES before it can reach +// ServerSession::ReturnPresentCredit ("present arrived with no backend object"). What the +// cases below assert is therefore the CLIENT's half of the credit: that it pays, when, and in +// whose id space. That the server returns exactly one credit per swap, after Present() has +// returned, is PipeApplier::OnPresent's own three lines and the device exit's `credit-waits` +// reading; a peer here cannot pin it without a backend to swap on. +struct PresentCreditPeer : Codec::WireVerbSink { + std::atomic lastSerial{0}; + Bool OnPresent(const MGPPresent& present) override { + lastSerial.store(present.FrameSerial); + Srv::ServerSessionInstance().ReturnPresentCredit(present.FrameSerial); + return true; + } + void Install() { + if (Srv::ServerLoopInstance().RunOnApplyThread( + [](void* self) { + auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{}); + decoder.SetVerbSink(static_cast(self)); + return MOBILEGL_OK; + }, + this) != MOBILEGL_OK) { + ::_exit(82); + } + } +}; + +// §1: the conjunction, and the half of it that is not the client's to decide. The cap bit is +// absent here, so the knob alone changes nothing - which is the whole reason the knob is +// parsed on every arm rather than only where it means something (Config.h). +TEST(RemoteRunAhead, TheKnobAloneDoesNotArmRunAheadWithoutTheServersCapBit) { + const auto child = RunInChild([] { + MG_Config::Ipc.RunAhead = 1; + StartControlSession(); // SetCapabilityBits(0) + if (ClientSessionInstance().RunAheadArmed()) ::_exit(101); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §1 again, from the other side: MOBILEGL_IPC_VERB_BARRIER=0 is the LOCKSTEP arm's negative +// control and it disarms run-ahead outright, because the barrier is the first term of the +// conjunction. Under run-ahead its documented red is the first barriered row's stale pull - +// that one is a scenario, not a unit case; what is pinned here is that the two controls do not +// silently compose into a third mode nobody designed. +TEST(RemoteRunAhead, ClearingTheVerbBarrierDisarmsRunAheadEvenWithTheCapBit) { + const auto child = RunInChild([] { + MG_Config::Ipc.RunAhead = 1; + MG_Config::Ipc.VerbBarrier = 0; + ::alarm(15); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + Srv::ServerSessionInstance().SetCapabilityBits( + static_cast(MG_Pipe::kCapRunAheadApply)); + Srv::ServerSessionInstance().SetConsumedSubsystems(kMGPipeSubsystemsMigratedAtP4a); + MG_Pipe::MGPCaps caps{}; + caps.CallMask = static_cast(MG_Pipe::kCapRunAheadApply) | + MG_Remote::MGCapsConsumerBits(kMGPipeSubsystemsMigratedAtP4a); + CapsMirrorInstance().Adopt(caps, MG_Backend::FormatCapabilityCache{}, RendererInfo{}, + String{"4.6"}, BackendType::DirectGLES); + if (ClientSessionInstance().Start(MG_Config::TransportMode::InProcess, {}) != MOBILEGL_OK) { + ::_exit(81); + } + if (ClientSessionInstance().RunAheadArmed()) ::_exit(101); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.4's RED-ONCE, BY COUNT. With a credit of 1 the client may have one present in flight, so +// the second and third presents each pay the credit before they encode - `credit-waits` is 2 +// after three presents and the present-ack watermark has reached 2. Delete the +// WaitForPresentAck arm from ClientSession::AcquirePresentCredit and the counter stays 0 while +// all three presents publish: red by count, which is the only way a wait that is usually +// already satisfied can be tested at all. +// +// The serial is asserted too, because the counter alone would survive a credit paid against +// the wrong id space (the pre-P5e FrameSerial was 0 and the server stamped its own). +TEST(RemoteRunAhead, ACreditOneClientPaysTheCreditAtEveryPresentAfterTheFirst) { + const auto child = RunInChild([] { + MG_Config::Ipc.PresentCredit = 1; + StartRunAheadSession(); + PresentCreditPeer peer; + peer.Install(); + ClientSession& session = ClientSessionInstance(); + if (session.PresentCreditWaits() != 0) ::_exit(101); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 0) ::_exit(102); // the first one is free + RemoteEmitTable().Present(); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 2) ::_exit(103); + // ONE CREDIT PER SWAP, in the client's own 1-based space: the server returned the + // second present's credit, so the watermark is at least 2. + if (session.Control() == nullptr || + session.Control()->presentAckSerial.load() < 2u) { + ::_exit(104); + } + session.Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.4's other half: with the credit raised the client stops paying until it is that far +// ahead. It is the control that keeps the case above from passing because of an unconditional +// counter rather than because of the credit. +TEST(RemoteRunAhead, ACreditThreeClientPaysNothingForItsFirstThreePresents) { + const auto child = RunInChild([] { + MG_Config::Ipc.PresentCredit = 3; + StartRunAheadSession(); + PresentCreditPeer peer; + peer.Install(); + ClientSession& session = ClientSessionInstance(); + RemoteEmitTable().Present(); + RemoteEmitTable().Present(); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 0) ::_exit(101); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 1) ::_exit(102); + session.Stop(); + }); + ExpectChildSuccess(child); +} + +// §3.1 / §3.5's RED-ONCE. `Clear` is a kWaitNone row, so under run-ahead the client publishes +// it and moves on - and therefore does not fill gPipeInputs for it. This case is the GREEN +// half: the validate point runs the tracker walk and the emitters and touches the block not at +// all, so nothing aborts. +// +// THE RED: in MGPipeValidateForVerb change `const Bool fillOwed = barriered;` to `= true` - +// which is exactly "leave one CopyField in an unbarriered fill" - and the guard below it fires +// Fatal{RoleViolation, "gPipeInputs"} on this very call. +TEST(RemoteRunAhead, AnUnbarrieredVerbDoesNotFillPipeInputsAndIsAllowed) { + const auto child = RunInChild([] { + StartRunAheadSession(); + MGPipeValidateForVerb(MGPipeVerb::Clear); + MGPipeLeaveVerb(); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §3.5, the aborting arm, driven at the guard itself: a GL-thread touch that is NOT the +// residual fill of a barriered record. Under lockstep this same call is a no-op (the apply +// thread is not inside the applier and MOBILEGL_IPC_BATCH_WAITS is 1); under run-ahead there +// is no such window to be outside of, because the client never parks for an unbarriered +// record - so the answer stops depending on timing and becomes the rule. +TEST(RemoteGuards, ClientPipeInputsTouchOutsideABarrieredFillUnderRunAheadIsFatalByName) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("ra-test-surface", + /*isBarrieredFill=*/false); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"gPipeInputs\"}"); +} + +// ... and the control: the barriered fill says so and is let through. Without this the case +// above would pass just as well against "abort unconditionally". +TEST(RemoteGuards, ClientPipeInputsTouchInsideABarrieredFillUnderRunAheadIsAllowed) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("ra-test-surface", + /*isBarrieredFill=*/true); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §3.3's RED-ONCE, and the one that turns the strict lane into a gate. The probe is the +// sticky forward itself, run on the apply thread inside a server-stamped verb with the +// current record marked UNBARRIERED - which is the state the sink puts that thread in for +// every kWaitNone row once the wait rule is live. MOBILEGL_IPC_STRICT_ERRORS is deliberately +// NOT set: the point is that the knob has stopped being the deciding input, because a row the +// client never filled has no value to count. +// +// THE RED: put CountBarrierPull's first line back to `if (MG_Config::Ipc.StrictErrors)` only +// and this case stops aborting - and, on the lane, every scenario that still pulls a row goes +// from a named abort to a wrong picture with a number beside it. +TEST(RemoteGuards, AResidualPullUnderAnUnbarrieredRecordIsFatalWithoutTheStrictKnob) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = false; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeServerStampVerbBoundary(MGPipeVerb::Clear); + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(false); + MG_Pipe::MGPipeStickyForwardPull(MGPipeInputField::GetProgramObject); + return MOBILEGL_OK; + }, + nullptr); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{UnmigratedPipeInput, \"GetProgramObject@Clear\"}"); +} + +// The control: the SAME probe inside a BARRIERED record is P5C's counted residual pull, and +// with the strict knob off it counts and carries on. Ruling 4 is this line - a barriered +// record keeps P5C's semantics exactly, because the client is parked in its own wait. +TEST(RemoteGuards, AResidualPullUnderABarrieredRecordStillOnlyCounts) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = false; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeServerStampVerbBoundary(MGPipeVerb::Clear); + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(true); + const Uint64 before = MG_Pipe::MGPipeResidualPullCount(); + MG_Pipe::MGPipeStickyForwardPull(MGPipeInputField::GetProgramObject); + if (MG_Pipe::MGPipeResidualPullCount() != before + 1) ::_exit(101); + return MOBILEGL_OK; + }, + nullptr); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.6's RED-ONCE, and the one the brief names: ~300 KiB of kEventGpuWritten published from +// the apply thread behind a sequence nobody waited for. SEG_EVENT is 256 KiB, so this is +// several ringfuls; under P5C's rule the FIRST Reserve that failed was Fatal{EventRingOverflow} +// and this case aborts by that name. With flow control the producer publishes, rings, parks on +// the latch and retries, and the burst completes. +// +// THE DRAIN RUNS ON A THREAD OF ITS OWN, and that is forced by the fixture rather than chosen: +// RunOnApplyThread is synchronous, so the thread that posted the burst cannot also be the +// thread that empties the ring. In production it is the GL thread draining at its own waits +// (§2.6's deadlock argument); here it is a drainer beside the poster, which puts the producer +// in exactly the state that argument describes. +TEST(RemoteRunAhead, AnEventBurstBehindAnUnwaitedSequenceFlowControlsInsteadOfAborting) { + const auto child = RunInChild([] { + StartRunAheadSession(); + std::atomic stop{false}; + std::atomic delivered{0}; + std::thread drainer([&stop, &delivered] { + // THE FIRST DRAIN IS DELAYED ON PURPOSE. A drainer that starts immediately can + // keep a 256 KiB ring from ever being full, and a red-once that depends on the + // scheduler losing a race is not a red-once. 50 ms is four orders of magnitude + // more than the burst needs to fill the ring, so the producer is provably parked + // on the latch before anything empties it. + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + while (!stop.load(std::memory_order_acquire)) { + delivered.fetch_add(ClientSessionInstance().DrainPublishedEvents()); + std::this_thread::yield(); + } + delivered.fetch_add(ClientSessionInstance().DrainPublishedEvents()); + }); + // 3000 records of 64 ranges each is ~3 MB against a 256 KiB ring - twelve ringfuls, + // with a drainer racing it - so the full latch is hit many times over rather than + // maybe once. That margin is what makes the red-once (restore the Fatal) reliable: + // a burst that merely might fill the ring would be a red-once that merely might be red. + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPRange ranges[64]; + for (Uint32 r = 0; r < 64; ++r) ranges[r] = MG_Pipe::MGPRange{r * 64ull, 64ull}; + for (Uint32 i = 0; i < 3000; ++i) { + MG_Pipe::gMGPipeCallbacks.OnGpuWritten(MG_Pipe::MGPipeHandle{7, 1}, 64, ranges); + } + return MOBILEGL_OK; + }, + nullptr); + stop.store(true, std::memory_order_release); + drainer.join(); + // LOSSLESS, which is the property flow control had to preserve: every one of the 3000 + // records crossed. A drop policy would have been the other way to survive a full ring, + // and P5C's §4.4 refuses it - a writeback or a GPU-write mark that did not arrive is a + // stale buffer, not a missing statistic. + if (delivered.load() < 3000u) ::_exit(101); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.7 / ruling 13: the deferred-destroy queue stays, and an enqueue from an unbarriered +// apply is the finding. Under strict it is the abort; this is the arm the lane owns. +TEST(RemoteGuards, ADeferredDestroyFromAnUnbarrieredApplyIsFatalUnderStrict) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = true; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(false); + (void)MG_Pipe::MGPipeDeferDestroyAndFreeIfOnApplyThread(MG_Pipe::MGPipeKind::Texture, + 4242); + return MOBILEGL_OK; + }, + nullptr); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"deferred-destroy\"}"); +} + +// The control, and the whole of ruling 13: a BARRIERED apply may still be a last owner (its +// fill's O-class rows, XFB's pinned targets), so the queue takes the death and the GL thread +// replays it. Deleting the queue would have made this an allocator touch from the server role. +TEST(RemoteGuards, ADeferredDestroyFromABarrieredApplyIsStillServed) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = true; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(true); + if (!MG_Pipe::MGPipeDeferDestroyAndFreeIfOnApplyThread(MG_Pipe::MGPipeKind::Texture, + 4243)) { + ::_exit(101); + } + return MOBILEGL_OK; + }, + nullptr); + MG_Pipe::MGPipeDrainDeferredDestroys(); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} #endif +// §2.5 / ruling 15 (ID-93): which half of resource_subdata wants its answer. One predicate, +// read by MGPipeRouteResourceSubData and by Wire_ResourceSubData - a second spelling of it is +// how the client comes to wait for an answer the emitter told the server not to bother with. +// The buffer target's whole packed field is 0 (MGPipeTypes.h asserts it beside the packer), +// which is what makes the test a comparison and not a mask. +TEST(RemoteRunAhead, OnlyTheTextureHalfOfResourceSubDataWantsItsReply) { + MG_Pipe::MGPSubData buffer{}; + buffer.Target = MG_Pipe::MGPipePackSubDataTarget(MG_Pipe::kMGPipeResourceTargetBuffer, 0u); + EXPECT_FALSE(MG_Pipe::MGPipeSubDataWantsItsReply(buffer)); + + MG_Pipe::MGPSubData texture{}; + texture.Target = MG_Pipe::MGPipePackSubDataTarget( + static_cast(MG_Pipe::MGPipeResourceTarget::Tex2D), + static_cast(TextureUploadTarget::Texture2D)); + EXPECT_TRUE(MG_Pipe::MGPipeSubDataWantsItsReply(texture)); +} + int main(int argc, char** argv) { namespace fs = std::filesystem; diff --git a/MobileGL/MG_Test/Wire/SessionTest.cpp b/MobileGL/MG_Test/Wire/SessionTest.cpp index 2d33969d..668f69e7 100644 --- a/MobileGL/MG_Test/Wire/SessionTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionTest.cpp @@ -550,6 +550,122 @@ TEST(SessionTest, PresentAckSerialIsWaitedOnWithGreaterOrEqualAndWakesThroughThe EXPECT_EQ(session->Control().producerParked.load(), 0u); } +// --------------------------------------------------------------------------- +// P5e (ra) - SEG_EVENT flow control (MG_Remote/CONTRACT-P5E.md §2.6) +// --------------------------------------------------------------------------- +// +// THE DEADLOCK THE CONTRACT ARGUES AWAY HAS TWO HALVES AND THIS IS THE FIRST. A run-ahead +// server that cannot Reserve on SEG_EVENT stops producing and therefore stops applying, so +// appliedSeq stops moving - and the client parked on appliedSeq is holding the only drain +// there is. A waiter that could ONLY be woken by the watermark would wait for a number nobody +// is going to publish. +// +// Red-once: point the case at WaitForApplied instead of WaitForAppliedOrEventBacklog and the +// join below never returns - the 5 s deadline fires and the wait reads TimedOut. +TEST(SessionTest, AFullEventRingWakesAClientParkedOnAppliedSeq) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + std::atomic released{false}; + std::atomic result{SessionWait::TimedOut}; + std::thread parked([session, &released, &result] { + result.store(session->producer.WaitForAppliedOrEventBacklog(4, 5000)); + released.store(true, std::memory_order_release); + }); + + while (session->Control().producerParked.load() == 0 && !released.load()) { + std::this_thread::yield(); + } + // NOT the watermark. The server latches "SEG_EVENT is full and I stopped" and rings; the + // client's business is to drain, not to conclude that its record was applied. + session->Control().eventRingFull.store(1, std::memory_order_release); + session->consumer.NotifyClient(); + + parked.join(); + EXPECT_TRUE(released.load()); + EXPECT_EQ(result.load(), SessionWait::Reached); + // AND THE WATERMARK IS UNTOUCHED, which is what makes the caller's re-read the right way + // to tell the two wakeups apart: a third SessionWait value would have turned every + // existing `== Reached` site into a bug. + EXPECT_EQ(session->Control().appliedSeq.load(), 0u); + EXPECT_TRUE(session->producer.EventRingIsFull()); +} + +// The same for the present credit, because a credit-paced client parks there for a whole frame +// at a time and is the likeliest waiter to be holding the drain when the ring fills. +TEST(SessionTest, AFullEventRingWakesAClientParkedOnThePresentCredit) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + std::atomic released{false}; + std::atomic result{SessionWait::TimedOut}; + std::thread parked([session, &released, &result] { + result.store(session->producer.WaitForPresentAckOrEventBacklog(2, 5000)); + released.store(true, std::memory_order_release); + }); + + while (session->Control().producerParked.load() == 0 && !released.load()) { + std::this_thread::yield(); + } + session->Control().eventRingFull.store(1, std::memory_order_release); + session->consumer.NotifyClient(); + + parked.join(); + EXPECT_TRUE(released.load()); + EXPECT_EQ(result.load(), SessionWait::Reached); + EXPECT_EQ(session->Control().presentAckSerial.load(), 0u); +} + +// THE SECOND HALF: the drain that empties the ring must RING, not merely clear. A cleared flag +// with no bell is the lost wakeup the forward direction's publish-then-ring order exists to +// prevent, and the server parked on this flag is inside a producer that cannot return without +// placing its event. +// +// THE BELL IS COUNTED RATHER THAN WAITED ON, and that is the difference between a test and a +// coincidence: a real doorbell REMEMBERS a notify delivered while nobody was parked and can +// return from a park for reasons of its own, so a thread that happened to wake proves nothing +// about who rang. This one records every Notify(). +// +// Red-once: drop the NotifyIfParked from EventRingConsumer::Drained() and `rings` stays 0. +namespace { + struct CountingBell : Doorbell { + std::atomic rings{0}; + void Notify() override { rings.fetch_add(1, std::memory_order_relaxed); } + bool Park(std::uint32_t) override { return false; } + void Reset() override {} + }; +} // namespace + +TEST(SessionTest, DrainingAFullEventRingClearsTheLatchAndRingsTheServer) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + CountingBell serverBell; + // The bell the CLIENT rings is the one the SERVER parks on - which is which is the + // session's knowledge and not the transport's (contract §3.9), so the fixture spells the + // pair the same way ClientSession::Start does. + session->eventIn.SetServerDoorbell(&serverBell); + + // A drain with the latch DOWN is the steady state - every wait exit drains - and it must + // not ring: a store to a shared cache line per drain is what the doorbell design exists to + // avoid (Doorbell.h's bidirectional argument). + session->eventIn.Drained(); + EXPECT_EQ(serverBell.rings.load(), 0u); + + // Now the server is parked on the latch, which is what the flag means. + session->Control().eventRingFull.store(1, std::memory_order_release); + session->Control().consumerParked.store(1, std::memory_order_release); + session->eventIn.Drained(); + + EXPECT_EQ(session->Control().eventRingFull.load(), 0u); + EXPECT_EQ(serverBell.rings.load(), 1u); + + // ONE-SHOT: the latch is down again, so the next drain is silent. Without the exchange - + // a plain store plus a read - a steady stream of drains would ring every time. + session->eventIn.Drained(); + EXPECT_EQ(serverBell.rings.load(), 1u); +} + // --------------------------------------------------------------------------- // kRecPad (R-9's last sentence, and the one with no other detector) // --------------------------------------------------------------------------- From 6ee4c0979bc5c69c1579a22469dd59d197f5b29f Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:22:49 -0400 Subject: [PATCH 4/5] =?UTF-8?q?[CI,=20Docs]=20(Disaggregated):=20P5e=20ra?= =?UTF-8?q?=20-=20the=20strict=20lane=20becomes=20one=20step=20with=20two?= =?UTF-8?q?=20arms,=20and=20=C2=A711/=C2=A714/=C2=A717=20say=20what=20land?= =?UTF-8?q?ed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONTRACT-P5E §7. integration-split-strict was an EXPECTED-RED control: every entry aborts on its first BARRIER-PULLED read, so the step gated on the run failing for that reason. After the flip it is a HARD GREEN GATE - an unbarriered record's pull is Fatal whatever the knob says (§3.3), so strict adds only the barriered rows, and those are a short named allowlist of @. A revert of any handle arm in vi/sb/pg/tx2/fb then goes red HERE, by field and verb, instead of in a picture. - THE ARM IS CHOSEN BY THE TREE. The step greps kMGPipeP5eRunAheadReady out of MG_Backend/Init.cpp, which is the one constant the integration commit flips and is exactly the fact the lane's expectation depends on. A lane whose flip is a separate human step is a lane that is wrong for as long as that step is outstanding - and the pre-flip arm is unchanged, so this commit moves no CI verdict today. - The allowlist names the package that retires each row: readbacks and the pack PBO (P8/P9), the CopyTex endpoint and GenerateMipmap (tx2), set_storage_block_binding (pg), XFB (P3b/P4b/P9), resource_copy_region's GL names (the declared P5b debt). A row leaves it by deletion, never by widening. The lane also warns on a non-zero rsp, which after P5e can only be a barriered row. - The allowlist is built with printf rather than a heredoc: a quoted heredoc's terminator has to start at column 0, which inside a YAML block scalar would end the scalar instead of the heredoc. - ARCHITECTURE §11.6: the wait rule is the generated WaitClass column now, not the call class, and it REPLACES the BatchWaits skip; glFinish stops being a no-op under run-ahead; the forced waits (EGL forwarders, the map/readback family) and glGetError's named window are listed. §11.7: the P9 plan is code - the producer blocks, the drain clears AND rings, the client's parks break on the latch, and two named refusals remain for the impossible cases. §14: the present credit is client-minted, 1-based, one per swap, paid before the encode. §17.7: what P5e is, in one section - the predicate, rule F, gPipeInputs' ownership, the latch and the control arms. - 附 A gains MOBILEGL_IPC_RUN_AHEAD and moves MOBILEGL_IPC_PRESENT_CREDIT out of the "P6+" line: a knob listed as not-yet-effective while it paces the frame is how an A/B stops being one. --- .github/workflows/test.yml | 111 +++++++++++++++++++++++------ docs/Disaggregated/ARCHITECTURE.md | 30 ++++++-- 2 files changed, 115 insertions(+), 26 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0afb10f0..7dbbba56 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1187,11 +1187,24 @@ jobs: MOBILEGL_IPC_STRICT_ERRORS: "1" run: ctest --output-on-failure -L unit --no-tests=error -j "$(nproc)" - # The expected-red half: every integration-split entry aborts on its first BARRIER-PULLED - # read under strict. The run is EXPECTED to fail; what the step gates on is that the - # failure is the strict Fatal and nothing else. A scenario that PASSED would mean strict - # never armed, and one whose private log lacks the marker failed for another reason. - - name: Integration-split aborts under strict with the strict marker + # ---- P5e (ra), MG_Remote/CONTRACT-P5E.md §7: ONE STEP, TWO ARMS, AND THE TREE PICKS ---- + # + # This lane was written as an EXPECTED-RED control: under strict every integration-split + # entry aborts on its first BARRIER-PULLED read, so the step gated on the run failing and + # on the strict marker being the reason. P5e turns it into a HARD GREEN GATE - once the + # client runs ahead, an unbarriered record's pull is a Fatal whatever this knob says + # (§3.3), so what strict adds is only the BARRIERED rows' pulls, and those are a short, + # named list. A revert of any handle arm in vi/sb/pg/tx2/fb then goes red HERE, by field + # and verb, instead of in a picture nobody diffs. + # + # THE ARM IS CHOSEN BY THE TREE, NOT BY A SECOND EDIT. kMGPipeP5eRunAheadReady in + # MG_Backend/Init.cpp is the one constant the integration commit flips, and it is exactly + # the fact this lane's expectation depends on: with it false no server publishes + # kCapRunAheadApply, every record is barriered, every scenario still aborts on its first + # pull and the expected-red control is the honest gate. Reading it here means the lane + # changes meaning in the same commit the runtime does - a lane whose flip is a separate + # human step is a lane that is wrong for as long as that step is outstanding. + - name: Integration-split under strict - expected-red before the P5e flip, hard green after working-directory: build-split env: MOBILEGL_ITEST_REQUIRE_GPU: "1" @@ -1199,27 +1212,83 @@ jobs: MOBILEGL_TRANSPORT: "inproc" run: | ulimit -c unlimited + ready=$(grep -oE 'kMGPipeP5eRunAheadReady = (true|false)' "${GITHUB_WORKSPACE}/MobileGL/MG_Backend/Init.cpp" | head -1 | awk '{print $3}') + echo "kMGPipeP5eRunAheadReady = ${ready}" rc=0 ctest -L integration-split --no-tests=error -j 4 > "${RUNNER_TEMP}/strict-split.txt" 2>&1 || rc=$? - cat "${RUNNER_TEMP}/strict-split.txt" | tail -5 - if [ "${rc}" -eq 0 ]; then - echo "::error::integration-split under MOBILEGL_IPC_STRICT_ERRORS=1 went GREEN - the object-class residual pulls (P3b/P4b/P7) are still pinned, so strict must abort every drawing scenario. If the object-class rows have just retired, rewrite this step as the green lane it then becomes." - exit 1 - fi - failed=$(grep -cE '^ *[0-9]+ - .* \(.*(Failed|Subprocess aborted).*\)' "${RUNNER_TEMP}/strict-split.txt" || true) - # Every entry that RAN (did not skip) must have failed; entries skipped by a missing - # GPU or an unmet pre-flight are reported, not counted. - ran=$(grep -cE 'tests passed, [0-9]+ tests failed out of [0-9]+' "${RUNNER_TEMP}/strict-split.txt" || true) + tail -5 "${RUNNER_TEMP}/strict-split.txt" echo "strict run: rc=${rc}" - # The strict marker, from the split entries' own private logs: the run's red must be - # the strict Fatal and not a bring-up defect. - marker='BARRIER-PULLED, MOBILEGL_IPC_STRICT_ERRORS=1' - hits=$(grep -rl "${marker}" MobileGL/MG_IntegrationTest/split-logs/ 2>/dev/null | wc -l) - if [ "${hits}" -lt 1 ]; then - echo "::error::no split entry's private log carries the strict marker '${marker}' - the lane went red without the strict Fatal firing, which is a defect, not the control" + + if [ "${ready}" != "true" ]; then + # ---- the pre-flip arm, unchanged from P5c ---- + if [ "${rc}" -eq 0 ]; then + echo "::error::integration-split under MOBILEGL_IPC_STRICT_ERRORS=1 went GREEN while kMGPipeP5eRunAheadReady is false - the object-class residual pulls (P3b/P4b/P7) are still pinned, so strict must abort every drawing scenario. If P5e has just landed, flip that constant in the same commit." + exit 1 + fi + marker='BARRIER-PULLED, MOBILEGL_IPC_STRICT_ERRORS=1' + hits=$(grep -rl "${marker}" MobileGL/MG_IntegrationTest/split-logs/ 2>/dev/null | wc -l) + if [ "${hits}" -lt 1 ]; then + echo "::error::no split entry's private log carries the strict marker '${marker}' - the lane went red without the strict Fatal firing, which is a defect, not the control" + exit 1 + fi + echo "integration-split under strict is red by design; ${hits} private log(s) carry the strict marker" + exit 0 + fi + + # ---- the post-flip arm: HARD GREEN, with §7's allowlist ---- + # + # EVERY ENTRY MUST COMPLETE. An abort here is either an unbarriered pull (§3.3's + # unconditional Fatal, which is a missed migration) or a barriered row that is not on + # the list below, and both are the lane's job to catch. + if [ "${rc}" -ne 0 ]; then + echo "::error::integration-split under MOBILEGL_IPC_STRICT_ERRORS=1 is RED on a run-ahead server. CONTRACT-P5E §7 makes this lane a hard green gate: an unbarriered record's pull aborts whatever the knob says, so a red here names a field and a verb in the failing entry's private log." + grep -hoE 'Fatal\{UnmigratedPipeInput, "[^"]*"\}' MobileGL/MG_IntegrationTest/split-logs/*.log 2>/dev/null | sort | uniq -c | sort -rn | head -20 exit 1 fi - echo "integration-split under strict is red by design; ${hits} private log(s) carry the strict marker" + + # THE ADMITTED MARKERS, `@`, AND NOTHING ELSE. These are the barriered + # rows of §2.2 - the ones whose apply still pulls a row, and whose legality is the + # client's own wait (ruling 4). Each line names the package that retires it; a row + # that leaves this list leaves it by deletion, never by widening. + # readbacks / the pack PBO ......... P8/P9 (read_pixels, get_texture_image) + # CopyTex endpoint ................. tx2's trailing item + # GenerateMipmap ................... tx2, until the VerbMipRes arm lands + # set_storage_block_binding ........ pg's trailing item (by MGPStorageBlockBinding::ShaderCso) + # XFB ............................... P3b/P4b/P9 (escalation (i) keeps the span lockstep) + # resource_copy_region's names ..... the declared P5b debt + printf '%s\n' \ + 'GetTextureUnitObject@ReadPixels' \ + 'GetTextureUnitObject@GetTextureImage' \ + 'GetTextureUnitObject@CopyTexImage2D' \ + 'GetTextureUnitObject@GenerateMipmap' \ + 'GetBufferBindingSlot@ReadPixels' \ + 'GetBufferBindingSlot@GetTextureImage' \ + 'GetProgramObject@ShaderStorageBlockBinding' \ + 'ValidateProgramName@ShaderStorageBlockBinding' \ + 'GetTextureObject@CopyImageSubData' \ + 'GetTransformFeedbackProgram@BeginTransformFeedback' \ + 'GetTransformFeedbackProgram@EndTransformFeedback' \ + 'GetBufferBindingPoint@BeginTransformFeedback' \ + 'GetBufferBindingPoint@EndTransformFeedback' \ + > "${RUNNER_TEMP}/allowed.txt" + + grep -rhoE '\[BARRIER-PULLED[^]]*\]' MobileGL/MG_IntegrationTest/split-logs/ 2>/dev/null > "${RUNNER_TEMP}/markers-raw.txt" || true + grep -rhoE 'Fatal\{UnmigratedPipeInput, "[^"]*"\}' MobileGL/MG_IntegrationTest/split-logs/ 2>/dev/null \ + | sed -E 's/.*"([^"]*)".*/\1/' | sort -u > "${RUNNER_TEMP}/markers.txt" || true + bad=$(comm -23 "${RUNNER_TEMP}/markers.txt" <(sort -u "${RUNNER_TEMP}/allowed.txt") || true) + if [ -n "${bad}" ]; then + echo "::error::a BARRIER-PULLED marker outside CONTRACT-P5E §7's allowlist appeared on a run-ahead server. Each one is a field an apply still reads out of client memory:" + echo "${bad}" + exit 1 + fi + + # `rsp` = 0 ON UNBARRIERED RECORDS, in every summary that printed it. A non-zero count + # would be an abort that did not fire, which is the one failure §3.3's unconditional + # arm cannot report by itself. + if grep -rhoE 'rsp=[0-9]+' MobileGL/MG_IntegrationTest/split-logs/ 2>/dev/null | grep -qvE 'rsp=0$'; then + echo "::warning::a scenario summary reports a non-zero rsp; under P5e every remaining pull is a BARRIERED row on the allowlist above, and the count belongs in the trailing inventory rather than in this lane" + fi + echo "integration-split under strict is GREEN with only the §7 allowlist admitted ($(wc -l < "${RUNNER_TEMP}/markers.txt") distinct marker(s))" - name: Upload strict lane logs if: always() diff --git a/docs/Disaggregated/ARCHITECTURE.md b/docs/Disaggregated/ARCHITECTURE.md index b82fdb44..5eae7f4b 100644 --- a/docs/Disaggregated/ARCHITECTURE.md +++ b/docs/Disaggregated/ARCHITECTURE.md @@ -412,12 +412,18 @@ server 没有第二份 `BufferObject`，不存在"staging → server 侧 shadow" - 不设"records ≥ 64 KiB"一类阈值。规则：每条记录（或每 8–16 条摊销）release-store `cmdHead`，仅当 `consumerParked` 时敲门铃。 - 显式门铃点：`present`、任何 `kNeedsAck` 请求、`eglMakeCurrent`、`glFlush`、`SEG_STAGE` 余量 < 1/4、**轮询类入口**（`glClientWaitSync` 任意 timeout、`glGetSynciv(GL_SYNC_STATUS)`、`glGetQueryObject*(AVAILABLE|NO_WAIT)`）；带 `GL_SYNC_FLUSH_COMMANDS_BIT` 无条件 publish。饥饿升级：同一 handle 连续 N 次（`MOBILEGL_IPC_POLL_ESCALATE`，默认 64）本地回答"未就绪"而 watermark 毫无移动 → 升级为一次阻塞 round trip（P10）。 -- `glFinish`/`glFlush` 保持纯 no-op。seq = 记录序数；两个互相独立的窗口：字节 credit 与 present credit（`presentsSent - presentAckSerial >= MOBILEGL_IPC_PRESENT_CREDIT` 时 `eglSwapBuffers` 阻塞）。server 不发 credit 消息，consumer 每 64 条记录更新一次 `appliedSeq`。 +- `glFlush` 保持纯 no-op。**`glFinish` 在 P5e 之后不再是**：wire 上仍然没有它的记录，但"至此发出的命令都已完成"是一条 run-ahead 队列能违背的承诺，所以它变成 `WaitForApplyToCatchUp`——等到 `appliedSeq` 追上最后一条发布的记录，再排空反向通道（`CONTRACT-P5E.md` §2.5）。lockstep 下它照旧什么都不做，因为那里 client 本来就把自己发出的每条命令都等完了。 +- seq = 记录序数；两个互相独立的窗口：字节 credit 与 present credit（`presentsSent - presentAckSerial >= MOBILEGL_IPC_PRESENT_CREDIT` 时 `eglSwapBuffers` 阻塞）。server 不发 credit 消息，consumer 每 64 条记录更新一次 `appliedSeq`。 +- **P5e：等待规则由生成的 `WaitClass` 列决定，不再由 call class 决定。**`MGPipeWaitClassFor(op)`（`PipeCalls.def` 的第五列）把每一行分成 `kWaitReply` / `kWaitPresent` / `kWaitApplied` / `kWaitNone`，`EmitAndWaitTails` 在 `RunAheadArmed()` 为真时照这一列走：`kWaitNone` 发布即返回，`kWaitApplied` 等一次 apply 再放掉 fill 的四个 O 类 pin，`kWaitPresent` 的等待已经在 `EmitPresent` 编码之前付过（下条）。它**替换**而不是叠加 `MOBILEGL_IPC_BATCH_WAITS` 的跳过规则：batch 读的是 call class，这一列读的是"这条记录的 apply 会读什么"，而 server 侧的 `MGPipeBarriered` 读的是后者——两条规则并存就是线两端对"client 现在是否被 park 着"给出不同答案。 +- **强制等待点**（§2.5）：`MapBuffer(READ)` / `GetBufferSubData` / `CopyBufferSubData` 源走 `SyncGpuWrites`（reply 行，不变）；`glFinish` 如上；每一个 `Server*` EGL forwarder 之前等一次——control mailbox 是在两批 drain **之间**泵的，make-current 落在两条 run-ahead 记录中间不是画错一个像素而是全错。`ServerSwapEGLBuffers` 不在此列：present 就是那次 swap。`glGetError` 的放宽也在这里具名：run-ahead verb 的 `kEventGlError` 在下一个排空点被看到，即最多晚一个 present credit。 -### 11.7 事件回传与溢出（P9） +### 11.7 事件回传与溢出（P9 定策略，P5e 落地） `SEG_EVENT` 承载十个回调加回读完成通知。client 排空点：`glGetError`、`glGetQueryObject*`、`glClientWaitSync`、`glGetSynciv`、`eglSwapBuffers`、`glMapBuffer*`/`glGetBufferSubData`/`glCopyBufferSubData`，以及每一次等待循环的每一轮。溢出策略（修一个双向死锁）：`EvLogLine` ≤WARN 有损；语义承载事件无损——ring 满时 server 置 `eventRingFull=1`、**在记录边界停止 apply**、敲反向门铃，client 排空后清标志并敲正向门铃。故障注入：client 被 credit 阻塞时灌满 `SEG_EVENT`；日志洪泛下注入一次 link 失败，那行 ERROR 必须出现且两侧恢复。 +**P5e 把上面这段从计划变成代码（`CONTRACT-P5E.md` §2.6）。**`Fatal{EventRingOverflow}` 的前提是"client 每个 verb 都排空，所以 256 KiB 满环是没有工作负载会产生的突发"；run-ahead 的 client 在自己的等待点排空，稳态上相隔一个 present，满环因此是普通积压，为它中止等于把反向通道的容量变成工作负载上限。现在 producer 改为阻塞：发布已在环里的部分、敲 client 门铃、park 在自己的 bell 上等标志清掉、重试。两条具名拒绝留下，而且是**不可能**而非**繁忙**的情形——单条事件大于半个环（再怎么排空也放不下），以及 30 秒都等不到排空（对端不在跑）。不发布 `kCapRunAheadApply` 的 server 原样保留 P5C 的 Fatal。 +死锁论证要两半才成立，所以两半都在：`EventRingConsumer::Drained()` **清标志的同时敲门铃**（清了不敲就是正向那条"先 publish 再敲"专门在防的丢唤醒），而 `SessionProducer::WaitForAppliedOrEventBacklog` / `WaitForPresentAckOrEventBacklog` 让 client 的每一次 park 都能被 `eventRingFull` 打断——于是"等着 client 排空的 server" 总能叫醒那个 client。`ServerLoop` 的 park 条件因此多一条：环满时等待中的记录**还不是**工作，因为 apply 它只会让 producer 停在半路；Stop 与 Control 仍然优先，拆机不被一个不排空的 client 拖住。 + ### 11.8 fence 与无 present 负载（P10） fence 完成度必须来自**真的逐 fence 退休**，不是 present 水位（DirectGLES 的 `g_completedFrameSerial` 只在 `Present()` 里前进，帧中 fence 会退化成帧计数推断——`DirectVulkan.cpp` 写明这是被修掉的 bug）。无 present 循环下 `retiredTail` 会饿死：DirectGLES 的 server 加**非 present fence tick**（距上次 `Present` 超过 8 ms 或每 4096 条已 apply 记录插一个 `glFenceSync`）；P8 加一个无 present 的 split 用例。P5b 已把五条 sync 槽搬上 apply 线程（§17.5）。 @@ -466,7 +472,9 @@ fence 完成度必须来自**真的逐 fence 退休**，不是 present 水位（ ## 14. Present、线程与帧节奏 - `eglSwapBuffers` → `present{frameSerial}` → publish + 敲门铃 → 返回，除非超出 credit。**`present` 与 `eglSwapBuffers` 严格 1:1**：两个后端的帧边界排空只在 `Present` 内发生，批量会饿死它们。 -- **`MOBILEGL_IPC_PRESENT_CREDIT` 默认 1**（P10）：延迟叠加，server 的 `Present` 末尾已在 `vkWaitForFences` 上等 2–3 帧，credit 2 就是端到端 4–5 帧；只有实测吞吐收益能抵掉延迟代价才调高。Magma 从不注册 `SetSwapInterval`，IPC credit 是它唯一的显式限帧器。 +- **`MOBILEGL_IPC_PRESENT_CREDIT` 默认 1**（P10 提出，P5e 落地，range 1..8）：延迟叠加，server 的 `Present` 末尾已在 `vkWaitForFences` 上等 2–3 帧，credit 2 就是端到端 4–5 帧；只有实测吞吐收益能抵掉延迟代价才调高。Magma 从不注册 `SetSwapInterval`，IPC credit 是它唯一的显式限帧器。 +- **P5e：credit 是 run-ahead 的唯一稳态背压，字节不是**（`CONTRACT-P5E.md` §2.4）。`MGPPresent::FrameSerial` 从"0，由 server 盖自己的帧数"改成**由 client 铸造、1 起算**——一个 credit 只能在付账方能推进的 id 空间里计量，而付账方是 client；`ServerVerbSink::OnPresent` 在 `Present()` 返回之后调 `ServerSession::ReturnPresentCredit(FrameSerial)`，一次 swap 一个 credit（那个函数自 v1 写下以来一直没有生产调用者，这是它的第一个）。credit 1 = client 在 server 应用并交换第 N 帧时发布第 N+1 帧的记录：一帧重叠，最多一帧附加延迟。等待发生在**编码之前**——先编码再 park 会把一次 SEG_CMD 预留持有整整一帧 server 时间。run-ahead server 上 `FrameSerial == 0` 是 `Fatal{ProtocolCorruption, "Present.FrameSerial"}`。 +- 预算（~850 draw + ~0.36 MB pmap/帧）：SEG_CMD 每帧 ~70 KB 对 8 MiB，SEG_STAGE ~1 MB/帧对 32 MiB，SEG_REPLY 不变。`WaitForCmdSpace` 与 stage bell 是安全网而不是节奏器，两者退出时都排空 SEG_EVENT。stats 行与 wire ledger 多一个 `credit-waits`：credit 1 下它是 0 就说明 server 从来不在帧的关键路径上，接近帧数就说明它一直是。 - 线程——client：**v1 不加线程**，编码在 GL 线程上直接写 ring；外来线程的 sync/query 读从 `RingControl` 无锁回答，必须发射的少数取 `ctrlMutex` 走 `AuxRequest`（SPSC ring 不允许第二个 producer）。server：`mgl-srv-io`（封帧、`SCM_RIGHTS`、doorbell、CTRL RPC）、`mgl-srv-apply`（**终身持有原生 context**，`MakeCurrent` 的缓存失效风暴变启动期一次性）。 - **核心放置**：全库无亲和性控制。规则：报总 CPU 工作量差（client tracker + encode + decode + server apply vs monolith `PrepareForDraw`）；复用 `ShaderCompilePool` 的大核探测把 `mgl-srv-apply` 绑到大核（`MOBILEGL_IPC_SERVER_AFFINITY`，默认 auto）。 - 拆机顺序：publish + server 排空并 ack → 停 apply 线程 → 关 transport → client 排空 compile pool → `MobileGL::Destroy()` → 释放 sync/query handle（P5 落地形状见 §17.3）。 @@ -571,6 +579,16 @@ P5c 的设计决定（六条全部落地，落地形状与偏差以 `MobileGL/MG 落地时的三条结构性补充（契约 §3.1 与 §5.4 的具名豁免）：审计漏了三族无句柄站点——绑定记录 P4b 才发射的 ensure/通告族（`MGPipeReverseAnnouncementScope`）、G6 前端键 twin registry（`MGPipeFrontendKeyedRegistryScope`，P3b/P4b 重键）、Magma 拆除期的隐藏资源；两个 scope 内的只读探测是具名、可 grep、带退役阶段的债，不是守卫的洞。 +### 17.7 P5e：client 在 Espryt draw 路径上跑在 apply 前面（权威为 `MobileGL/MG_Remote/CONTRACT-P5E.md`） + +P5d 把 `inproc` 变成 client 线程 CPU-bound，帧仍是 `client 工作 +（每 draw）等 apply + 交接`。那个等待存在，是因为 draw 的 apply 仍在解引用 client 拥有的内存：`FieldOwnership.def` 的对象类 BARRIER-PULLED 行，以及两个 P5C 具名豁免 scope 里的前端键 twin registry。**P5e 的一句话：client 发布完就走；server 用记录带的句柄解析每一个对象；它唯一可以读的 client 内存，是一条 client 确实被挡在后面的记录的残余 fill。** + +- **barriered 谓词**（§2.1，裁定 3 / ID-83）：`MGPipeBarriered(op, payload, applierState)` = 静态 `WaitClass` 列 ≠ `kWaitNone`，或 XFB span 打开时的 `kCtxVerb`，或带 `kDrawClientArrays` 的 draw。两侧算同一个函数、同一份数据——client 在 emit 时用 `ctx.IsTransformFeedbackActive()`，server 用应用过的 `MGPContextValues` 镜像，而 `set_context_values` 在 ring 上先于 verb，所以两者一致。没有第四条升级；加一条是整合者的裁定加契约里的一行。 +- **规则 F**（§0）：**unbarriered 记录的 apply 不读任何 client 内存**——不解引用前端对象、不读 BARRIER-PULLED 行、不探测也不铸造 `MGPipeSlots()`、不持有前端对象的 `SharedPtr`。违反是具名 Fatal，**与 `MOBILEGL_IPC_STRICT_ERRORS` 无关**：那个值按构造就是撕裂或陈旧的，没有"计一笔"的臂。barriered 记录原样保留 P5C 的语义（裁定 4 / ID-84），这正是第一次落地能做小的原因：回读、CopyTex、`GetTexImage`、`set_storage_block_binding`、XFB 都留在 barriered 且拖后。 +- **`gPipeInputs` 变成 server 角色内存**（§3）：unbarriered verb 的 validate 只跑第 2 步（tracker 走查）和第 3 步（发射器）——它们产出记录，这正是 unbarriered verb 被允许交给 server 的全部——序号自增、verb 戳、撤回 server 戳与整个第 4 步都不跑。`MGPipeInputUnfreshRead` 的计数器在 unbarriered 记录下变成**无条件中止**，这是把 `integration-split-strict` 从"预期红"变成硬绿门的那一处（§7）。 +- **run-ahead 的臂**：`RunAheadArmed()` = `m_barrierArmed && Ipc.RunAhead && Caps().HasCap(kCapRunAheadApply)`，在 `Start` 之后第一次 caps 采纳时**锁存**，此后只能被关掉、不能被打开——已经不等就发布出去的记录收不回来。Magma 的 `CallMask` 永远不带 bit 10（`MGPipeRunAheadCapBitsFor` 是纯函数，`MagmaPipeIdentityTest` 钉住），Espryt 则由 `MG_Backend/Init.cpp` 的 `kMGPipeP5eRunAheadReady` 一个常量把整相位关在惰性里，直到整合提交把它翻过来。 +- **对照臂**：`MOBILEGL_IPC_RUN_AHEAD=0` 是 A/B 的**对照**而不是负控制——server 代码、fill 决定、每一条记录两臂都一样，唯一的差别是 client 等不等；`MOBILEGL_IPC_VERB_BARRIER=0` 保留它自己的意思，并且因为它是那个合取式的第一项，它会直接把 run-ahead 关掉。 + ## 附 A：开关 CMake： @@ -610,13 +628,15 @@ CMake： | `MOBILEGL_IPC_PERSISTENT_HASH_SUPPRESS` | 1 | 推送只发内容变了的块（追踪 buffer 走 mprotect 位图，未追踪走内容哈希）；`0` 恢复全范围推送（A/B 对照） | | `MOBILEGL_IPC_BATCH_WAITS` | 1 | 值类记录（无 reply slot 的 kCtxState/kCtxCso/kCtxObject）发布即返回，barrier 推迟到下一个拉取类 verb；**`generate_mipmap` 例外**（它的 apply 读 `MGB_CTX->GetActiveTextureUnit()`，规则：只有 apply 不读残余填充字段的记录才可免等）；`0` 恢复 R-1 逐条 barrier（`MOBILEGL_PIPE_VERIFY` 强制 0） | | `MOBILEGL_IPC_ADOPT_TIER` | 2 | `auto/0/1/2`；P5 split 用 emulated（T2） | -| `MOBILEGL_IPC_VERB_BARRIER` | 1 | 每 verb 等 `appliedSeq == emitSeq`；`0` 只作 E1 阴性对照 | +| `MOBILEGL_IPC_VERB_BARRIER` | 1 | 每 verb 等 `appliedSeq == emitSeq`；`0` 只作 E1 阴性对照，并且因为它是 `RunAheadArmed()` 合取式的第一项，它同时把 run-ahead 关掉 | +| `MOBILEGL_IPC_RUN_AHEAD` | 1 | P5e：发布完 unbarriered 记录即返回（§11.6 的 `WaitClass` 列）。**只是合取式的一半**——server 不发布 `kCapRunAheadApply` 时它等于 0 并日志一次；`0` 是 **A/B 对照**（两臂 server 代码与记录完全相同，只差 client 等不等），不是阴性对照。`MOBILEGL_PIPE_VERIFY` 强制 0 | +| `MOBILEGL_IPC_PRESENT_CREDIT` | 1 | P5e：client 可以在手的 present 数，range 1..8。credit 1 = 一帧重叠、最多一帧附加延迟；2 是设备测量臂。stats 行的 `credit-waits` 是它真正阻塞的次数 | | `MOBILEGL_IPC_STRICT_ERRORS` | 0 | BARRIER-PULLED residual input 提升为具名 Fatal | | `MOBILEGL_IPC_AUDIT` | 0 | retire 后 `0xDD` 填退休 staging | | `MOBILEGL_IPC_SERVER_AFFINITY` | `auto` | apply 线程亲和性；日志报告内核实际采纳的掩码（Redmi 的内核对 app 线程一律忽略 `sched_setaffinity`，解析为 0xff） | | `MOBILEGL_IPC_SERVER_PATH` | 空 | P6 消费 | -P6+ 生效：`MOBILEGL_IPC_PRESENT_CREDIT`、`MOBILEGL_IPC_POLL_ESCALATE`、shadow shm、`MOBILEGL_IPC_RESPAWN`、`MOBILEGL_IPC_IDLE_EXIT_S`。显式不设立：`MOBILEGL_IPC_PROGRAM`（没有 relink 档）、`MOBILEGL_IPC_VALIDATE_SERVER`（server 没有 `MG_Impl` 校验器）。 +P6+ 生效：`MOBILEGL_IPC_POLL_ESCALATE`、shadow shm、`MOBILEGL_IPC_RESPAWN`、`MOBILEGL_IPC_IDLE_EXIT_S`（`MOBILEGL_IPC_PRESENT_CREDIT` 已在 P5e 生效，见上表）。显式不设立：`MOBILEGL_IPC_PROGRAM`（没有 relink 档）、`MOBILEGL_IPC_VALIDATE_SERVER`（server 没有 `MG_Impl` 校验器）。 ## 附 B：边界计数器（`MobileGL/MG_Util/Metrics/PipeStats.h`） From 48b2f56ef8d984e9d0adbc1af35aa2d550633265 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:27:00 -0400 Subject: [PATCH 5/5] [MG_Remote] (Disaggregated): P5e ra - the run-ahead latch goes down with the session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONTRACT-P5E §1. The latch means "what THIS session's first caps adoption said", and ClientSession is a process singleton that Start/Stop can cycle. Left standing, a second Start would inherit a verdict about a server that is gone - and because a later snapshot may only turn run-ahead OFF, an inherited `true` could never be corrected by the honest answer arriving afterwards. - The present serial resets with it: a credit's id space belongs to the session that paces on it, and a server that never saw frame 1 cannot return its credit. --- MobileGL/MG_Remote/Client/ClientSession.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index b1332132..d9fcc6dc 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -802,6 +802,14 @@ namespace MobileGL::MG_Remote::Client { m_serverTransport.reset(); m_transport = nullptr; m_started = false; + // P5e (ra, §1): the LATCH GOES DOWN WITH THE SESSION. It is "what the first caps + // adoption of THIS session said", and a session that starts again - the same singleton, + // a second Start - must take it again from that session's own first snapshot rather + // than inherit a verdict about a server that is gone. The present serial goes with it + // for the same reason: the credit's id space belongs to the session that paces on it. + m_runAheadArmed = false; + m_runAheadLatched = false; + m_presentsSent = 0; if (g_active == this) { g_active = nullptr; }