X-Git-Url: https://git.lttng.org/?a=blobdiff_plain;f=urcu%2Fstatic%2Frculfstack.h;h=9d69fa736996c4dfc704bbffc88d9f14395344c8;hb=79dba87985c7dda26ae56e4c30451acea6b00bdd;hp=3f48b7eabbc33c10a626400fb28dc6d4545c4776;hpb=c5f52d0c6b1890f0c66f172665ae6793634f5287;p=urcu.git

diff --git a/urcu/static/rculfstack.h b/urcu/static/rculfstack.h
index 3f48b7e..9d69fa7 100644
--- a/urcu/static/rculfstack.h
+++ b/urcu/static/rculfstack.h
@@ -27,7 +27,6 @@
  */
 
 #include <urcu/uatomic.h>
-/* A urcu implementation header should be already included. */
 
 #ifdef __cplusplus
 extern "C" {
@@ -44,6 +43,28 @@ void _cds_lfs_init_rcu(struct cds_lfs_stack_rcu *s)
 	s->head = NULL;
 }
 
+/*
+ * Lock-free stack push is not subject to ABA problem, so no need to
+ * take the RCU read-side lock. Even if "head" changes between two
+ * uatomic_cmpxchg() invocations here (being popped, and then pushed
+ * again by one or more concurrent threads), the second
+ * uatomic_cmpxchg() invocation only cares about pushing a new entry at
+ * the head of the stack, ensuring consistency by making sure the new
+ * node->next is the same pointer value as the value replaced as head.
+ * It does not care about the content of the actual next node, so it can
+ * very well be reallocated between the two uatomic_cmpxchg().
+ *
+ * We take the approach of expecting the stack to be usually empty, so
+ * we first try an initial uatomic_cmpxchg() on a NULL old_head, and
+ * retry if the old head was non-NULL (the value read by the first
+ * uatomic_cmpxchg() is used as old head for the following loop). The
+ * upside of this scheme is to minimize the amount of cacheline traffic,
+ * always performing an exclusive cacheline access, rather than doing
+ * non-exclusive followed by exclusive cacheline access (which would be
+ * required if we first read the old head value). This design decision
+ * might be revisited after more throrough benchmarking on various
+ * platforms.
+ */
 static inline
 void _cds_lfs_push_rcu(struct cds_lfs_stack_rcu *s, struct cds_lfs_node_rcu *node)
 {
@@ -64,7 +85,7 @@ void _cds_lfs_push_rcu(struct cds_lfs_stack_rcu *s, struct cds_lfs_node_rcu *nod
 }
 
 /*
- * Should be called under rcu read-side lock.
+ * Acts as a RCU reader.
  *
  * The caller must wait for a grace period to pass before freeing the returned
  * node or modifying the cds_lfs_node_rcu structure.
@@ -77,18 +98,22 @@ _cds_lfs_pop_rcu(struct cds_lfs_stack_rcu *s)
 	for (;;) {
 		struct cds_lfs_node_rcu *head;
 
+		rcu_read_lock();
 		head = rcu_dereference(s->head);
 		if (head) {
 			struct cds_lfs_node_rcu *next = rcu_dereference(head->next);
 
 			if (uatomic_cmpxchg(&s->head, head, next) == head) {
+				rcu_read_unlock();
 				return head;
 			} else {
 				/* Concurrent modification. Retry. */
+				rcu_read_unlock();
 				continue;
 			}
 		} else {
 			/* Empty stack */
+			rcu_read_unlock();
 			return NULL;
 		}
 	}