formalize turning T_NEXUS into other node types

src/vdp_pesi.c

@@ -102,7 +102,7 @@ static const void * const gzip_fini = &gzip_fini;
 
 enum n_type {
 	T_INVALID = 0,
-	T_NEXUS,	// XXX can change into T_SUBREQ / T_FINAL
+	T_NEXUS,	// can change into T_SUBREQ / T_FINAL / T_DATA
 	T_DATA,
 	T_CRC,
 	T_SUBREQ,
@@ -593,6 +593,63 @@ node_insert(struct bytes_tree *tree, struct node *parent,
 	    tree->npending);
 }
 
+/*
+ * node mutation: turn an existing nexus into something else
+ *
+ * howto:
+ *
+ * 0) node must be T_NEXUS / ST_PRIVATE, untouched
+ *
+ * 1) call node_mutate_prep()
+ *
+ * 2) modify data in the destination-type specific struct
+ *
+ * 3) call node_mutate_lock() with destination type/state
+ *
+ * 4) optionally do more things under the lock
+ *
+ * 5) call node_mutate_unlock()
+ *
+ */
+static void
+node_mutate_prep(struct bytes_tree *tree, struct node *node)
+{
+	CHECK_OBJ_NOTNULL(node, NODE_MAGIC);
+	CHECK_OBJ_NOTNULL(node->parent, NODE_MAGIC);
+
+	assert(node->state == ST_PRIVATE);
+	assert(node->type == T_NEXUS);
+	assert(node != tree->root);
+
+	AZ(node->nexus.npending_private);
+	AZ(node->nexus.oc);
+
+	memset(&node->nexus, 0, sizeof node->nexus);
+}
+
+static inline void
+node_mutate_lock(struct bytes_tree *tree, struct node *node,
+		 enum n_type type, enum n_state state)
+{
+	/* these checks can be relexed when needed */
+	assert(type == T_DATA ||
+	       type == T_SUBREQ ||
+	       type == T_FINAL);
+	assert(state == ST_DATA);
+
+	Lck_Lock(&tree->tree_lock);
+	node->type = type;
+	node->state = state;
+	if (node->parent->state != ST_PRIVATE)
+		AZ(pthread_cond_signal(&tree->cond));
+}
+
+static inline void
+node_mutate_unlock(struct bytes_tree *tree)
+{
+	Lck_Unlock(&tree->tree_lock);
+}
+
 /*
  * CHK_ORDER: within child list can only transition from ST_DELIVERED -> !
  * ST_DELIVERED iow, once a child is found pending, all others must be also
@@ -986,19 +1043,12 @@ ved_task(struct worker *wrk, void *priv)
 		req_fini(&node->req, wrk);
 
 		/* turn into T_DATA node */
-		assert(node->state == ST_PRIVATE);
-		assert(node->type == T_NEXUS);
-		AZ(node->nexus.npending_private);
-		AZ(node->nexus.oc);
+		node_mutate_prep(pesi_tree->tree, node);
 		memset(&node->data, 0, sizeof node->data);
 
-		Lck_Lock(&pesi_tree->tree->tree_lock);
-		node->type = T_DATA;
-		node->state = ST_DATA;
-		AN(node->parent);
-		if (node->parent->state != ST_PRIVATE)
-			AZ(pthread_cond_signal(&pesi_tree->tree->cond));
-		Lck_Unlock(&pesi_tree->tree->tree_lock);
+		node_mutate_lock(pesi_tree->tree, node,
+		    T_DATA, ST_DATA);
+		node_mutate_unlock(pesi_tree->tree);
 
 		task_fini(pesi_tree, pesi);
 		break;
@@ -2602,31 +2652,25 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 	else if (is_final && !is_esi) {
 		VSLdbg(req, "vped_deliver: T_FINAL");
 
-		/* XXX WIP / STILL HACKY */
-		XXXAZ(push_vdps(req, &parent->nexus.gzip));
+		AZ(push_vdps(req, &parent->nexus.gzip));
 		AZ(VDP_Push(req, &vped_to_parent, parent->req));
 
-		/* XXX TODO CHANGE NODE TYPES -- this is a NEXUS atm */
-		assert(node->type == T_NEXUS);
-		assert(node->state == ST_PRIVATE);
-		assert(node->nexus.npending_private == 0);
-
-		/* XXX can reduce locking */
-		Lck_Lock(&tree->tree_lock);
-
-		node->type = T_FINAL;
-		node->state = ST_DATA;
+		node_mutate_prep(tree, node);
 
 		AZ(pthread_mutex_init(&node->final.fi_mtx, NULL));
 		AZ(pthread_cond_init(&node->final.fi_cond, NULL));
-
 		assert(node->final.fi_state == FI_READY);
 
-		if (parent->state != ST_PRIVATE)
-			AZ(pthread_cond_signal(&tree->cond));
+		node_mutate_lock(tree, node, T_FINAL, ST_DATA);
 
+		/*
+		 * we deliberately take the fi_mtx under the tree mutex (via
+		 * node_mutate_lock) to ensure unpending never gets unlocked
+		 * access to ensure we get the signal
+		 */
 		AZ(pthread_mutex_lock(&node->final.fi_mtx));
-		Lck_Unlock(&tree->tree_lock);
+
+		node_mutate_unlock(tree);
 
 		if (node->final.fi_state < FI_GO)
 			AZ(pthread_cond_wait(&node->final.fi_cond,
@@ -2646,21 +2690,12 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 	}
 	else if (! is_esi) {
 		VSLdbg(req, "vped_deliver: T_SUBREQ");
-		/* XXX WIP / STILL HACKY */
+
 		AZ(req->objcore->flags & OC_F_FINAL);
-		XXXAZ(push_vdps(req, &parent->nexus.gzip));
+		AZ(push_vdps(req, &parent->nexus.gzip));
 		AZ(VDP_Push(req, &vped_to_parent, parent->req));
 
-		/* XXX TODO CHANGE NODE TYPES -- this is a NEXUS atm */
-		assert(node->type == T_NEXUS);
-		assert(node->state == ST_PRIVATE);
-		assert(node->nexus.npending_private == 0);
-
-		Lck_Lock(&tree->tree_lock);
-		// XXX we should not need to lock here once node types are
-		// cleaned up
-		node->type = T_SUBREQ;
-		node->state = ST_DATA;
+		node_mutate_prep(tree, node);
 
 		/* our caller, cnt_transmit(), releases the refs to boc and oc
 		 * when we return, so in order to hand this request to the
@@ -2673,9 +2708,9 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 
 		AZ(pthread_cond_init(&node->subreq.cond, NULL));
 
-		if (parent->state != ST_PRIVATE)
-			AZ(pthread_cond_signal(&tree->cond));
-		Lck_Unlock(&tree->tree_lock);
+		node_mutate_lock(tree, node, T_SUBREQ, ST_DATA);
+		node_mutate_unlock(tree);
+
 		/*
 		 * to be done in the other thread:
 		 * - VDP_DeliverObj()