Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Feb 2025 20:20:07 GMT
From:      Doug Moore <dougm@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 90710f4a3473 - main - pctrie: add parent pointer to nodes
Message-ID:  <202502102020.51AKK7Ig036651@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by dougm:

URL: https://cgit.FreeBSD.org/src/commit/?id=90710f4a3473f7cf0dd1cfa17a7c3176cc471c71

commit 90710f4a3473f7cf0dd1cfa17a7c3176cc471c71
Author:     Doug Moore <dougm@FreeBSD.org>
AuthorDate: 2025-02-10 20:17:28 +0000
Commit:     Doug Moore <dougm@FreeBSD.org>
CommitDate: 2025-02-10 20:17:28 +0000

    pctrie: add parent pointer to nodes
    
    Add a parent pointer field to the pctrie_node struct, and maintain
    it as part of inserting and removing pctrie items.
    
    Much code is written to maintain one, or an array, of trailing
    pointers; that maintenance can be dropped with parent pointers
    available. Code affected includes remove, reclaim, lookup_le, and
    lookup_ge. In places, functions written once for iterator-free
    searching and again for iterator-maintaining searching can be replaced
    with a single function that serves both purposes.
    
    Fix an overflow error in lower-bound testing for lookup_le.
    
    Reviewed by:    markj
    Tested by:      pho
    Differential Revision:  https://reviews.freebsd.org/D48588
---
 sys/kern/subr_pctrie.c | 656 ++++++++++++++++---------------------------------
 sys/sys/pctrie.h       | 162 +++++-------
 2 files changed, 268 insertions(+), 550 deletions(-)

diff --git a/sys/kern/subr_pctrie.c b/sys/kern/subr_pctrie.c
index a078f0587f37..16690c3521bf 100644
--- a/sys/kern/subr_pctrie.c
+++ b/sys/kern/subr_pctrie.c
@@ -81,6 +81,7 @@ struct pctrie_node {
 	uint64_t	pn_owner;			/* Owner of record. */
 	pn_popmap_t	pn_popmap;			/* Valid children. */
 	uint8_t		pn_clev;			/* Level * WIDTH. */
+	smr_pctnode_t	pn_parent;			/* Parent node. */
 	smr_pctnode_t	pn_child[PCTRIE_COUNT];		/* Child nodes. */
 };
 
@@ -107,28 +108,6 @@ pctrie_keybarr(struct pctrie_node *node, uint64_t index, int *slot)
 	return (false);
 }
 
-/*
- * Check radix node.
- */
-static __inline void
-pctrie_node_put(struct pctrie_node *node)
-{
-#ifdef INVARIANTS
-	int slot;
-
-	KASSERT(powerof2(node->pn_popmap),
-	    ("pctrie_node_put: node %p has too many children %04x", node,
-	    node->pn_popmap));
-	for (slot = 0; slot < PCTRIE_COUNT; slot++) {
-		if ((node->pn_popmap & (1 << slot)) != 0)
-			continue;
-		KASSERT(smr_unserialized_load(&node->pn_child[slot], true) ==
-		    PCTRIE_NULL,
-		    ("pctrie_node_put: node %p has a child", node));
-	}
-#endif
-}
-
 enum pctrie_access { PCTRIE_SMR, PCTRIE_LOCKED, PCTRIE_UNSERIALIZED };
 
 /*
@@ -185,6 +164,16 @@ pctrie_root_load(struct pctrie *ptree, smr_t smr, enum pctrie_access access)
 	return (pctrie_node_load(pctrie_root(ptree), smr, access));
 }
 
+/*
+ * Get the child of a node.
+ */
+static __inline smr_pctnode_t *
+pctrie_child(struct pctrie *ptree, struct pctrie_node *node, uint64_t index)
+{
+	return (node == NULL ? pctrie_root(ptree) :
+	    &node->pn_child[pctrie_slot(node, index)]);
+}
+
 /*
  * Returns TRUE if the specified node is a leaf and FALSE otherwise.
  */
@@ -221,6 +210,24 @@ pctrie_toptr(struct pctrie_node *node, int keyoff)
 	return ((void *)(((uintptr_t)node & ~PCTRIE_FLAGS) - keyoff));
 }
 
+/*
+ * Make 'parent' a parent of 'child'.
+ */
+static __inline void
+pctrie_setparent(struct pctrie_node *child, struct pctrie_node *parent)
+{
+	pctrie_node_store(&child->pn_parent, parent, PCTRIE_UNSERIALIZED);
+}
+
+/*
+ * Return the parent of 'node'.
+ */
+static __inline struct pctrie_node *
+pctrie_parent(struct pctrie_node *node)
+{
+	return (pctrie_node_load(&node->pn_parent, NULL, PCTRIE_UNSERIALIZED));
+}
+
 /*
  * Make 'child' a child of 'node'.
  */
@@ -260,12 +267,6 @@ pctrie_node_size(void)
 	return (sizeof(struct pctrie_node));
 }
 
