[libc-commits] [libc] 64d169c - [libc][NFC] Simplifly inbox and outbox state handling

[Joseph Huber via libc-commits]

Author: Joseph Huber
Date: 2023-05-16T13:12:00-05:00
New Revision: 64d169c74d2cd3730893dffacd0061b937e55169

URL: https://github.com/llvm/llvm-project/commit/64d169c74d2cd3730893dffacd0061b937e55169
DIFF: https://github.com/llvm/llvm-project/commit/64d169c74d2cd3730893dffacd0061b937e55169.diff

LOG: [libc][NFC] Simplifly inbox and outbox state handling

Currently we use a template parameter called `InvertInbox` to invert the
inbox when we load it. This is more easily understood as a static check
on whether or not the process running it is the server. Inverting the
inbox makes the states 1 0 and 0 1 own the buffer, so it's easier to
simply say that the server own the buffer if in != out. Also clean up some of
the comments.

Reviewed By: JonChesterfield

Differential Revision: https://reviews.llvm.org/D150365

Added: 
    

Modified: 
    libc/src/__support/RPC/rpc.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/RPC/rpc.h b/libc/src/__support/RPC/rpc.h
index 540adcebd76b7..a73c540fa0b1b 100644
--- a/libc/src/__support/RPC/rpc.h
+++ b/libc/src/__support/RPC/rpc.h
@@ -78,27 +78,18 @@ struct alignas(64) Packet {
 constexpr uint64_t DEFAULT_PORT_COUNT = 64;
 
 /// A common process used to synchronize communication between a client and a
-/// server. The process contains an inbox and an outbox used for signaling
-/// ownership of the shared buffer between both sides.
+/// server. The process contains a read-only inbox and a write-only outbox used
+/// for signaling ownership of the shared buffer between both sides. We assign
+/// ownership of the buffer to the client if the inbox and outbox bits match,
+/// otherwise it is owned by the server.
 ///
-/// No process writes to its inbox. Each toggles the bit in the outbox to pass
-/// ownership to the other process.
-/// When inbox == outbox, the current state machine owns the buffer.
-/// Initially the client is able to open any port as it will load 0 from both.
-/// The server inbox read is inverted, so it loads inbox==1, outbox==0 until
-/// the client has written to its outbox.
-///
-/// This process is designed to support mostly arbitrary combinations of 'send'
-/// and 'recv' operations on the shared buffer as long as these operations are
-/// mirrored by the other process. These operations exchange ownership of the
-/// fixed-size buffer between the users of the protocol. The assumptions when
-/// using this process are as follows:
-///   - The client will always start with a 'send' operation
-///   - The server will always start with a 'recv' operation
-///   - For every 'send' / 'recv' call on one side of the process there is a
-///     mirrored 'recv' / 'send' call.
-///
-template <bool InvertInbox> struct Process {
+/// This process is designed to allow the client and the server to exchange data
+/// using a fixed size packet in a mostly arbitrary order using the 'send' and
+/// 'recv' operations. The following restrictions to this scheme apply:
+///   - The client will always start with a 'send' operation.
+///   - The server will always start with a 'recv' operation.
+///   - Every 'send' or 'recv' call is mirrored by the other process.
+template <bool Invert> struct Process {
   LIBC_INLINE Process() = default;
   LIBC_INLINE Process(const Process &) = delete;
   LIBC_INLINE Process &operator=(const Process &) = delete;
@@ -148,23 +139,12 @@ template <bool InvertInbox> struct Process {
                                  alignof(Packet))));
   }
 
-  /// Inverting the bits loaded from the inbox in exactly one of the pair of
-  /// processes means that each can use the same state transitions.
-  /// Whichever process has InvertInbox==false is the initial owner.
-  /// Inbox equal Outbox => current process owns the buffer
-  /// Inbox difer Outbox => current process does not own the buffer
-  /// At startup, memory is zero initialised and raw loads of either mailbox
-  /// would return zero. Thus both would succeed in opening a port and data
-  /// races result. If either inbox or outbox is inverted for one process, that
-  /// process interprets memory as Inbox!=Outbox and thus waits for the other.
-  /// It is simpler to invert reads from the inbox than writes to the outbox.
+  /// Retrieve the inbox state from memory shared between processes.
   LIBC_INLINE uint32_t load_inbox(uint64_t index) {
-    uint32_t i = inbox[index].load(cpp::MemoryOrder::RELAXED);
-    return InvertInbox ? !i : i;
+    return inbox[index].load(cpp::MemoryOrder::RELAXED);
   }
 
   /// Retrieve the outbox state from memory shared between processes.
-  /// Never needs to invert the associated read.
   LIBC_INLINE uint32_t load_outbox(uint64_t index) {
     return outbox[index].load(cpp::MemoryOrder::RELAXED);
   }
@@ -179,9 +159,12 @@ template <bool InvertInbox> struct Process {
     return inverted_outbox;
   }
 
-  /// Determines if this process needs to wait for ownership of the buffer.
+  /// Determines if this process needs to wait for ownership of the buffer. We
+  /// invert the condition on one of the processes to indicate that if one
+  /// process owns the buffer then the other does not.
   LIBC_INLINE static bool buffer_unavailable(uint32_t in, uint32_t out) {
-    return in != out;
+    bool cond = in != out;
+    return Invert ? !cond : cond;
   }
 
   /// Attempt to claim the lock at index. Return true on lock taken.
@@ -279,12 +262,12 @@ template <bool InvertInbox> struct Process {
 
   /// Offset of the inbox in memory. This is the same as the outbox if inverted.
   LIBC_INLINE static uint64_t inbox_offset(uint64_t port_count) {
-    return InvertInbox ? mailbox_bytes(port_count) : 0;
+    return Invert ? mailbox_bytes(port_count) : 0;
   }
 
   /// Offset of the outbox in memory. This is the same as the inbox if inverted.
   LIBC_INLINE static uint64_t outbox_offset(uint64_t port_count) {
-    return InvertInbox ? 0 : mailbox_bytes(port_count);
+    return Invert ? 0 : mailbox_bytes(port_count);
   }
 
   /// Offset of the buffer containing the packets after the inbox and outbox.