[PATCH v2 0/8] Improve the validation for maple tree and some cleanup

[PATCH v2 0/8] Improve the validation for maple tree and some cleanup

[Peng Zhang]

These patches do the following:
001 - 002: Small cleanup to maple tree.
003 - 006: Improve the validation for maple tree.
007 - 008: Drop some functions that will no longer be used.

Thanks Liam for the review.

Changes since v1:
 - Add fixes tag and the necessary Cc. [1/8]
 - Add the verification that gaps beyond the node limit are zero. [3/8]
 - Revise comment. [5/8]
 - Reformat the code. [6/8]

Peng Zhang (8):
  maple_tree: set the node limit when creating a new root node
  maple_tree: don't use MAPLE_ARANGE64_META_MAX to indicate no gap
  maple_tree: make mas_validate_gaps() to check metadata
  maple_tree: fix mas_validate_child_slot() to check last missed slot
  maple_tree: make mas_validate_limits() check root node and node limit
  maple_tree: update mt_validate()
  maple_tree: replace mas_logical_pivot() with mas_safe_pivot()
  maple_tree: drop mas_first_entry()

 include/linux/maple_tree.h |   2 -
 lib/maple_tree.c           | 258 ++++++++++++-------------------------
 2 files changed, 79 insertions(+), 181 deletions(-)

-- 
2.20.1

[PATCH v2 1/8] maple_tree: set the node limit when creating a new root node

[Peng Zhang]

Set the node limit of the root node so that the last pivot of all nodes
is the node limit (if the node is not full).

This patch also fixes a bug in mas_rev_awalk().  Effectively, always
setting a maximum makes mas_logical_pivot() behave as mas_safe_pivot().
Without this fix, it is possible that very small tasks would fail to
find the correct gap.  Although this has not been observed with real
tasks, it has been reported to happen in m68k nommu running the maple
tree tests.

Link: https://lore.kernel.org/linux-mm/CAMuHMdV4T53fOw7VPoBgPR7fP6RYqf=CBhD_y_vOg53zZX_DnA@mail.gmail.com/
Fixes: 54a611b60590 ("Maple Tree: add new data structure")
Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: <stable@vger.kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
---
 lib/maple_tree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index d3072858c280..f55e59bd9122 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -3692,7 +3692,8 @@ static inline int mas_root_expand(struct ma_state *mas, void *entry)
 	mas->offset = slot;
 	pivots[slot] = mas->last;
 	if (mas->last != ULONG_MAX)
-		slot++;
+		pivots[++slot] = ULONG_MAX;
+
 	mas->depth = 1;
 	mas_set_height(mas);
 	ma_set_meta(node, maple_leaf_64, 0, slot);
-- 
2.20.1

[PATCH v2 2/8] maple_tree: don't use MAPLE_ARANGE64_META_MAX to indicate no gap

[Peng Zhang]

Do not use a special offset to indicate that there is no gap. When there
is no gap, offset can point to any valid slots because its gap is 0.

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 include/linux/maple_tree.h |  2 --
 lib/maple_tree.c           | 13 ++-----------
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
index e18ecbefc7f7..4e004d86c780 100644
--- a/include/linux/maple_tree.h
+++ b/include/linux/maple_tree.h
@@ -29,14 +29,12 @@
 #define MAPLE_NODE_SLOTS	31	/* 256 bytes including ->parent */
 #define MAPLE_RANGE64_SLOTS	16	/* 256 bytes */
 #define MAPLE_ARANGE64_SLOTS	10	/* 240 bytes */
-#define MAPLE_ARANGE64_META_MAX	15	/* Out of range for metadata */
 #define MAPLE_ALLOC_SLOTS	(MAPLE_NODE_SLOTS - 1)
 #else
 /* 32bit sizes */
 #define MAPLE_NODE_SLOTS	63	/* 256 bytes including ->parent */
 #define MAPLE_RANGE64_SLOTS	32	/* 256 bytes */
 #define MAPLE_ARANGE64_SLOTS	21	/* 240 bytes */
-#define MAPLE_ARANGE64_META_MAX	31	/* Out of range for metadata */
 #define MAPLE_ALLOC_SLOTS	(MAPLE_NODE_SLOTS - 2)
 #endif /* defined(CONFIG_64BIT) || defined(BUILD_VDSO32_64) */
 
diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index f55e59bd9122..6a8982146338 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -1610,8 +1610,6 @@ ma_max_gap(struct maple_node *node, unsigned long *gaps, enum maple_type mt,
  * mas_max_gap() - find the largest gap in a non-leaf node and set the slot.
  * @mas: The maple state.
  *
- * If the metadata gap is set to MAPLE_ARANGE64_META_MAX, there is no gap.
- *
  * Return: The gap value.
  */
 static inline unsigned long mas_max_gap(struct ma_state *mas)
@@ -1628,9 +1626,6 @@ static inline unsigned long mas_max_gap(struct ma_state *mas)
 	node = mas_mn(mas);
 	MAS_BUG_ON(mas, mt != maple_arange_64);
 	offset = ma_meta_gap(node, mt);
-	if (offset == MAPLE_ARANGE64_META_MAX)
-		return 0;
-
 	gaps = ma_gaps(node, mt);
 	return gaps[offset];
 }
