From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Fri, 7 Jun 2013 14:23:40 +0000 (-0400)
Subject: rcuja: remove "free_node_cb" from destroy
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=99e6e3dc384b72322e7d180c622ee1249345a27c;p=userspace-rcu.git

rcuja: remove "free_node_cb" from destroy

It is now up to the caller to ensure that the Judy array is emptied
before destroying it.

For instance, if nodes are accessible through another data structure
than the Judy array, it may very well be valid to destroy the Judy array
without freeing memory associated to those nodes.

Destroy still takes care of freeing memory used for Judy internal nodes.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---

diff --git a/rcuja/rcuja-internal.h b/rcuja/rcuja-internal.h
index 2d3f131..8f5f375 100644
--- a/rcuja/rcuja-internal.h
+++ b/rcuja/rcuja-internal.h
@@ -173,8 +173,7 @@ unsigned long ja_node_type(struct cds_ja_inode_flag *node);
 
 __attribute__((visibility("protected")))
 void rcuja_free_all_children(struct cds_ja_shadow_node *shadow_node,
-		struct cds_ja_inode_flag *node_flag,
-		void (*free_node_cb)(struct cds_ja_node *node));
+		struct cds_ja_inode_flag *node_flag);
 
 __attribute__((visibility("protected")))
 struct cds_ja_shadow_node *rcuja_shadow_lookup_lock(struct cds_lfht *ht,
@@ -203,8 +202,7 @@ int rcuja_shadow_clear(struct cds_lfht *ht,
 
 __attribute__((visibility("protected")))
 void rcuja_shadow_prune(struct cds_lfht *ht,
-		unsigned int flags,
-		void (*free_node_cb)(struct cds_ja_node *node));
+		unsigned int flags);
 
 __attribute__((visibility("protected")))
 struct cds_lfht *rcuja_create_ht(const struct rcu_flavor_struct *flavor);
@@ -220,16 +218,8 @@ void free_cds_ja_node(struct cds_ja *ja, struct cds_ja_inode *node);
  * Receives a struct cds_ja_node * as parameter, which is used as start
  * of duplicate list and loop cursor.
  */
-#define cds_ja_for_each_duplicate(pos)					\
-	for (; (pos) != NULL; (pos) = (pos)->next)
-
-/*
- * Iterate through duplicates returned by cds_ja_lookup*()
- * Safe against removal of entries during traversal.
- */
-#define cds_ja_for_each_duplicate_safe(_pos, _next)			\
-	for (; (_pos) != NULL ? ((_next) = (_pos)->next, 1) : 0;	\
-		(_pos) = (_next))
+#define cds_ja_for_each_duplicate(pos)				\
+       for (; (pos) != NULL; (pos) = (pos)->next)
 
 //#define DEBUG
 
diff --git a/rcuja/rcuja-shadow-nodes.c b/rcuja/rcuja-shadow-nodes.c
index 718c082..11ada39 100644
--- a/rcuja/rcuja-shadow-nodes.c
+++ b/rcuja/rcuja-shadow-nodes.c
@@ -377,8 +377,7 @@ rcu_unlock:
  */
 __attribute__((visibility("protected")))
 void rcuja_shadow_prune(struct cds_lfht *ht,
-		unsigned int flags,
-		void (*free_node_cb)(struct cds_ja_node *node))
+		unsigned int flags)
 {
 	const struct rcu_flavor_struct *flavor;
 	struct cds_ja_shadow_node *shadow_node;
@@ -400,11 +399,6 @@ void rcuja_shadow_prune(struct cds_lfht *ht,
 			goto unlock;
 		if ((flags & RCUJA_SHADOW_CLEAR_FREE_NODE)
 				&& shadow_node->level) {
-			if (shadow_node->level == shadow_node->ja->tree_depth - 1) {
-				rcuja_free_all_children(shadow_node,
-						shadow_node->node_flag,
-						free_node_cb);
-			}
 			if (flags & RCUJA_SHADOW_CLEAR_FREE_LOCK) {
 				flavor->update_call_rcu(&shadow_node->head,
 					free_shadow_node_and_node_and_lock);
diff --git a/rcuja/rcuja.c b/rcuja/rcuja.c
index 364570e..fb3e9b1 100644
--- a/rcuja/rcuja.c
+++ b/rcuja/rcuja.c
@@ -2646,91 +2646,6 @@ ja_error:
 	return NULL;
 }
 
-/*
- * Called from RCU read-side CS.
- */
-__attribute__((visibility("protected")))
-void rcuja_free_all_children(struct cds_ja_shadow_node *shadow_node,
-		struct cds_ja_inode_flag *node_flag,
-		void (*rcu_free_node)(struct cds_ja_node *node))
-{
-	unsigned int type_index;
-	struct cds_ja_inode *node;
-	const struct cds_ja_type *type;
-
-	node = ja_node_ptr(node_flag);
-	assert(node != NULL);
-	type_index = ja_node_type(node_flag);
-	type = &ja_types[type_index];
-
-	switch (type->type_class) {
-	case RCU_JA_LINEAR:
-	{
-		uint8_t nr_child =
-			ja_linear_node_get_nr_child(type, node);
-		unsigned int i;
-
-		for (i = 0; i < nr_child; i++) {
-			struct cds_ja_inode_flag *iter;
-			struct cds_ja_node *node_iter, *n;
-			uint8_t v;
-
-			ja_linear_node_get_ith_pos(type, node, i, &v, &iter);
-			node_iter = (struct cds_ja_node *) iter;
-			cds_ja_for_each_duplicate_safe(node_iter, n) {
-				rcu_free_node(node_iter);
-			}
-		}
-		break;
-	}
-	case RCU_JA_POOL:
-	{
-		unsigned int pool_nr;
-
-		for (pool_nr = 0; pool_nr < (1U << type->nr_pool_order); pool_nr++) {
-			struct cds_ja_inode *pool =
-				ja_pool_node_get_ith_pool(type, node, pool_nr);
-			uint8_t nr_child =
-				ja_linear_node_get_nr_child(type, pool);
-			unsigned int j;
-
-			for (j = 0; j < nr_child; j++) {
-				struct cds_ja_inode_flag *iter;
-				struct cds_ja_node *node_iter, *n;
-				uint8_t v;
-
-				ja_linear_node_get_ith_pos(type, pool, j, &v, &iter);
-				node_iter = (struct cds_ja_node *) iter;
-				cds_ja_for_each_duplicate_safe(node_iter, n) {
-					rcu_free_node(node_iter);
-				}
-			}
-		}
-		break;
-	}
-	case RCU_JA_NULL:
-		break;
-	case RCU_JA_PIGEON:
-	{
-		unsigned int i;
-
-		for (i = 0; i < JA_ENTRY_PER_NODE; i++) {
-			struct cds_ja_inode_flag *iter;
-			struct cds_ja_node *node_iter, *n;
-
-			iter = ja_pigeon_node_get_ith_pos(type, node, i);
-			node_iter = (struct cds_ja_node *) iter;
-			cds_ja_for_each_duplicate_safe(node_iter, n) {
-				rcu_free_node(node_iter);
-			}
-		}
-		break;
-	}
-	default:
-		assert(0);
-	}
-}
-
 static
 void print_debug_fallback_distribution(struct cds_ja *ja)
 {
@@ -2780,16 +2695,14 @@ int ja_final_checks(struct cds_ja *ja)
  * on the Judy array while it is being destroyed (ensured by the
  * caller).
  */
-int cds_ja_destroy(struct cds_ja *ja,
-		void (*free_node_cb)(struct cds_ja_node *node))
+int cds_ja_destroy(struct cds_ja *ja)
 {
 	const struct rcu_flavor_struct *flavor;
 	int ret;
 
 	flavor = cds_lfht_rcu_flavor(ja->ht);
 	rcuja_shadow_prune(ja->ht,
-		RCUJA_SHADOW_CLEAR_FREE_NODE | RCUJA_SHADOW_CLEAR_FREE_LOCK,
-		free_node_cb);
+		RCUJA_SHADOW_CLEAR_FREE_NODE | RCUJA_SHADOW_CLEAR_FREE_LOCK);
 	flavor->thread_offline();
 	ret = rcuja_delete_ht(ja->ht);
 	if (ret)
diff --git a/tests/test_urcu_ja.c b/tests/test_urcu_ja.c
index 5dd1661..7de723e 100644
--- a/tests/test_urcu_ja.c
+++ b/tests/test_urcu_ja.c
@@ -433,7 +433,7 @@ int test_8bit_key(void)
 		return -1;
 	}
 
-	ret = cds_ja_destroy(test_ja, free_node);
+	ret = cds_ja_destroy(test_ja);
 	if (ret) {
 		fprintf(stderr, "Error destroying judy array\n");
 		return -1;
@@ -632,7 +632,7 @@ int test_16bit_key(void)
 		return -1;
 	}
 
-	ret = cds_ja_destroy(test_ja, free_node);
+	ret = cds_ja_destroy(test_ja);
 	if (ret) {
 		fprintf(stderr, "Error destroying judy array\n");
 		return -1;
@@ -770,7 +770,7 @@ int test_sparse_key(unsigned int bits, int nr_dup)
 		return -1;
 	}
 
-	ret = cds_ja_destroy(test_ja, free_node);
+	ret = cds_ja_destroy(test_ja);
 	if (ret) {
 		fprintf(stderr, "Error destroying judy array\n");
 		return -1;
@@ -1122,7 +1122,7 @@ int do_mt_test(void)
 		return -1;
 	}
 
-	ret = cds_ja_destroy(test_ja, free_node);
+	ret = cds_ja_destroy(test_ja);
 	if (ret) {
 		fprintf(stderr, "Error destroying judy array\n");
 		goto end;
diff --git a/urcu/rcuja.h b/urcu/rcuja.h
index 7546332..aafa4cf 100644
--- a/urcu/rcuja.h
+++ b/urcu/rcuja.h
@@ -155,18 +155,14 @@ struct cds_ja *cds_ja_new(unsigned int key_bits)
 /*
  * cds_ja_destroy - Destroy a Judy array.
  * @ja: the Judy array.
- * @rcu_free_node_cb: callback performing memory free of leftover nodes.
  *
  * Returns 0 on success, negative error value on error.
  * There should be no more concurrent add, delete, nor look-up performed
  * on the Judy array while it is being destroyed (ensured by the caller).
- * There is no need for the @rcu_free_node_cb callback to wait for grace
- * periods, since there are no more concurrent users of the Judy array.
  * RCU read-side lock should _not_ be held when calling this function,
  * however, QSBR threads need to be online.
  */
-int cds_ja_destroy(struct cds_ja *ja,
-		void (*free_node_cb)(struct cds_ja_node *node));
+int cds_ja_destroy(struct cds_ja *ja);
 
 /*
  * cds_ja_for_each_duplicate_rcu: Iterate through duplicates.
@@ -178,17 +174,17 @@ int cds_ja_destroy(struct cds_ja *ja,
  * of duplicate list and loop cursor.
  * _NOT_ safe against node removal within iteration.
  */
-#define cds_ja_for_each_duplicate_rcu(pos)				\
+#define cds_ja_for_each_duplicate_rcu(pos)					\
 	for (; (pos) != NULL; (pos) = rcu_dereference((pos)->next))
 
 /*
- * cds_ja_for_each_duplicate_safe_rcu: Iterate through duplicates.
+ * cds_ja_for_each_duplicate_safe: Iterate through duplicates.
  * @pos: struct cds_ja_node *, start of duplicate list and loop cursor.
  * @p: struct cds_ja_node *, temporary pointer to next.
  *
- * Iterate through duplicates returned by cds_ja_lookup*()
- * This must be done while rcu_read_lock() is held.
+ * Iterate through duplicates returned by cds_ja_lookup*().
  * Safe against node removal within iteration.
+ * This must be done while rcu_read_lock() is held.
  */
 #define cds_ja_for_each_duplicate_safe_rcu(pos, p)			\
 	for (; (pos) != NULL ?						\