Use READ_ONCE() for seq counters (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 5 Apr 2018 17:37:14 +0000 (13:37 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 30 Apr 2018 18:36:28 +0000 (14:36 -0400)
READ_ONCE() is like ACCESS_ONCE(), just clearly a read.

There are concurrent writes to the field, ordering matters, etc.  In
seqctr_retry, it was possible for the compiler to generate two reads from
the data structure for the "is locked" and "!=" tests.  We might have had
strange results there.

The seq stuff might need a closer look.

Reinstall your kernel headers.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/ros/atomic.h

index f13d5a5..b37a50f 100644 (file)
@@ -31,11 +31,16 @@ static inline bool seqctr_retry(seq_ctr_t old_ctr, seq_ctr_t new_ctr);
  */
 static inline bool seq_is_locked(seq_ctr_t seq_ctr)
 {
-       return seq_ctr % 2;
+       return READ_ONCE(seq_ctr) % 2;
 }
 
 static inline bool seqctr_retry(seq_ctr_t old_ctr, seq_ctr_t new_ctr)
 {
+       seq_ctr_t old_val;
+
        rmb();  /* don't allow protected reads to reorder after the check */
-       return (seq_is_locked(old_ctr)) || (old_ctr != new_ctr);
+       /* Even though old_ctr is passed in, we might be inlined and can't guarantee
+        * old_ctr was in a register or otherwise won't be re-read. */
+       old_val = READ_ONCE(old_ctr);
+       return (seq_is_locked(old_val)) || (old_val != new_ctr);
 }