Fix storing in XArray check_split tests

Hi,

(resending to fix Message-ID, also gently nudging for feedback!)

I hit an assertion while making some modifications to lib/test_xarray.c [1] and I believe this is the fix.

In check_split, the tests split the XArray node and then store values after the split to verify that splitting worked. While storing and retrieval works as expected, the node's metadata, specifically node->nr_values, is not updated correctly.

This led to the assertion being hit in [1], since the storing process did not increment node->nr_values sufficiently, while the erasing process assumed the fully-incremented node->nr_values state.

Would like to check my understanding on these:

1. In the multi-index xarray world, is node->nr_values definitely the total number of values *and siblings* in the node?

2. IIUC xas_store() has significantly different behavior when entry is NULL vs non-NULL: when entry is NULL, xas_store() does not make assumptions on the number of siblings and erases all the way till the next non-sibling entry. This sounds fair to me, but it's also kind of surprising that it is differently handled when entry is non-NULL, where xas_store() respects xas->xa_sibs.

3. If xas_store() is dependent on its caller to set up xas correctly (also sounds fair), then there are places where xas_store() is used, like replace_page_cache_folio() or migrate_huge_page_move_mapping(), where xas is set up assuming 0 order pages. Are those buggy?

[1] https://lore.kernel.org/all/[email protected]/

Ackerley Tng (2):
XArray tests: Fix check_split tests to store correctly XArray tests: Verify xa_erase behavior in check_split

 lib/test_xarray.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

--
2.53.0.rc1.225.gd81095ad13-goog

__xa_store() does not set up xas->xa_sibs, and when it calls xas_store(), xas_store() stops prematurely and does not update node->nr_values to count all values and siblings. Hence, when working with multi-index XArrays, __xa_store() cannot be used.

Fix tests by calling xas_store() directly with xas->xa_sibs correctly set up.

---
 lib/test_xarray.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

index 5ca0aefee9aa..e71e8ff76900 100644
@@ -1846,8 +1846,14 @@ static void check_split_1(struct xarray *xa, unsigned long index,
 	xas_split_alloc(&xas, xa, order, GFP_KERNEL);
 	xas_lock(&xas);
 	xas_split(&xas, xa, order);
-	for (i = 0; i < (1 << order); i += (1 << new_order))
-		__xa_store(xa, index + i, xa_mk_index(index + i), 0);
+	for (i = 0; i < (1 << order); i += (1 << new_order)) {
+		xas_set_order(&xas, index + i, new_order);
+		/*
+		 * Don't worry about -ENOMEM, xas_split_alloc() and
+		 * xas_split() ensures that all nodes are allocated.
+		 */
+		xas_store(&xas, xa_mk_index(index + i));
+	}
 	xas_unlock(&xas);
 
 	for (i = 0; i < (1 << order); i++) {

@@ -1893,8 +1899,14 @@ static void check_split_2(struct xarray *xa, unsigned long index,
 		xas_unlock(&xas);
 		goto out;
 	}
-	for (i = 0; i < (1 << order); i += (1 << new_order))
-		__xa_store(xa, index + i, xa_mk_index(index + i), 0);
+	for (i = 0; i < (1 << order); i += (1 << new_order)) {
+		xas_set_order(&xas, index + i, new_order);
+		/*
+		 * Don't worry about -ENOMEM, xas_split_alloc() and
+		 * xas_split() ensures that all nodes are allocated.
+		 */
+		xas_store(&xas, xa_mk_index(index + i));
+	}
 	xas_unlock(&xas);
 
 	for (i = 0; i < (1 << order); i++) {

2.53.0.rc1.225.gd81095ad13-goog

Both __xa_store() and xa_erase() use xas_store() under the hood, but when the entry being stored is NULL (as in the case of xa_erase()), xas->xa_sibs (and max) is only checked if the next entry is not a sibling, hence allowing xas_store() to keep iterating, hence updating node->nr_values correctly.

Add xa_erase() to check_split tests that verify functionality, with the added intent to illustrate the usage differences between __xa_store(), xas_store() and xa_erase() with regard to multi-index XArrays.

---
 lib/test_xarray.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index e71e8ff76900..bb9471a3df65 100644
@@ -1874,6 +1874,10 @@ static void check_split_1(struct xarray *xa, unsigned long index,
 	rcu_read_unlock();
 	XA_BUG_ON(xa, found != 1 << (order - new_order));
 
+	for (i = 0; i < (1 << order); i += (1 << new_order))
+		xa_erase(xa, index + i);
+	XA_BUG_ON(xa, !xa_empty(xa));
+
 	xa_destroy(xa);
 }
 

@@ -1926,6 +1930,10 @@ static void check_split_2(struct xarray *xa, unsigned long index,
 	}
 	rcu_read_unlock();
 	XA_BUG_ON(xa, found != 1 << (order - new_order));
+
+	for (i = 0; i < (1 << order); i += (1 << new_order))
+		xa_erase(xa, index + i);
+	XA_BUG_ON(xa, !xa_empty(xa));
 out:
 	xas_destroy(&xas);
 	xa_destroy(xa);

2.53.0.rc1.225.gd81095ad13-goog