-enum pctrie_insert_neighbor_mode {
-	PCTRIE_INSERT_NEIGHBOR_NONE,
-	PCTRIE_INSERT_NEIGHBOR_LT,
-	PCTRIE_INSERT_NEIGHBOR_GT,
-};
-
 /*
  * Look for where to insert the key-value pair into the trie.  Complete the
  * insertion if it replaces a null leaf.  Return the insertion location if the
@@ -273,19 +274,10 @@ enum pctrie_insert_neighbor_mode {
  *
  * If the key is already present in the trie, populate *found_out as if by
  * pctrie_lookup().
- *
- * With mode PCTRIE_INSERT_NEIGHBOR_GT or PCTRIE_INSERT_NEIGHBOR_LT, set
- * *neighbor_out to the lowest level node we encounter during the insert lookup
- * that is a parent of the next greater or lesser entry.  The value is not
- * defined if the key was already present in the trie.
- *
- * Note that mode is expected to be a compile-time constant, and this procedure
- * is expected to be inlined into callers with extraneous code optimized out.
  */
 static __always_inline void *
 pctrie_insert_lookup_compound(struct pctrie *ptree, uint64_t *val,
-    uint64_t **found_out, struct pctrie_node **neighbor_out,
-    enum pctrie_insert_neighbor_mode mode)
+    struct pctrie_node **parent_out, uint64_t **found_out)
 {
 	uint64_t index;
 	struct pctrie_node *node, *parent;
@@ -308,51 +300,29 @@ pctrie_insert_lookup_compound(struct pctrie *ptree, uint64_t *val,
 				else
 					pctrie_addnode(parent, index,
 					    pctrie_toleaf(val), PCTRIE_LOCKED);
+				*parent_out = parent;
 				return (NULL);
 			}
 			if (*pctrie_toval(node) == index) {
 				*found_out = pctrie_toval(node);
+				*parent_out = parent;
 				return (NULL);
 			}
 			break;
 		}
 		if (pctrie_keybarr(node, index, &slot))
 			break;
-		/*
-		 * Descend.  If we're tracking the next neighbor and this node
-		 * contains a neighboring entry in the right direction, record
-		 * it.
-		 */
-		if (mode == PCTRIE_INSERT_NEIGHBOR_LT) {
-			if ((node->pn_popmap & ((1 << slot) - 1)) != 0)
-				*neighbor_out = node;
-		} else if (mode == PCTRIE_INSERT_NEIGHBOR_GT) {
-			if ((node->pn_popmap >> slot) > 1)
-				*neighbor_out = node;
-		}
 		parent = node;
 		node = pctrie_node_load(&node->pn_child[slot], NULL,
 		    PCTRIE_LOCKED);
 	}
 
-	/*
-	 * The caller will split this node.  If we're tracking the next
-	 * neighbor, record the old node if the old entry is in the right
-	 * direction.
-	 */
-	if (mode == PCTRIE_INSERT_NEIGHBOR_LT) {
-		if (*pctrie_toval(node) < index)
-			*neighbor_out = node;
-	} else if (mode == PCTRIE_INSERT_NEIGHBOR_GT) {
-		if (*pctrie_toval(node) > index)
-			*neighbor_out = node;
-	}
-
 	/*
 	 * 'node' must be replaced in the tree with a new branch node, with
 	 * children 'node' and 'val'. Return the place that points to 'node'
 	 * now, and will point to to the new branching node later.
 	 */
+	*parent_out = parent;
 	return ((parent == NULL) ? pctrie_root(ptree): &parent->pn_child[slot]);
 }
 
