From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Fri, 7 Aug 2020 03:08:26 +0000 (-0700)
Subject: remove useless spin lock in lib/ast
X-Git-Tag: 2.46.0~20^2^2~127^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dcc5537ce3242ebfd6b8195c7950cfe40c1fd121;p=graphviz

remove useless spin lock in lib/ast

This spin lock incorrectly implied to the reader that this code was thread safe.
IMHO there are two primary issues with this spin lock and one secondary issue:

  1. The intention of the spin lock is clearly to provide mutual exclusion
     within fmtbuf(). However this function returns a pointer to a static
     (thread-global) buffer. Excluding two threads from being in this function
     concurrently does not help if both exit the function with pointers into the
     same memory.

  2. The spin lock uses no atomics or volatile. As a result, accesses to the
     lock can be freely rearranged or coalesced, with no regard to thread
     interleaving. This can result in one thread never seeing another thread's
     update (unlock) to the lock. Even worse, my local compiler simply
     recognizes what is happening and omits the lock operations entirely.

  3. (secondary) If this lock worked as intended, it would spin writing to the
     lock variable. This would cause fairly pessimistic cache line bouncing as
     all waiting processors try to keep the line containing the lock in
     exclusive state in their local cache. A more performant way to achieve
     this is to spin waiting to see the lock value change *without* modifying
     the lock yourself. This allows multiple waiting processors to have the line
     in shared state, and minimizes cache coherence traffic.
---

diff --git a/lib/ast/fmtbuf.c b/lib/ast/fmtbuf.c
index a9b3d47c4..1d64e825f 100644
--- a/lib/ast/fmtbuf.c
+++ b/lib/ast/fmtbuf.c
@@ -16,24 +16,19 @@
 
 /*
  * return small format buffer chunk of size n
- * spin lock for thread access
  * format buffers are short lived
  */
 
 static char buf[16 * 1024];
 static char *nxt = buf;
-static int lck = -1;
 
 char *fmtbuf(size_t n)
 {
     register char *cur;
 
-    while (++lck)
-	lck--;
     if (n > (&buf[elementsof(buf)] - nxt))
 	nxt = buf;
     cur = nxt;
     nxt += n;
-    lck--;
     return cur;
 }