@@ -1662,10 +1657,7 @@ static inline void mas_parent_gap(struct ma_state *mas, unsigned char offset,
 ascend:
 	MAS_BUG_ON(mas, pmt != maple_arange_64);
 	meta_offset = ma_meta_gap(pnode, pmt);
-	if (meta_offset == MAPLE_ARANGE64_META_MAX)
-		meta_gap = 0;
-	else
-		meta_gap = pgaps[meta_offset];
+	meta_gap = pgaps[meta_offset];
 
 	pgaps[offset] = new;
 
@@ -1678,7 +1670,6 @@ static inline void mas_parent_gap(struct ma_state *mas, unsigned char offset,
 
 		ma_set_meta_gap(pnode, pmt, offset);
 	} else if (new < meta_gap) {
-		meta_offset = 15;
 		new = ma_max_gap(pnode, pgaps, pmt, &meta_offset);
 		ma_set_meta_gap(pnode, pmt, meta_offset);
 	}
@@ -2076,7 +2067,7 @@ static inline void mab_mas_cp(struct maple_big_node *b_node,
 	end = j - 1;
 	if (likely(!ma_is_leaf(mt) && mt_is_alloc(mas->tree))) {
 		unsigned long max_gap = 0;
-		unsigned char offset = 15;
+		unsigned char offset = 0;
 
 		gaps = ma_gaps(node, mt);
 		do {
-- 
2.20.1

[PATCH v2 3/8] maple_tree: make mas_validate_gaps() to check metadata

[Peng Zhang]

Make mas_validate_gaps() check whether the offset in the metadata points
to the largest gap. By the way, simplify this function.

Add the verification that gaps beyond the node limit are zero.

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
---
 lib/maple_tree.c | 78 ++++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 36 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 6a8982146338..a5776fab1875 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -6983,15 +6983,16 @@ EXPORT_SYMBOL_GPL(mt_dump);
 static void mas_validate_gaps(struct ma_state *mas)
 {
 	struct maple_enode *mte = mas->node;
-	struct maple_node *p_mn;
+	struct maple_node *p_mn, *node = mte_to_node(mte);
+	enum maple_type mt = mte_node_type(mas->node);
 	unsigned long gap = 0, max_gap = 0;
 	unsigned long p_end, p_start = mas->min;
-	unsigned char p_slot;
+	unsigned char p_slot, offset;
 	unsigned long *gaps = NULL;
-	unsigned long *pivots = ma_pivots(mte_to_node(mte), mte_node_type(mte));
-	int i;
+	unsigned long *pivots = ma_pivots(node, mt);
+	unsigned int i;
 
-	if (ma_is_dense(mte_node_type(mte))) {
+	if (ma_is_dense(mt)) {
 		for (i = 0; i < mt_slot_count(mte); i++) {
 			if (mas_get_slot(mas, i)) {
 				if (gap > max_gap)
@@ -7004,52 +7005,59 @@ static void mas_validate_gaps(struct ma_state *mas)
 		goto counted;
 	}
 
-	gaps = ma_gaps(mte_to_node(mte), mte_node_type(mte));
+	gaps = ma_gaps(node, mt);
 	for (i = 0; i < mt_slot_count(mte); i++) {
-		p_end = mas_logical_pivot(mas, pivots, i, mte_node_type(mte));
+		p_end = mas_logical_pivot(mas, pivots, i, mt);
 
 		if (!gaps) {
-			if (mas_get_slot(mas, i)) {
-				gap = 0;
-				goto not_empty;
-			}
-
-			gap += p_end - p_start + 1;
+			if (!mas_get_slot(mas, i))
+				gap = p_end - p_start + 1;
 		} else {
 			void *entry = mas_get_slot(mas, i);
 
 			gap = gaps[i];
-			if (!entry) {
-				if (gap != p_end - p_start + 1) {
-					pr_err("%p[%u] -> %p %lu != %lu - %lu + 1\n",
-						mas_mn(mas), i,
-						mas_get_slot(mas, i), gap,
-						p_end, p_start);
-					mt_dump(mas->tree, mt_dump_hex);
-
-					MT_BUG_ON(mas->tree,
-						gap != p_end - p_start + 1);
-				}
-			} else {
-				if (gap > p_end - p_start + 1) {
-					pr_err("%p[%u] %lu >= %lu - %lu + 1 (%lu)\n",
-					mas_mn(mas), i, gap, p_end, p_start,
-					p_end - p_start + 1);
-					MT_BUG_ON(mas->tree,
-						gap > p_end - p_start + 1);
-				}
+			MT_BUG_ON(mas->tree, !entry);
+
+			if (gap > p_end - p_start + 1) {
+				pr_err("%p[%u] %lu >= %lu - %lu + 1 (%lu)\n",
+				       mas_mn(mas), i, gap, p_end, p_start,
+				       p_end - p_start + 1);
+				MT_BUG_ON(mas->tree, gap > p_end - p_start + 1);
 			}
 		}
 
 		if (gap > max_gap)
 			max_gap = gap;
-not_empty:
+
 		p_start = p_end + 1;
 		if (p_end >= mas->max)
 			break;
 	}
 
 counted:
+	if (mt == maple_arange_64) {
+		offset = ma_meta_gap(node, mt);
+		if (offset > i) {
+			pr_err("gap offset %p[%u] is invalid\n", node, offset);
+			MT_BUG_ON(mas->tree, 1);
+		}
+
+		if (gaps[offset] != max_gap) {
+			pr_err("gap %p[%u] is not the largest gap %lu\n",
+			       node, offset, max_gap);
+			MT_BUG_ON(mas->tree, 1);
+		}
+
+		MT_BUG_ON(mas->tree, !gaps);
+		for (i++ ; i < mt_slot_count(mte); i++) {
+			if (gaps[i] != 0) {
+				pr_err("gap %p[%u] beyond node limit != 0\n",
+				       node, i);
+				MT_BUG_ON(mas->tree, 1);
+			}
+		}
+	}
+
 	if (mte_is_root(mte))
 		return;
 
@@ -7059,10 +7067,8 @@ static void mas_validate_gaps(struct ma_state *mas)
 	if (ma_gaps(p_mn, mas_parent_type(mas, mte))[p_slot] != max_gap) {
 		pr_err("gap %p[%u] != %lu\n", p_mn, p_slot, max_gap);
 		mt_dump(mas->tree, mt_dump_hex);
+		MT_BUG_ON(mas->tree, 1);
 	}
-
-	MT_BUG_ON(mas->tree,
-		  ma_gaps(p_mn, mas_parent_type(mas, mte))[p_slot] != max_gap);
 }
 
 static void mas_validate_parent_slot(struct ma_state *mas)
-- 
2.20.1

[PATCH v2 4/8] maple_tree: fix mas_validate_child_slot() to check last missed slot

[Peng Zhang]

Don't break the loop before checking the last slot. Also here check if
non-leaf nodes are missing children.

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 lib/maple_tree.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index a5776fab1875..f750ebef595f 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -7119,11 +7119,12 @@ static void mas_validate_child_slot(struct ma_state *mas)
 
 	for (i = 0; i < mt_slots[type]; i++) {
 		child = mas_slot(mas, slots, i);
-		if (!pivots[i] || pivots[i] == mas->max)
-			break;
 
-		if (!child)
-			break;
+		if (!child) {
+			pr_err("Non-leaf node lacks child at %p[%u]\n",
+			       mas_mn(mas), i);
+			MT_BUG_ON(mas->tree, 1);
+		}
 
 		if (mte_parent_slot(child) != i) {
 			pr_err("Slot error at %p[%u]: child %p has pslot %u\n",
@@ -7138,6 +7139,9 @@ static void mas_validate_child_slot(struct ma_state *mas)
 			       mte_to_node(mas->node));
 			MT_BUG_ON(mas->tree, 1);
 		}
+
+		if (i < mt_pivots[type] && pivots[i] == mas->max)
+			break;
 	}
 }
 
-- 
2.20.1

[PATCH v2 5/8] maple_tree: make mas_validate_limits() check root node and node limit

[Peng Zhang]

Update mas_validate_limits() to check root node, check node limit pivot
if there is enough room for it to exist and check data_end. Remove the
check for child existence as it is done in mas_validate_child_slot().

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
---
 lib/maple_tree.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index f750ebef595f..072532fa18ee 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -7146,7 +7146,9 @@ static void mas_validate_child_slot(struct ma_state *mas)
 }
 
 /*
- * Validate all pivots are within mas->min and mas->max.
+ * Validate all pivots are within mas->min and mas->max, check metadata ends
+ * where the maximum ends and ensure there is no slots or pivots set outside of
+ * the end of the data.
  */
 static void mas_validate_limits(struct ma_state *mas)
 {
@@ -7156,26 +7158,15 @@ static void mas_validate_limits(struct ma_state *mas)
 	void __rcu **slots = ma_slots(mte_to_node(mas->node), type);
 	unsigned long *pivots = ma_pivots(mas_mn(mas), type);
 
-	/* all limits are fine here. */
-	if (mte_is_root(mas->node))
-		return;
-
 	for (i = 0; i < mt_slots[type]; i++) {
 		unsigned long piv;
 
 		piv = mas_safe_pivot(mas, pivots, i, type);
 
-		if (!piv && (i != 0))
-			break;
-
-		if (!mte_is_leaf(mas->node)) {
-			void *entry = mas_slot(mas, slots, i);
-
-			if (!entry)
-				pr_err("%p[%u] cannot be null\n",
-				       mas_mn(mas), i);
-
-			MT_BUG_ON(mas->tree, !entry);
+		if (!piv && (i != 0)) {
+			pr_err("Missing node limit pivot at %p[%u]",
+			       mas_mn(mas), i);
+			MAS_WARN_ON(mas, 1);
 		}
 
 		if (prev_piv > piv) {
@@ -7198,6 +7189,13 @@ static void mas_validate_limits(struct ma_state *mas)
 		if (piv == mas->max)
 			break;
 	}
+
+	if (mas_data_end(mas) != i) {
+		pr_err("node%p: data_end %u != the last slot offset %u\n",
+		       mas_mn(mas), mas_data_end(mas), i);
+		MT_BUG_ON(mas->tree, 1);
+	}
+
 	for (i += 1; i < mt_slots[type]; i++) {
 		void *entry = mas_slot(mas, slots, i);
 
-- 
2.20.1

[PATCH v2 6/8] maple_tree: update mt_validate()

[Peng Zhang]

Instead of using mas_first_entry() to find the leftmost leaf, use a
simple loop instead. Remove an unneeded check for root node. To make
the error message more accurate, check pivots first and then slots,
because checking slots depend on the node limit pivot to break the loop.

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
---
 lib/maple_tree.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 072532fa18ee..1ad11799e197 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -7270,21 +7270,20 @@ void mt_validate(struct maple_tree *mt)
 	if (!mas_searchable(&mas))
 		goto done;
 
-	mas_first_entry(&mas, mas_mn(&mas), ULONG_MAX, mte_node_type(mas.node));
+	while (!mte_is_leaf(mas.node))
+		mas_descend(&mas);
+
 	while (!mas_is_none(&mas)) {
 		MAS_WARN_ON(&mas, mte_dead_node(mas.node));
-		if (!mte_is_root(mas.node)) {
-			end = mas_data_end(&mas);
-			if (MAS_WARN_ON(&mas,
-					(end < mt_min_slot_count(mas.node)) &&
-					(mas.max != ULONG_MAX))) {
-				pr_err("Invalid size %u of %p\n", end,
-				       mas_mn(&mas));
-			}
+		end = mas_data_end(&mas);
+		if (MAS_WARN_ON(&mas, (end < mt_min_slot_count(mas.node)) &&
+				(mas.max != ULONG_MAX))) {
+			pr_err("Invalid size %u of %p\n", end, mas_mn(&mas));
 		}
+
 		mas_validate_parent_slot(&mas);
-		mas_validate_child_slot(&mas);
 		mas_validate_limits(&mas);
+		mas_validate_child_slot(&mas);
 		if (mt_is_alloc(mt))
 			mas_validate_gaps(&mas);
 		mas_dfs_postorder(&mas, ULONG_MAX);
-- 
2.20.1

[PATCH v2 7/8] maple_tree: replace mas_logical_pivot() with mas_safe_pivot()

[Peng Zhang]

Replace mas_logical_pivot() with mas_safe_pivot() and drop
mas_logical_pivot() since it won't be used anymore. We can do this since
now all nodes will have node limit pivot (if it is not full node).

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 lib/maple_tree.c | 33 +++------------------------------
 1 file changed, 3 insertions(+), 30 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 1ad11799e197..5eb5eede2727 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -728,33 +728,6 @@ mas_safe_min(struct ma_state *mas, unsigned long *pivots, unsigned char offset)
 	return mas->min;
 }
 
-/*
- * mas_logical_pivot() - Get the logical pivot of a given offset.
- * @mas: The maple state
- * @pivots: The pointer to the maple node pivots
- * @offset: The offset into the pivot array
- * @type: The maple node type
- *
- * When there is no value at a pivot (beyond the end of the data), then the
- * pivot is actually @mas->max.
- *
- * Return: the logical pivot of a given @offset.
- */
-static inline unsigned long
-mas_logical_pivot(struct ma_state *mas, unsigned long *pivots,
-		  unsigned char offset, enum maple_type type)
-{
-	unsigned long lpiv = mas_safe_pivot(mas, pivots, offset, type);
-
-	if (likely(lpiv))
-		return lpiv;
-
-	if (likely(offset))
-		return mas->max;
-
-	return lpiv;
-}
-
 /*
  * mte_set_pivot() - Set a pivot to a value in an encoded maple node.
  * @mn: The encoded maple node
@@ -2202,7 +2175,7 @@ static noinline_for_kasan void mas_store_b_node(struct ma_wr_state *wr_mas,
 		goto b_end;
 
 	/* Handle new range ending before old range ends */
-	piv = mas_logical_pivot(mas, wr_mas->pivots, offset_end, wr_mas->type);
+	piv = mas_safe_pivot(mas, wr_mas->pivots, offset_end, wr_mas->type);
 	if (piv > mas->last) {
 		if (piv == ULONG_MAX)
 			mas_bulk_rebalance(mas, b_node->b_end, wr_mas->type);
@@ -4934,7 +4907,7 @@ static inline bool mas_anode_descend(struct ma_state *mas, unsigned long size)
 	min = mas_safe_min(mas, pivots, offset);
 	data_end = ma_data_end(node, type, pivots, mas->max);
 	for (; offset <= data_end; offset++) {
-		pivot = mas_logical_pivot(mas, pivots, offset, type);
+		pivot = mas_safe_pivot(mas, pivots, offset, type);
 
 		/* Not within lower bounds */
 		if (mas->index > pivot)
@@ -7007,7 +6980,7 @@ static void mas_validate_gaps(struct ma_state *mas)
 
 	gaps = ma_gaps(node, mt);
 	for (i = 0; i < mt_slot_count(mte); i++) {
-		p_end = mas_logical_pivot(mas, pivots, i, mt);
+		p_end = mas_safe_pivot(mas, pivots, i, mt);
 
 		if (!gaps) {
 			if (!mas_get_slot(mas, i))
-- 
2.20.1

[PATCH v2 8/8] maple_tree: drop mas_first_entry()

[Peng Zhang]

The internal function mas_first_entry() is no longer used, so drop it.

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 lib/maple_tree.c | 72 ------------------------------------------------
 1 file changed, 72 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 5eb5eede2727..bd4557eeb42c 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -6662,78 +6662,6 @@ static inline struct maple_enode *mas_get_slot(struct ma_state *mas,
 			offset);
 }
 
-
-/*
- * mas_first_entry() - Go the first leaf and find the first entry.
- * @mas: the maple state.
- * @limit: the maximum index to check.
- * @*r_start: Pointer to set to the range start.
- *
- * Sets mas->offset to the offset of the entry, r_start to the range minimum.
- *
- * Return: The first entry or MAS_NONE.
- */
-static inline void *mas_first_entry(struct ma_state *mas, struct maple_node *mn,
-		unsigned long limit, enum maple_type mt)
-
-{
-	unsigned long max;
-	unsigned long *pivots;
-	void __rcu **slots;
-	void *entry = NULL;
-
-	mas->index = mas->min;
-	if (mas->index > limit)
-		goto none;
-
-	max = mas->max;
-	mas->offset = 0;
-	while (likely(!ma_is_leaf(mt))) {
-		MAS_WARN_ON(mas, mte_dead_node(mas->node));
-		slots = ma_slots(mn, mt);
-		entry = mas_slot(mas, slots, 0);
-		pivots = ma_pivots(mn, mt);
-		if (unlikely(ma_dead_node(mn)))
-			return NULL;
-		max = pivots[0];
-		mas->node = entry;
-		mn = mas_mn(mas);
-		mt = mte_node_type(mas->node);
-	}
-	MAS_WARN_ON(mas, mte_dead_node(mas->node));
-
-	mas->max = max;
-	slots = ma_slots(mn, mt);
-	entry = mas_slot(mas, slots, 0);
-	if (unlikely(ma_dead_node(mn)))
-		return NULL;
-
-	/* Slot 0 or 1 must be set */
-	if (mas->index > limit)
-		goto none;
-
-	if (likely(entry))
-		return entry;
-
-	mas->offset = 1;
-	entry = mas_slot(mas, slots, 1);
-	pivots = ma_pivots(mn, mt);
-	if (unlikely(ma_dead_node(mn)))
-		return NULL;
-
-	mas->index = pivots[0] + 1;
-	if (mas->index > limit)
-		goto none;
-
-	if (likely(entry))
-		return entry;
-
-none:
-	if (likely(!ma_dead_node(mn)))
-		mas->node = MAS_NONE;
-	return NULL;
-}
-
 /* Depth first search, post-order */
 static void mas_dfs_postorder(struct ma_state *mas, unsigned long max)
 {
-- 
2.20.1

Re: [PATCH v2 0/8] Improve the validation for maple tree and some cleanup

[Geert Uytterhoeven]

Hi Peng,

On Tue, Jul 11, 2023 at 5:56 AM Peng Zhang <zhangpeng.00@bytedance.com> wrote:
> These patches do the following:
> 001 - 002: Small cleanup to maple tree.
> 003 - 006: Improve the validation for maple tree.
> 007 - 008: Drop some functions that will no longer be used.
>
> Thanks Liam for the review.
>
> Changes since v1:
>  - Add fixes tag and the necessary Cc. [1/8]
>  - Add the verification that gaps beyond the node limit are zero. [3/8]
>  - Revise comment. [5/8]
>  - Reformat the code. [6/8]
>
> Peng Zhang (8):
>   maple_tree: set the node limit when creating a new root node
>   maple_tree: don't use MAPLE_ARANGE64_META_MAX to indicate no gap
>   maple_tree: make mas_validate_gaps() to check metadata
>   maple_tree: fix mas_validate_child_slot() to check last missed slot
>   maple_tree: make mas_validate_limits() check root node and node limit
>   maple_tree: update mt_validate()
>   maple_tree: replace mas_logical_pivot() with mas_safe_pivot()
>   maple_tree: drop mas_first_entry()
>
>  include/linux/maple_tree.h |   2 -
>  lib/maple_tree.c           | 258 ++++++++++++-------------------------
>  2 files changed, 79 insertions(+), 181 deletions(-)

Thanks for your series!

I gave it a try with test_maple_tree on m68k/ARAnyM, and the net
impact is:

     TEST STARTING

     BUG at next_prev_test:2014 (1)
     Pass: 3749128 Run:3749129
    -BUG at check_empty_area_window:2655 (1)
    -Pass: 3754275 Run:3754277
    -BUG at check_empty_area_window:2656 (1)
    -Pass: 3754275 Run:3754278
    -BUG at check_empty_area_window:2657 (1)
    -Pass: 3754275 Run:3754279
    -BUG at check_empty_area_window:2661 (1)
    -Pass: 3754275 Run:3754280
    -BUG at check_empty_area_window:2662 (1)
    -Pass: 3754275 Run:3754281
    -maple_tree: 3804518 of 3804524 tests passed
    +maple_tree: 3804523 of 3804524 tests passed

So only one bug left to squash ;-)

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 3/8] maple_tree: make mas_validate_gaps() to check metadata

[Liam R. Howlett]

* Peng Zhang <zhangpeng.00@bytedance.com> [230710 23:55]:
> Make mas_validate_gaps() check whether the offset in the metadata points
> to the largest gap. By the way, simplify this function.
> 
> Add the verification that gaps beyond the node limit are zero.
> 
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  lib/maple_tree.c | 78 ++++++++++++++++++++++++++----------------------
>  1 file changed, 42 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 6a8982146338..a5776fab1875 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -6983,15 +6983,16 @@ EXPORT_SYMBOL_GPL(mt_dump);
>  static void mas_validate_gaps(struct ma_state *mas)
>  {
>  	struct maple_enode *mte = mas->node;
> -	struct maple_node *p_mn;
> +	struct maple_node *p_mn, *node = mte_to_node(mte);
> +	enum maple_type mt = mte_node_type(mas->node);
>  	unsigned long gap = 0, max_gap = 0;
>  	unsigned long p_end, p_start = mas->min;
> -	unsigned char p_slot;
> +	unsigned char p_slot, offset;
>  	unsigned long *gaps = NULL;
> -	unsigned long *pivots = ma_pivots(mte_to_node(mte), mte_node_type(mte));
> -	int i;
> +	unsigned long *pivots = ma_pivots(node, mt);
> +	unsigned int i;
>  
> -	if (ma_is_dense(mte_node_type(mte))) {
> +	if (ma_is_dense(mt)) {
>  		for (i = 0; i < mt_slot_count(mte); i++) {
>  			if (mas_get_slot(mas, i)) {
>  				if (gap > max_gap)
> @@ -7004,52 +7005,59 @@ static void mas_validate_gaps(struct ma_state *mas)
>  		goto counted;
>  	}
>  
> -	gaps = ma_gaps(mte_to_node(mte), mte_node_type(mte));
> +	gaps = ma_gaps(node, mt);
>  	for (i = 0; i < mt_slot_count(mte); i++) {
> -		p_end = mas_logical_pivot(mas, pivots, i, mte_node_type(mte));
> +		p_end = mas_logical_pivot(mas, pivots, i, mt);
>  
>  		if (!gaps) {
> -			if (mas_get_slot(mas, i)) {
> -				gap = 0;
> -				goto not_empty;
> -			}
> -
> -			gap += p_end - p_start + 1;
> +			if (!mas_get_slot(mas, i))
> +				gap = p_end - p_start + 1;
>  		} else {
>  			void *entry = mas_get_slot(mas, i);
>  
>  			gap = gaps[i];
> -			if (!entry) {
> -				if (gap != p_end - p_start + 1) {
> -					pr_err("%p[%u] -> %p %lu != %lu - %lu + 1\n",
> -						mas_mn(mas), i,
> -						mas_get_slot(mas, i), gap,
> -						p_end, p_start);
> -					mt_dump(mas->tree, mt_dump_hex);
> -
> -					MT_BUG_ON(mas->tree,
> -						gap != p_end - p_start + 1);
> -				}
> -			} else {
> -				if (gap > p_end - p_start + 1) {
> -					pr_err("%p[%u] %lu >= %lu - %lu + 1 (%lu)\n",
> -					mas_mn(mas), i, gap, p_end, p_start,
> -					p_end - p_start + 1);
> -					MT_BUG_ON(mas->tree,
> -						gap > p_end - p_start + 1);
> -				}
> +			MT_BUG_ON(mas->tree, !entry);
> +
> +			if (gap > p_end - p_start + 1) {
> +				pr_err("%p[%u] %lu >= %lu - %lu + 1 (%lu)\n",
> +				       mas_mn(mas), i, gap, p_end, p_start,
> +				       p_end - p_start + 1);
> +				MT_BUG_ON(mas->tree, gap > p_end - p_start + 1);
>  			}
>  		}
>  
>  		if (gap > max_gap)
>  			max_gap = gap;
> -not_empty:
> +
>  		p_start = p_end + 1;
>  		if (p_end >= mas->max)
>  			break;
>  	}
>  
>  counted:
> +	if (mt == maple_arange_64) {
> +		offset = ma_meta_gap(node, mt);
> +		if (offset > i) {
> +			pr_err("gap offset %p[%u] is invalid\n", node, offset);
> +			MT_BUG_ON(mas->tree, 1);
> +		}
> +
> +		if (gaps[offset] != max_gap) {
> +			pr_err("gap %p[%u] is not the largest gap %lu\n",
> +			       node, offset, max_gap);
> +			MT_BUG_ON(mas->tree, 1);
> +		}
> +
> +		MT_BUG_ON(mas->tree, !gaps);
> +		for (i++ ; i < mt_slot_count(mte); i++) {
> +			if (gaps[i] != 0) {
> +				pr_err("gap %p[%u] beyond node limit != 0\n",
> +				       node, i);
> +				MT_BUG_ON(mas->tree, 1);
> +			}
> +		}
> +	}
> +
>  	if (mte_is_root(mte))
>  		return;
>  
> @@ -7059,10 +7067,8 @@ static void mas_validate_gaps(struct ma_state *mas)
>  	if (ma_gaps(p_mn, mas_parent_type(mas, mte))[p_slot] != max_gap) {
>  		pr_err("gap %p[%u] != %lu\n", p_mn, p_slot, max_gap);
>  		mt_dump(mas->tree, mt_dump_hex);
> +		MT_BUG_ON(mas->tree, 1);
>  	}
> -
> -	MT_BUG_ON(mas->tree,
> -		  ma_gaps(p_mn, mas_parent_type(mas, mte))[p_slot] != max_gap);
>  }
>  
>  static void mas_validate_parent_slot(struct ma_state *mas)
> -- 
> 2.20.1
> 

Re: [PATCH v2 6/8] maple_tree: update mt_validate()

[Liam R. Howlett]

* Peng Zhang <zhangpeng.00@bytedance.com> [230710 23:55]:
> Instead of using mas_first_entry() to find the leftmost leaf, use a
> simple loop instead. Remove an unneeded check for root node. To make
> the error message more accurate, check pivots first and then slots,
> because checking slots depend on the node limit pivot to break the loop.
> 
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  lib/maple_tree.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 072532fa18ee..1ad11799e197 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -7270,21 +7270,20 @@ void mt_validate(struct maple_tree *mt)
>  	if (!mas_searchable(&mas))
>  		goto done;
>  
> -	mas_first_entry(&mas, mas_mn(&mas), ULONG_MAX, mte_node_type(mas.node));
> +	while (!mte_is_leaf(mas.node))
> +		mas_descend(&mas);
> +
>  	while (!mas_is_none(&mas)) {
>  		MAS_WARN_ON(&mas, mte_dead_node(mas.node));
> -		if (!mte_is_root(mas.node)) {
> -			end = mas_data_end(&mas);
> -			if (MAS_WARN_ON(&mas,
> -					(end < mt_min_slot_count(mas.node)) &&
> -					(mas.max != ULONG_MAX))) {
> -				pr_err("Invalid size %u of %p\n", end,
> -				       mas_mn(&mas));
> -			}
> +		end = mas_data_end(&mas);
> +		if (MAS_WARN_ON(&mas, (end < mt_min_slot_count(mas.node)) &&
> +				(mas.max != ULONG_MAX))) {
> +			pr_err("Invalid size %u of %p\n", end, mas_mn(&mas));
>  		}
> +
>  		mas_validate_parent_slot(&mas);
> -		mas_validate_child_slot(&mas);
>  		mas_validate_limits(&mas);
> +		mas_validate_child_slot(&mas);
>  		if (mt_is_alloc(mt))
>  			mas_validate_gaps(&mas);
>  		mas_dfs_postorder(&mas, ULONG_MAX);
> -- 
> 2.20.1
> 
> 
> -- 
> maple-tree mailing list
> maple-tree@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/maple-tree

Re: [PATCH v2 5/8] maple_tree: make mas_validate_limits() check root node and node limit

[Liam R. Howlett]

* Peng Zhang <zhangpeng.00@bytedance.com> [230710 23:55]:
> Update mas_validate_limits() to check root node, check node limit pivot
> if there is enough room for it to exist and check data_end. Remove the
> check for child existence as it is done in mas_validate_child_slot().
> 
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  lib/maple_tree.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index f750ebef595f..072532fa18ee 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -7146,7 +7146,9 @@ static void mas_validate_child_slot(struct ma_state *mas)
>  }
>  
>  /*
> - * Validate all pivots are within mas->min and mas->max.
> + * Validate all pivots are within mas->min and mas->max, check metadata ends
> + * where the maximum ends and ensure there is no slots or pivots set outside of
> + * the end of the data.
>   */
>  static void mas_validate_limits(struct ma_state *mas)
>  {
> @@ -7156,26 +7158,15 @@ static void mas_validate_limits(struct ma_state *mas)
>  	void __rcu **slots = ma_slots(mte_to_node(mas->node), type);
>  	unsigned long *pivots = ma_pivots(mas_mn(mas), type);
>  
> -	/* all limits are fine here. */
> -	if (mte_is_root(mas->node))
> -		return;
> -
>  	for (i = 0; i < mt_slots[type]; i++) {
>  		unsigned long piv;
>  
>  		piv = mas_safe_pivot(mas, pivots, i, type);
>  
> -		if (!piv && (i != 0))
> -			break;
> -
> -		if (!mte_is_leaf(mas->node)) {
> -			void *entry = mas_slot(mas, slots, i);
> -
> -			if (!entry)
> -				pr_err("%p[%u] cannot be null\n",
> -				       mas_mn(mas), i);
> -
> -			MT_BUG_ON(mas->tree, !entry);
> +		if (!piv && (i != 0)) {
> +			pr_err("Missing node limit pivot at %p[%u]",
> +			       mas_mn(mas), i);
> +			MAS_WARN_ON(mas, 1);
>  		}
>  
>  		if (prev_piv > piv) {
> @@ -7198,6 +7189,13 @@ static void mas_validate_limits(struct ma_state *mas)
>  		if (piv == mas->max)
>  			break;
>  	}
> +
> +	if (mas_data_end(mas) != i) {
> +		pr_err("node%p: data_end %u != the last slot offset %u\n",
> +		       mas_mn(mas), mas_data_end(mas), i);
> +		MT_BUG_ON(mas->tree, 1);
> +	}
> +
>  	for (i += 1; i < mt_slots[type]; i++) {
>  		void *entry = mas_slot(mas, slots, i);
>  
> -- 
> 2.20.1
> 
> 

Re: [PATCH v2 0/8] Improve the validation for maple tree and some cleanup

[Liam R. Howlett]

* Geert Uytterhoeven <geert@linux-m68k.org> [230711 06:10]:
> Hi Peng,
> 
> On Tue, Jul 11, 2023 at 5:56 AM Peng Zhang <zhangpeng.00@bytedance.com> wrote:
> > These patches do the following:
> > 001 - 002: Small cleanup to maple tree.
> > 003 - 006: Improve the validation for maple tree.
> > 007 - 008: Drop some functions that will no longer be used.
> >
> > Thanks Liam for the review.
> >
> > Changes since v1:
> >  - Add fixes tag and the necessary Cc. [1/8]
> >  - Add the verification that gaps beyond the node limit are zero. [3/8]
> >  - Revise comment. [5/8]
> >  - Reformat the code. [6/8]
> >
> > Peng Zhang (8):
> >   maple_tree: set the node limit when creating a new root node
> >   maple_tree: don't use MAPLE_ARANGE64_META_MAX to indicate no gap
> >   maple_tree: make mas_validate_gaps() to check metadata
> >   maple_tree: fix mas_validate_child_slot() to check last missed slot
> >   maple_tree: make mas_validate_limits() check root node and node limit
> >   maple_tree: update mt_validate()
> >   maple_tree: replace mas_logical_pivot() with mas_safe_pivot()
> >   maple_tree: drop mas_first_entry()
> >
> >  include/linux/maple_tree.h |   2 -
> >  lib/maple_tree.c           | 258 ++++++++++++-------------------------
> >  2 files changed, 79 insertions(+), 181 deletions(-)
> 
> Thanks for your series!
> 
> I gave it a try with test_maple_tree on m68k/ARAnyM, and the net
> impact is:
> 
>      TEST STARTING
> 
>      BUG at next_prev_test:2014 (1)
>      Pass: 3749128 Run:3749129
>     -BUG at check_empty_area_window:2655 (1)
>     -Pass: 3754275 Run:3754277
>     -BUG at check_empty_area_window:2656 (1)
>     -Pass: 3754275 Run:3754278
>     -BUG at check_empty_area_window:2657 (1)
>     -Pass: 3754275 Run:3754279
>     -BUG at check_empty_area_window:2661 (1)
>     -Pass: 3754275 Run:3754280
>     -BUG at check_empty_area_window:2662 (1)
>     -Pass: 3754275 Run:3754281
>     -maple_tree: 3804518 of 3804524 tests passed
>     +maple_tree: 3804523 of 3804524 tests passed
> 
> So only one bug left to squash ;-)

That one is a bug for 32b in the test, I have two fixes and I'll send
them soon.

Thanks for re-testing.

Regards,
Liam