@@ -361,14 +331,15 @@ pctrie_insert_lookup_compound(struct pctrie *ptree, uint64_t *val,
  * if the key already exists, and do not look for neighboring entries.
  */
 void *
-pctrie_insert_lookup_strict(struct pctrie *ptree, uint64_t *val)
+pctrie_insert_lookup_strict(struct pctrie *ptree, uint64_t *val,
+    struct pctrie_node **parent_out)
 {
 	void *parentp;
 	uint64_t *found;
 
 	found = NULL;
-	parentp = pctrie_insert_lookup_compound(ptree, val, &found, NULL,
-	    PCTRIE_INSERT_NEIGHBOR_NONE);
+	parentp = pctrie_insert_lookup_compound(ptree, val, parent_out,
+	    &found);
 	if (__predict_false(found != NULL))
 		panic("%s: key %jx is already present", __func__,
 		    (uintmax_t)*val);
@@ -381,71 +352,47 @@ pctrie_insert_lookup_strict(struct pctrie *ptree, uint64_t *val)
  */
 void *
 pctrie_insert_lookup(struct pctrie *ptree, uint64_t *val,
-    uint64_t **found_out)
-{
-	*found_out = NULL;
-	return (pctrie_insert_lookup_compound(ptree, val, found_out, NULL,
-	    PCTRIE_INSERT_NEIGHBOR_NONE));
-}
-
-/*
- * Wrap pctrie_insert_lookup_compound to implement find or insert and find next
- * greater entry.  Find a subtree that contains the next entry greater than the
- * newly-inserted or to-be-inserted entry.
- */
-void *
-pctrie_insert_lookup_gt(struct pctrie *ptree, uint64_t *val,
-    uint64_t **found_out, struct pctrie_node **neighbor_out)
+    struct pctrie_node **parent_out, uint64_t **found_out)
 {
 	*found_out = NULL;
-	*neighbor_out = NULL;
-	return (pctrie_insert_lookup_compound(ptree, val, found_out,
-	    neighbor_out, PCTRIE_INSERT_NEIGHBOR_GT));
+	return (pctrie_insert_lookup_compound(ptree, val, parent_out,
+	    found_out));
 }
 
 /*
- * Wrap pctrie_insert_lookup_compound to implement find or insert and find next
- * lesser entry.  Find a subtree that contains the next entry less than the
- * newly-inserted or to-be-inserted entry.
- */
-void *
-pctrie_insert_lookup_lt(struct pctrie *ptree, uint64_t *val,
-    uint64_t **found_out, struct pctrie_node **neighbor_out)
-{
-	*found_out = NULL;
-	*neighbor_out = NULL;
-	return (pctrie_insert_lookup_compound(ptree, val, found_out,
-	    neighbor_out, PCTRIE_INSERT_NEIGHBOR_LT));
-}
-
-/*
- * Uses new node to insert key-value pair into the trie at given location.
+ * Inserts newly allocated node 'child' into trie at location 'parentp', with
+ * parent 'parent' and two children, 'val' and whatever non-NULL node or leaf
+ * was at 'parentp' to begin with.
  */
 void
-pctrie_insert_node(void *parentp, struct pctrie_node *parent, uint64_t *val)
+pctrie_insert_node(uint64_t *val, struct pctrie_node *parent, void *parentp,
+    struct pctrie_node *child)
 {
 	struct pctrie_node *node;
 	uint64_t index, newind;
 
 	/*
-	 * Clear the last child pointer of the newly allocated parent.  We want
+	 * Clear the last child pointer of the newly allocated child.  We want
 	 * to clear it after the final section has exited so lookup can not
 	 * return false negatives.  It is done here because it will be
 	 * cache-cold in the dtor callback.
 	 */
-	if (parent->pn_popmap != 0) {
-		pctrie_node_store(&parent->pn_child[ffs(parent->pn_popmap) - 1],
+	if (child->pn_popmap != 0) {
+		pctrie_node_store(&child->pn_child[ffs(child->pn_popmap) - 1],
 		    PCTRIE_NULL, PCTRIE_UNSERIALIZED);
-		parent->pn_popmap = 0;
+		child->pn_popmap = 0;
 	}
 
 	/*
-	 * Recover the values of the two children of the new parent node.  If
+	 * Recover the values of the two children of the new child node.  If
 	 * 'node' is not a leaf, this stores into 'newind' the 'owner' field,
 	 * which must be first in the node.
 	 */
 	index = *val;
 	node = pctrie_node_load(parentp, NULL, PCTRIE_UNSERIALIZED);
+	pctrie_setparent(child, parent);
+	if (!pctrie_isleaf(node))
+		pctrie_setparent(node, child);
 	newind = *pctrie_toval(node);
 
 	/*
@@ -456,17 +403,17 @@ pctrie_insert_node(void *parentp, struct pctrie_node *parent, uint64_t *val)
 	_Static_assert(sizeof(long long) >= sizeof(uint64_t),
 	    "uint64 too wide");
 	_Static_assert(sizeof(uint64_t) * NBBY <=
-	    (1 << (sizeof(parent->pn_clev) * NBBY)), "pn_clev too narrow");
-	parent->pn_clev = rounddown(ilog2(index ^ newind), PCTRIE_WIDTH);
-	parent->pn_owner = PCTRIE_COUNT;
-	parent->pn_owner = index & -(parent->pn_owner << parent->pn_clev);
+	    (1 << (sizeof(child->pn_clev) * NBBY)), "pn_clev too narrow");
+	child->pn_clev = rounddown(ilog2(index ^ newind), PCTRIE_WIDTH);
+	child->pn_owner = PCTRIE_COUNT;
+	child->pn_owner = index & -(child->pn_owner << child->pn_clev);
 
 
 	/* These writes are not yet visible due to ordering. */
-	pctrie_addnode(parent, index, pctrie_toleaf(val), PCTRIE_UNSERIALIZED);
-	pctrie_addnode(parent, newind, node, PCTRIE_UNSERIALIZED);
+	pctrie_addnode(child, index, pctrie_toleaf(val), PCTRIE_UNSERIALIZED);
+	pctrie_addnode(child, newind, node, PCTRIE_UNSERIALIZED);
 	/* Synchronize to make the above visible. */
-	pctrie_node_store(parentp, parent, PCTRIE_LOCKED);
+	pctrie_node_store(parentp, child, PCTRIE_LOCKED);
 }
 
 /*
@@ -531,41 +478,44 @@ pctrie_lookup_unlocked(struct pctrie *ptree, uint64_t index, smr_t smr)
 }
 
 /*
- * Returns the last node examined in the search for the index, and updates the
- * search path to that node.
+ * Returns the last node examined in the search for the index, and sets the
+ * parent of that node.
  */
 static __always_inline struct pctrie_node *
-_pctrie_iter_lookup_node(struct pctrie_iter *it, uint64_t index, smr_t smr,
-    enum pctrie_access access)
+_pctrie_lookup_node(struct pctrie *ptree, struct pctrie_node *node,
+    uint64_t index, struct pctrie_node **parent_out,
+    smr_t smr, enum pctrie_access access)
 {
-	struct pctrie_node *node;
+	struct pctrie_node *parent;
 	int slot;
 
 	/*
 	 * Climb the search path to find the lowest node from which to start the
 	 * search for a value matching 'index'.
 	 */
-	while (it->top != 0) {
-		node = it->path[it->top - 1];
+	while (node != NULL) {
 		KASSERT(!powerof2(node->pn_popmap),
 		    ("%s: freed node in iter path", __func__));
-		if (!pctrie_keybarr(node, index, &slot)) {
-			node = pctrie_node_load(
-			    &node->pn_child[slot], smr, access);
+		if (!pctrie_keybarr(node, index, &slot))
 			break;
-		}
-		--it->top;
+		node = pctrie_parent(node);
+	}
+
+	if (node == NULL) {
+		parent = NULL;
+		node = pctrie_root_load(ptree, smr, access);
+	} else {
+		parent = node;
+		node = pctrie_node_load(&node->pn_child[slot], smr, access);
 	}
-	if (it->top == 0)
-		node = pctrie_root_load(it->ptree, smr, access);
 
 	/* Seek a node that matches index. */
 	while (!pctrie_isleaf(node) && !pctrie_keybarr(node, index, &slot)) {
-		KASSERT(it->top < nitems(it->path),
-		    ("%s: path overflow in trie %p", __func__, it->ptree));
-		it->path[it->top++] = node;
+		parent = node;
 		node = pctrie_node_load(&node->pn_child[slot], smr, access);
 	}
+	if (parent_out != NULL)
+		*parent_out = parent;
 	return (node);
 }
 
@@ -579,7 +529,8 @@ _pctrie_iter_lookup(struct pctrie_iter *it, uint64_t index, smr_t smr,
 	struct pctrie_node *node;
 
 	it->index = index;
-	node = _pctrie_iter_lookup_node(it, index, smr, access);
+	node = _pctrie_lookup_node(it->ptree, it->node, index, &it->node,
+	    smr, access);
 	return (pctrie_match_value(node, index));
 }
 
@@ -603,13 +554,14 @@ pctrie_iter_insert_lookup(struct pctrie_iter *it, uint64_t *val)
 	struct pctrie_node *node;
 
 	it->index = *val;
-	node = _pctrie_iter_lookup_node(it, *val, NULL, PCTRIE_LOCKED);
+	node = _pctrie_lookup_node(it->ptree, it->node, *val, &it->node,
+	    NULL, PCTRIE_LOCKED);
 	if (node == PCTRIE_NULL) {
-		if (it->top == 0)
+		if (it->node == NULL)
 			pctrie_node_store(pctrie_root(it->ptree),
 			    pctrie_toleaf(val), PCTRIE_LOCKED);
 		else
-			pctrie_addnode(it->path[it->top - 1], it->index,
+			pctrie_addnode(it->node, it->index,
 			    pctrie_toleaf(val), PCTRIE_LOCKED);
 		return (NULL);
 	}
@@ -622,10 +574,7 @@ pctrie_iter_insert_lookup(struct pctrie_iter *it, uint64_t *val)
 	 * children 'node' and 'val'. Return the place that points to 'node'
 	 * now, and will point to to the new branching node later.
 	 */
-	if (it->top == 0)
-		return (pctrie_root(it->ptree));
-	node = it->path[it->top - 1];
-	return (&node->pn_child[pctrie_slot(node, it->index)]);
+	return (pctrie_child(it->ptree, it->node, it->index));
 }
 
 /*
@@ -678,122 +627,21 @@ pctrie_iter_prev(struct pctrie_iter *it)
 	return (_pctrie_iter_stride(it, -1, NULL, PCTRIE_LOCKED));
 }
 
-/*
- * Returns the value with the least index that is greater than or equal to the
- * specified index, or NULL if there are no such values.
- *
- * Requires that access be externally synchronized by a lock.
- */
-static __inline uint64_t *
-pctrie_lookup_ge_node(struct pctrie_node *node, uint64_t index)
-{
-	struct pctrie_node *succ;
-	uint64_t *m;
-	int slot;
-
-	/*
-	 * Descend the trie as if performing an ordinary lookup for the
-	 * specified value.  However, unlike an ordinary lookup, as we descend
-	 * the trie, we use "succ" to remember the last branching-off point,
-	 * that is, the interior node under which the least value that is both
-	 * outside our current path down the trie and greater than the specified
-	 * index resides.  (The node's popmap makes it fast and easy to
-	 * recognize a branching-off point.)  If our ordinary lookup fails to
-	 * yield a value that is greater than or equal to the specified index,
-	 * then we will exit this loop and perform a lookup starting from
-	 * "succ".  If "succ" is not NULL, then that lookup is guaranteed to
-	 * succeed.
-	 */
-	succ = NULL;
-	for (;;) {
-		if (pctrie_isleaf(node)) {
-			if ((m = pctrie_toval(node)) != NULL && *m >= index)
-				return (m);
-			break;
-		}
-		if (pctrie_keybarr(node, index, &slot)) {
-			/*
-			 * If all values in this subtree are > index, then the
-			 * least value in this subtree is the answer.
-			 */
-			if (node->pn_owner > index)
-				succ = node;
-			break;
-		}
-
-		/*
-		 * Just in case the next search step leads to a subtree of all
-		 * values < index, check popmap to see if a next bigger step, to
-		 * a subtree of all pages with values > index, is available.  If
-		 * so, remember to restart the search here.
-		 */
-		if ((node->pn_popmap >> slot) > 1)
-			succ = node;
-		node = pctrie_node_load(&node->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	}
-
-	/*
-	 * Restart the search from the last place visited in the subtree that
-	 * included some values > index, if there was such a place.
-	 */
-	if (succ == NULL)
-		return (NULL);
-	if (succ != node) {
-		/*
-		 * Take a step to the next bigger sibling of the node chosen
-		 * last time.  In that subtree, all values > index.
-		 */
-		slot = pctrie_slot(succ, index) + 1;
-		KASSERT((succ->pn_popmap >> slot) != 0,
-		    ("%s: no popmap siblings past slot %d in node %p",
-		    __func__, slot, succ));
-		slot += ffs(succ->pn_popmap >> slot) - 1;
-		succ = pctrie_node_load(&succ->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	}
-
-	/* 
-	 * Find the value in the subtree rooted at "succ" with the least index.
-	 */
-	while (!pctrie_isleaf(succ)) {
-		KASSERT(succ->pn_popmap != 0,
-		    ("%s: no popmap children in node %p",  __func__, succ));
-		slot = ffs(succ->pn_popmap) - 1;
-		succ = pctrie_node_load(&succ->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	}
-	return (pctrie_toval(succ));
-}
-
-uint64_t *
-pctrie_lookup_ge(struct pctrie *ptree, uint64_t index)
-{
-	return (pctrie_lookup_ge_node(
-	    pctrie_root_load(ptree, NULL, PCTRIE_LOCKED), index));
-}
-
-uint64_t *
-pctrie_subtree_lookup_gt(struct pctrie_node *node, uint64_t index)
-{
-	if (node == NULL || index + 1 == 0)
-		return (NULL);
-	return (pctrie_lookup_ge_node(node, index + 1));
-}
-
 /*
  * Find first leaf >= index, and fill iter with the path to the parent of that
  * leaf.  Return NULL if there is no such leaf less than limit.
  */
-uint64_t *
-pctrie_iter_lookup_ge(struct pctrie_iter *it, uint64_t index)
+static __inline uint64_t *
+_pctrie_lookup_ge(struct pctrie *ptree, struct pctrie_node *node,
+    uint64_t index, struct pctrie_node **parent_out, uint64_t limit)
 {
-	struct pctrie_node *node;
+	struct pctrie_node *parent;
 	uint64_t *m;
 	int slot;
 
 	/* Seek a node that matches index. */
-	node = _pctrie_iter_lookup_node(it, index, NULL, PCTRIE_LOCKED);
+	node = _pctrie_lookup_node(ptree, node, index, &parent,
+	    NULL, PCTRIE_LOCKED);
 
 	/*
 	 * If no such node was found, and instead this path leads only to nodes
@@ -801,36 +649,58 @@ pctrie_iter_lookup_ge(struct pctrie_iter *it, uint64_t index)
 	 */
 	if (node == PCTRIE_NULL || *pctrie_toval(node) < index) {
 		/* Climb the path to find a node with a descendant > index. */
-		while (it->top != 0) {
-			node = it->path[it->top - 1];
+		for (node = parent; node != NULL; node = pctrie_parent(node)) {
 			slot = pctrie_slot(node, index) + 1;
 			if ((node->pn_popmap >> slot) != 0)
 				break;
-			--it->top;
 		}
-		if (it->top == 0)
+		if (node == NULL) {
+			if (parent_out != NULL)
+				*parent_out = NULL;
 			return (NULL);
+		}
 
 		/* Step to the least child with a descendant > index. */
 		slot += ffs(node->pn_popmap >> slot) - 1;
+		parent = node;
 		node = pctrie_node_load(&node->pn_child[slot], NULL,
 		    PCTRIE_LOCKED);
 	}
 	/* Descend to the least leaf of the subtrie. */
 	while (!pctrie_isleaf(node)) {
-		if (it->limit != 0 && node->pn_owner >= it->limit)
+		if (limit != 0 && node->pn_owner >= limit)
 			return (NULL);
 		slot = ffs(node->pn_popmap) - 1;
-		KASSERT(it->top < nitems(it->path),
-		    ("%s: path overflow in trie %p", __func__, it->ptree));
-		it->path[it->top++] = node;
+		parent = node;
 		node = pctrie_node_load(&node->pn_child[slot], NULL,
 		    PCTRIE_LOCKED);
 	}
+	if (parent_out != NULL)
+		*parent_out = parent;
 	m = pctrie_toval(node);
-	if (it->limit != 0 && *m >= it->limit)
+	if (limit != 0 && *m >= limit)
 		return (NULL);
-	it->index = *m;
+	return (m);
+}
+
+uint64_t *
+pctrie_lookup_ge(struct pctrie *ptree, uint64_t index)
+{
+	return (_pctrie_lookup_ge(ptree, NULL, index, NULL, 0));
+}
+
+/*
+ * Find first leaf >= index, and fill iter with the path to the parent of that
+ * leaf.  Return NULL if there is no such leaf less than limit.
+ */
+uint64_t *
+pctrie_iter_lookup_ge(struct pctrie_iter *it, uint64_t index)
+{
+	uint64_t *m;
+
+	m = _pctrie_lookup_ge(it->ptree, it->node, index, &it->node, it->limit);
+	if (m != NULL)
+		it->index = *m;
 	return (m);
 }
 
@@ -851,91 +721,76 @@ pctrie_iter_jump_ge(struct pctrie_iter *it, int64_t jump)
 	return (pctrie_iter_lookup_ge(it, index));
 }
 
-#ifdef INVARIANTS
-void
-pctrie_subtree_lookup_gt_assert(struct pctrie_node *node, uint64_t index,
-    struct pctrie *ptree, uint64_t *res)
-{
-	uint64_t *expected;
-
-	if (index + 1 == 0)
-		expected = NULL;
-	else
-		expected = pctrie_lookup_ge(ptree, index + 1);
-	KASSERT(res == expected,
-	    ("pctrie subtree lookup gt result different from root lookup: "
-	    "ptree %p, index %ju, subtree %p, found %p, expected %p", ptree,
-	    (uintmax_t)index, node, res, expected));
-}
-#endif
-
 /*
- * Returns the value with the greatest index that is less than or equal to the
- * specified index, or NULL if there are no such values.
- *
- * Requires that access be externally synchronized by a lock.
+ * Find first leaf <= index, and fill iter with the path to the parent of that
+ * leaf.  Return NULL if there is no such leaf greater than limit.
  */
 static __inline uint64_t *
-pctrie_lookup_le_node(struct pctrie_node *node, uint64_t index)
+_pctrie_lookup_le(struct pctrie *ptree, struct pctrie_node *node,
+    uint64_t index, struct pctrie_node **parent_out, uint64_t limit)
 {
-	struct pctrie_node *pred;
+	struct pctrie_node *parent;
 	uint64_t *m;
 	int slot;
 
+	/* Seek a node that matches index. */
+	node = _pctrie_lookup_node(ptree, node, index, &parent, NULL,
+	    PCTRIE_LOCKED);
+
 	/*
-	 * Mirror the implementation of pctrie_lookup_ge_node, described above.
+	 * If no such node was found, and instead this path leads only to nodes
+	 * > index, back up to find a subtrie with the greatest value < index.
 	 */
-	pred = NULL;
-	for (;;) {
-		if (pctrie_isleaf(node)) {
-			if ((m = pctrie_toval(node)) != NULL && *m <= index)
-				return (m);
-			break;
+	if (node == PCTRIE_NULL || *pctrie_toval(node) > index) {
+		/* Climb the path to find a node with a descendant < index. */
+		for (node = parent; node != NULL; node = pctrie_parent(node)) {
+			slot = pctrie_slot(node, index);
+			if ((node->pn_popmap & ((1 << slot) - 1)) != 0)
+				break;
 		}
-		if (pctrie_keybarr(node, index, &slot)) {
-			if (node->pn_owner < index)
-				pred = node;
-			break;
+		if (node == NULL) {
+			if (parent_out != NULL)
+				*parent_out = NULL;
+			return (NULL);
 		}
-		if ((node->pn_popmap & ((1 << slot) - 1)) != 0)
-			pred = node;
+
+		/* Step to the greatest child with a descendant < index. */
+		slot = ilog2(node->pn_popmap & ((1 << slot) - 1));
+		parent = node;
 		node = pctrie_node_load(&node->pn_child[slot], NULL,
 		    PCTRIE_LOCKED);
 	}
-	if (pred == NULL)
-		return (NULL);
-	if (pred != node) {
-		slot = pctrie_slot(pred, index);
-		KASSERT((pred->pn_popmap & ((1 << slot) - 1)) != 0,
-		    ("%s: no popmap siblings before slot %d in node %p",
-		    __func__, slot, pred));
-		slot = ilog2(pred->pn_popmap & ((1 << slot) - 1));
-		pred = pctrie_node_load(&pred->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	}
-	while (!pctrie_isleaf(pred)) {
-		KASSERT(pred->pn_popmap != 0,
-		    ("%s: no popmap children in node %p",  __func__, pred));
-		slot = ilog2(pred->pn_popmap);
-		pred = pctrie_node_load(&pred->pn_child[slot], NULL,
+	/* Descend to the greatest leaf of the subtrie. */
+	while (!pctrie_isleaf(node)) {
+		if (limit != 0 && limit >= node->pn_owner +
+		    ((uint64_t)PCTRIE_COUNT << node->pn_clev) - 1)
+			return (NULL);
+		slot = ilog2(node->pn_popmap);
+		parent = node;
+		node = pctrie_node_load(&node->pn_child[slot], NULL,
 		    PCTRIE_LOCKED);
 	}
-	return (pctrie_toval(pred));
+	if (parent_out != NULL)
+		*parent_out = parent;
+	m = pctrie_toval(node);
+	if (limit != 0 && *m <= limit)
+		return (NULL);
+	return (m);
 }
 
 uint64_t *
 pctrie_lookup_le(struct pctrie *ptree, uint64_t index)
 {
-	return (pctrie_lookup_le_node(
-	    pctrie_root_load(ptree, NULL, PCTRIE_LOCKED), index));
+	return (_pctrie_lookup_le(ptree, NULL, index, NULL, 0));
 }
 
 uint64_t *
-pctrie_subtree_lookup_lt(struct pctrie_node *node, uint64_t index)
+pctrie_subtree_lookup_lt(struct pctrie *ptree, struct pctrie_node *node,
+    uint64_t index)
 {
-	if (node == NULL || index == 0)
+	if (index == 0)
 		return (NULL);
-	return (pctrie_lookup_le_node(node, index - 1));
+	return (_pctrie_lookup_le(ptree, node, index - 1, NULL, 0));
 }
 
 /*
@@ -945,50 +800,11 @@ pctrie_subtree_lookup_lt(struct pctrie_node *node, uint64_t index)
 uint64_t *
 pctrie_iter_lookup_le(struct pctrie_iter *it, uint64_t index)
 {
-	struct pctrie_node *node;
 	uint64_t *m;
-	int slot;
-
-	/* Seek a node that matches index. */
-	node = _pctrie_iter_lookup_node(it, index, NULL, PCTRIE_LOCKED);
 
-	/*
-	 * If no such node was found, and instead this path leads only to nodes
-	 * > index, back up to find a subtrie with the greatest value < index.
-	 */
-	if (node == PCTRIE_NULL || *pctrie_toval(node) > index) {
-		/* Climb the path to find a node with a descendant < index. */
-		while (it->top != 0) {
-			node = it->path[it->top - 1];
-			slot = pctrie_slot(node, index);
-			if ((node->pn_popmap & ((1 << slot) - 1)) != 0)
-				break;
-			--it->top;
-		}
-		if (it->top == 0)
-			return (NULL);
-
-		/* Step to the greatest child with a descendant < index. */
-		slot = ilog2(node->pn_popmap & ((1 << slot) - 1));
-		node = pctrie_node_load(&node->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	}
-	/* Descend to the greatest leaf of the subtrie. */
-	while (!pctrie_isleaf(node)) {
-		if (it->limit != 0 && it->limit >=
-		    node->pn_owner + (PCTRIE_COUNT << node->pn_clev) - 1)
-			return (NULL);
-		slot = ilog2(node->pn_popmap);
-		KASSERT(it->top < nitems(it->path),
-		    ("%s: path overflow in trie %p", __func__, it->ptree));
-		it->path[it->top++] = node;
-		node = pctrie_node_load(&node->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	}
-	m = pctrie_toval(node);
-	if (it->limit != 0 && *m <= it->limit)
-		return (NULL);
-	it->index = *m;
+	m = _pctrie_lookup_le(it->ptree, it->node, index, &it->node, it->limit);
+	if (m != NULL)
+		it->index = *m;
 	return (m);
 }
 
@@ -1009,35 +825,27 @@ pctrie_iter_jump_le(struct pctrie_iter *it, int64_t jump)
 	return (pctrie_iter_lookup_le(it, index));
 }
 
-#ifdef INVARIANTS
-void
-pctrie_subtree_lookup_lt_assert(struct pctrie_node *node, uint64_t index,
-    struct pctrie *ptree, uint64_t *res)
-{
-	uint64_t *expected;
-
-	if (index == 0)
-		expected = NULL;
-	else
-		expected = pctrie_lookup_le(ptree, index - 1);
-	KASSERT(res == expected,
-	    ("pctrie subtree lookup lt result different from root lookup: "
-	    "ptree %p, index %ju, subtree %p, found %p, expected %p", ptree,
-	    (uintmax_t)index, node, res, expected));
-}
-#endif
-
-static void
-pctrie_remove(struct pctrie *ptree, uint64_t index, struct pctrie_node *parent,
-    struct pctrie_node *node, struct pctrie_node **freenode)
+/*
+ * If 'child', a leaf and a child of 'parent', is not NULL and has key 'index',
+ * then remove it from the pctrie and return its value.  If doing so produces an
+ * internal node with only one child, purge it from the pctrie and save it in
+ * *freenode for later disposal.
+ */
+static uint64_t *
+pctrie_remove(struct pctrie *ptree, struct pctrie_node *node, uint64_t index,
+    struct pctrie_node *child, struct pctrie_node **freenode)
 {
-	struct pctrie_node *child;
+	uint64_t *m;
 	int slot;
 
+	*freenode = NULL;
+	m = pctrie_match_value(child, index);
+	if (m == NULL)
+		return (m);
 	if (node == NULL) {
 		pctrie_node_store(pctrie_root(ptree),
 		    PCTRIE_NULL, PCTRIE_LOCKED);
-		return;
+		return (m);
 	}
 	slot = pctrie_slot(node, index);
 	KASSERT((node->pn_popmap & (1 << slot)) != 0,
@@ -1046,28 +854,19 @@ pctrie_remove(struct pctrie *ptree, uint64_t index, struct pctrie_node *parent,
 	node->pn_popmap ^= 1 << slot;
 	pctrie_node_store(&node->pn_child[slot], PCTRIE_NULL, PCTRIE_LOCKED);
 	if (!powerof2(node->pn_popmap))
-		return;
+		return (m);
 	KASSERT(node->pn_popmap != 0, ("%s: bad popmap all zeroes", __func__));
 	slot = ffs(node->pn_popmap) - 1;
 	child = pctrie_node_load(&node->pn_child[slot], NULL, PCTRIE_LOCKED);
 	KASSERT(child != PCTRIE_NULL,
 	    ("%s: bad popmap slot %d in node %p", __func__, slot, node));
-	if (parent == NULL)
-		pctrie_node_store(pctrie_root(ptree), child, PCTRIE_LOCKED);
-	else {
-		slot = pctrie_slot(parent, index);
-		KASSERT(node ==
-		    pctrie_node_load(&parent->pn_child[slot], NULL,
-		    PCTRIE_LOCKED), ("%s: invalid child value", __func__));
-		pctrie_node_store(&parent->pn_child[slot], child,
-		    PCTRIE_LOCKED);
-	}
-	/*
-	 * The child is still valid and we can not zero the
-	 * pointer until all SMR references are gone.
-	 */
-	pctrie_node_put(node);
 	*freenode = node;
+	node = pctrie_parent(node);
+	if (!pctrie_isleaf(child))
+		pctrie_setparent(child, node);
+	pctrie_node_store(pctrie_child(ptree, node, index), child,
+	    PCTRIE_LOCKED);
+	return (m);
 }
 
 /*
@@ -1078,24 +877,18 @@ uint64_t *
 pctrie_remove_lookup(struct pctrie *ptree, uint64_t index,
     struct pctrie_node **freenode)
 {
-	struct pctrie_node *child, *node, *parent;
-	uint64_t *m;
+	struct pctrie_node *child, *node;
 	int slot;
 
-	DEBUG_POISON_POINTER(parent);
-	*freenode = node = NULL;
+	node = NULL;
 	child = pctrie_root_load(ptree, NULL, PCTRIE_LOCKED);
 	while (!pctrie_isleaf(child)) {
-		parent = node;
 		node = child;
 		slot = pctrie_slot(node, index);
 		child = pctrie_node_load(&node->pn_child[slot], NULL,
 		    PCTRIE_LOCKED);
 	}
-	m = pctrie_match_value(child, index);
-	if (m != NULL)
-		pctrie_remove(ptree, index, parent, node, freenode);
-	return (m);
+	return (pctrie_remove(ptree, node, index, child, freenode));
 }
 
 /*
@@ -1105,27 +898,14 @@ pctrie_remove_lookup(struct pctrie *ptree, uint64_t index,
 uint64_t *
 pctrie_iter_remove(struct pctrie_iter *it, struct pctrie_node **freenode)
 {
-	struct pctrie_node *child, *node, *parent;
+	struct pctrie_node *child;
 	uint64_t *m;
-	int slot;
 
-	DEBUG_POISON_POINTER(parent);
-	*freenode = NULL;
-	if (it->top >= 1) {
-		parent = (it->top >= 2) ? it->path[it->top - 2] : NULL;
-		node = it->path[it->top - 1];
-		slot = pctrie_slot(node, it->index);
-		child = pctrie_node_load(&node->pn_child[slot], NULL,
-		    PCTRIE_LOCKED);
-	} else {
-		node = NULL;
-		child = pctrie_root_load(it->ptree, NULL, PCTRIE_LOCKED);
-	}
-	m = pctrie_match_value(child, it->index);
-	if (m != NULL)
-		pctrie_remove(it->ptree, it->index, parent, node, freenode);
+	child = pctrie_node_load(pctrie_child(it->ptree, it->node, it->index),
+	    NULL, PCTRIE_LOCKED);
+	m = pctrie_remove(it->ptree, it->node, it->index, child, freenode);
 	if (*freenode != NULL)
-		--it->top;
+		it->node = pctrie_parent(it->node);
 	return (m);
 }
 
@@ -1137,25 +917,16 @@ uint64_t *
 pctrie_iter_value(struct pctrie_iter *it)
 {
 	struct pctrie_node *node;
-	int slot;
 
-	if (it->top == 0)
-		node = pctrie_root_load(it->ptree, NULL,
-		    PCTRIE_LOCKED);
-	else {
-		node = it->path[it->top - 1];
-		slot = pctrie_slot(node, it->index);
*** 319 LINES SKIPPED ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502102020.51AKK7Ig036651>