Re: + xarray-add-xas_split-fix.patch added to -mm tree

Re: + xarray-add-xas_split-fix.patch added to -mm tree

[Matthew Wilcox]

On Fri, Sep 04, 2020 at 10:23:53AM -0700, akpm@linux-foundation.org wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: xarray-add-xas_split-fix

Can I ask you to add another -fix patch?

In my testing, I found that splitting a THP that was of order 6 or higher
meant that marking it as dirty no longer worked.  It was pretty weird.
I figured out that what I was doing was setting all the mark bits in the
new child node unconditionally (instead of only if the split entry was
marked).  Once the mark bits were set in the node, calling xa_set_mark()
no longer works because it sees the leaf of the tree is already marked,
and so it does not walk up the tree to set the mark on the parents.
And thus the call to xa_get_mark() never walks _down_ to see the child
is marked.

It doesn't affect your tree because it's only used for read-only pages,
but in my tree it causes failures once I induce splitting to happen
more frequently.

I've included an update to the test-suite which ensures this doesn't
happen again (if you apply just the test suite update, it'll cause
failures in the test suite).

From edcd692083586b2b218ce3d935429a2349969ba9 Mon Sep 17 00:00:00 2001
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Date: Tue, 8 Sep 2020 17:55:22 -0400
Subject: [PATCH] fix xarray split

---
 lib/test_xarray.c | 3 +++
 lib/xarray.c      | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/test_xarray.c b/lib/test_xarray.c
index 37f7fed3e982..4115979cc716 100644
--- a/lib/test_xarray.c
+++ b/lib/test_xarray.c
@@ -1546,6 +1546,9 @@ static void check_split_1(struct xarray *xa, unsigned long index,
 	}
 	XA_BUG_ON(xa, i != 1 << order);
 
+	xa_set_mark(xa, index, XA_MARK_0);
+	XA_BUG_ON(xa, !xa_get_mark(xa, index, XA_MARK_0));
+
 	xa_destroy(xa);
 }
 
diff --git a/lib/xarray.c b/lib/xarray.c
index 53045e14ad22..9180a7d48315 100644
--- a/lib/xarray.c
+++ b/lib/xarray.c
@@ -973,10 +973,11 @@ static void node_set_marks(struct xa_node *node, unsigned int offset,
 	xa_mark_t mark = XA_MARK_0;
 
 	for (;;) {
-		if (marks & (1 << (__force unsigned int)mark))
+		if (marks & (1 << (__force unsigned int)mark)) {
 			node_set_mark(node, offset, mark);
-		if (child)
-			node_mark_all(child, mark);
+			if (child)
+				node_mark_all(child, mark);
+		}
 		if (mark == XA_MARK_MAX)
 			break;
 		mark_inc(mark);
-- 
2.28.0

Re: + xarray-add-xas_split-fix.patch added to -mm tree

[Matthew Wilcox]

On Tue, Sep 08, 2020 at 11:06:44PM +0100, Matthew Wilcox wrote:
> On Fri, Sep 04, 2020 at 10:23:53AM -0700, akpm@linux-foundation.org wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Subject: xarray-add-xas_split-fix
> 
> Can I ask you to add another -fix patch?

... updated to also fix the documentation.  Thanks, Mauro.

commit 71c3da6f580dc6ce98143d92da768c85638d4518
Author: Matthew Wilcox (Oracle) <willy@infradead.org>
Date:   Tue Sep 8 17:55:22 2020 -0400

    fix xarray split

diff --git a/lib/test_xarray.c b/lib/test_xarray.c
index 16fe5adcac62..8262c3f05a5d 100644
--- a/lib/test_xarray.c
+++ b/lib/test_xarray.c
@@ -1524,6 +1524,9 @@ static void check_split_1(struct xarray *xa, unsigned long index,
 	}
 	XA_BUG_ON(xa, i != 1 << order);
 
+	xa_set_mark(xa, index, XA_MARK_0);
+	XA_BUG_ON(xa, !xa_get_mark(xa, index, XA_MARK_0));
+
 	xa_destroy(xa);
 }
 
diff --git a/lib/xarray.c b/lib/xarray.c
index ff3652657bd8..b573db455c43 100644
--- a/lib/xarray.c
+++ b/lib/xarray.c
@@ -973,10 +973,11 @@ static void node_set_marks(struct xa_node *node, unsigned int offset,
 	xa_mark_t mark = XA_MARK_0;
 
 	for (;;) {
-		if (marks & (1 << (__force unsigned int)mark))
+		if (marks & (1 << (__force unsigned int)mark)) {
 			node_set_mark(node, offset, mark);
-		if (child)
-			node_mark_all(child, mark);
+			if (child)
+				node_mark_all(child, mark);
+		}
 		if (mark == XA_MARK_MAX)
 			break;
 		mark_inc(mark);
@@ -984,8 +985,18 @@ static void node_set_marks(struct xa_node *node, unsigned int offset,
 }
 
 /**
- * Allocate memory for splitting an entry of @order size into the order
- * stored in the @xas.
+ * xas_split_alloc() - Allocate memory for splitting an entry.
+ * @xas: XArray operation state.
+ * @entry: New entry which will be stored in the array.
+ * @order: New entry order.
+ * @gfp: Memory allocation flags.
+ *
+ * This function should be called before calling xas_split().
+ * If necessary, it will allocate new nodes (and fill them with @entry)
+ * to prepare for the upcoming split of an entry of @order size into
+ * entries of the order stored in the @xas.
+ *
+ * Context: May sleep if @gfp flags permit.
  */
 void xas_split_alloc(struct xa_state *xas, void *entry, unsigned int order,
 		gfp_t gfp)