]> git.ipfire.org Git - thirdparty/bind9.git/commit
Convert all atomic operations in isc_rwlock to release-acquire memory ordering
authorOndřej Surý <ondrej@isc.org>
Sat, 1 Feb 2020 09:48:20 +0000 (10:48 +0100)
committerOndřej Surý <ondrej@isc.org>
Tue, 11 Feb 2020 10:50:04 +0000 (11:50 +0100)
commit3c8a441a0e666eeecfceb9a5364eb0794ceae498
treea21d79d0119b888d15ea2a7c90bea6e0d9115e0c
parent1da0994ea4dd09dbf8b503bfa08a6f874e24978c
Convert all atomic operations in isc_rwlock to release-acquire memory ordering

The memory ordering in the rwlock was all wrong, I am copying excerpts
from the https://en.cppreference.com/w/c/atomic/memory_order#Relaxed_ordering
for the convenience of the reader:

  Relaxed ordering

  Atomic operations tagged memory_order_relaxed are not synchronization
  operations; they do not impose an order among concurrent memory
  accesses. They only guarantee atomicity and modification order
  consistency.

  Release-Acquire ordering

  If an atomic store in thread A is tagged memory_order_release and an
  atomic load in thread B from the same variable is tagged
  memory_order_acquire, all memory writes (non-atomic and relaxed atomic)
  that happened-before the atomic store from the point of view of thread
  A, become visible side-effects in thread B. That is, once the atomic
  load is completed, thread B is guaranteed to see everything thread A
  wrote to memory.

  The synchronization is established only between the threads releasing
  and acquiring the same atomic variable. Other threads can see different
  order of memory accesses than either or both of the synchronized
  threads.

Which basically means that we had no or weak synchronization between
threads using the same variables in the rwlock structure.  There should
not be a significant performance drop because the critical sections were
already protected by:

  while(1) {
    if (relaxed_atomic_operation) {
      break;
    }
    LOCK(lock);
    if (!relaxed_atomic_operation) {
      WAIT(sem, lock);
    }
    UNLOCK(lock)l
  }

I would add one more thing to "Don't do your own crypto, folks.":

  - Also don't do your own locking, folks.
lib/isc/rwlock.c