Merge "Remove ambiguous SeqLock comments." into main am: 510d403f3d

Original change: https://android-review.googlesource.com/c/platform/hardware/google/aemu/+/2773067

Change-Id: I1eb04d74337ad03083bf7c5d11eaf468e891f3b2
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

1 files changed, 2 insertions, 52 deletions

diff --git a/base/include/aemu/base/synchronization/Lock.h b/base/include/aemu/base/synchronization/Lock.h
index 62d13a3..2b88f05 100644
--- a/base/include/aemu/base/synchronization/Lock.h
+++ b/base/include/aemu/base/synchronization/Lock.h
@@ -230,57 +230,8 @@ private:
     DISALLOW_COPY_ASSIGN_AND_MOVE(AutoReadLock);
 };
 
-// Seqlock (cross platform)
-// Based on:
-// https://lwn.net/Articles/21812/
-// https://github.com/rigtorp/Seqlock
-//
-// A seqlock is meant to address performance issues with using reader/writer
-// locks to protect data structures where the time spent performing operations
-// while the lock is held is very short or even comparable to the time spent
-// locking/unlocking in the first place. This is very common in situations
-// where we have some globally accessible array of objects and multiple threads
-// performing short little read/write operations on them (i.e., pretty much
-// anything that uses entity component system architecture that needs to be
-// accessed by multiple threads).
-//
-// The basic idea of a seqlock is to store a sequence number (like a version
-// number) that writers increment, but readers only read. When beginning write
-// access, the sequence number is incremented, and after write access ends, the
-// sequence number is incremented again. This way, when a reader is trying to
-// read and it notices a change in the sequence number (or, as an optimization,
-// that the number is odd (because writes should always end up incrementing the
-// sequence number by 2 if they complete)), it can try again until there is no
-// change.
-//
-// The problem, however, is that we need to be very careful about how we set
-// and compare the sequence numbers, because compilers/hardware easily reorder
-// instructions involving what seems to be just simple integer arithmetic.
-// (see https://www.hpl.hp.com/techreports/2012/HPL-2012-68.pdf) Atomic
-// primitives need to be used for all accesses to the sequence number.
-//
-// In particular, the atomic updates to the sequence number and the actual
-// non-atomic data accesses are allowed to be reordered by the compiler, which
-// introduces problems when accessing the data (still allowing reads of an
-// update in progress); we need smp_rmb.
-// https://elixir.bootlin.com/linux/latest/source/tools/arch/arm64/include/asm/barrier.h#L25
-//
-// arm64: memory barrier instruction
-// asm volatile("dmb ishld" ::: "memory")
-// x86: compiler barrier
-// std::atomic_signal_fence(std::memory_order_acq_rel);
-//
-// This smp_rmb needs to be added before and after the read operation.
-//
-// On the write side, we use 
-// arm64: memory barrier instruction
-// asm volatile("dmb ishst" ::: "memory")
-// x86: compiler barrier
-// std::atomic_signal_fence(std::memory_order_acq_rel);
-// 
-// https://github.com/rigtorp/Seqlock has a version that seems to address these issues, while
-// https://elixir.bootlin.com/linux/latest/source/include/linux/seqlock.h shows how to implement in the kernel.
-//
+// Sequence lock implementation
+// See https://en.wikipedia.org/wiki/Seqlock for more info
 static inline __attribute__((always_inline)) void SmpWmb() {
 #if defined(__aarch64__)
         asm volatile("dmb ishst" ::: "memory");
@@ -326,7 +277,6 @@ public:
     uint32_t beginRead() {
         uint32_t res;
 
-        // see https://elixir.bootlin.com/linux/latest/source/include/linux/seqlock.h#L128; if odd we definitely know there's a write in progress, and shouldn't proceed any further.
 repeat:
         res = mSeq.load(std::memory_order_acquire);
         if (SEQLOCK_UNLIKELY(res & 1)) {