[libc-commits] [PATCH] D150365: [libc][NFC] Simplifly inbox and outbox state handling

[Joseph Huber via Phabricator via libc-commits]

jhuber6 created this revision.
jhuber6 added reviewers: tianshilei1992, jdoerfert, JonChesterfield, sivachandra.
Herald added subscribers: libc-commits, ecnelises, tschuett.
Herald added projects: libc-project, All.
jhuber6 requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150365

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


Index: libc/src/__support/RPC/rpc.h
===================================================================
--- libc/src/__support/RPC/rpc.h
+++ libc/src/__support/RPC/rpc.h
@@ -77,27 +77,18 @@
 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 ProcessIsServer> struct Process {
   LIBC_INLINE Process() = default;
   LIBC_INLINE Process(const Process &) = delete;
   LIBC_INLINE Process &operator=(const Process &) = delete;
@@ -120,9 +111,9 @@
     this->port_count = port_count;
     this->lane_size = lane_size;
     this->inbox = reinterpret_cast<cpp::Atomic<uint32_t> *>(
-        static_cast<char *>(state) + (InvertInbox ? s : p));
+        static_cast<char *>(state) + (ProcessIsServer ? s : p));
     this->outbox = reinterpret_cast<cpp::Atomic<uint32_t> *>(
-        static_cast<char *>(state) + (InvertInbox ? p : s));
+        static_cast<char *>(state) + (ProcessIsServer ? p : s));
     this->packet = reinterpret_cast<Packet *>(static_cast<char *>(state) +
                                               memory_offset_buffer(port_count));
   }
@@ -149,23 +140,12 @@
                          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);
   }
@@ -180,9 +160,11 @@
     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. The
+  /// client owns the buffer if the inbox and outbox bits are equal. Conversely,
+  /// the server owns the buffer if the inbox and outbox bits are not equal.
   LIBC_INLINE static bool buffer_unavailable(uint32_t in, uint32_t out) {
-    return in != out;
+    return ProcessIsServer ? in == out : in != out;
   }
 
   /// Attempt to claim the lock at index. Return true on lock taken.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150365.521299.patch
Type: text/x-patch
Size: 4987 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230511/7d580fed/attachment-0001.bin>