Re: [RFC v1 5/5] maple_tree: replace preallocation with slub percpu array prefill

["Liam R. Howlett" <Liam.Howlett@Oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 2350 bytes --]

* Liam R. Howlett <Liam.Howlett@Oracle.com> [230808 10:37]:
> * Vlastimil Babka <vbabka@suse.cz> [230808 05:53]:
> > With the percpu array we can try not doing the preallocations in maple
> > tree, and instead make sure the percpu array is prefilled, and using
> > GFP_ATOMIC in places that relied on the preallocation (in case we miss
> > or fail trylock on the array), i.e. mas_store_prealloc(). For now simply
> > add __GFP_NOFAIL there as well.
> > 
> > First I tried to change mas_node_count_gfp() to not preallocate anything
> > anywhere, but that lead to warns and panics, even though the other
> > caller mas_node_count() uses GFP_NOWAIT | __GFP_NOWARN so it has no
> > guarantees... So I changed just mas_preallocate(). I let it still to
> > truly preallocate a single node, but maybe it's not necessary?
> 
> Ah, yes.  I added a check to make sure we didn't allocate more nodes
> when using preallocations.  This check is what you are hitting when you
> don't allocate anything.  This is tracked in mas_flags by
> setting/clearing MA_STATE_PREALLOC.  Good news, that check works!

Adding the attached patch to your series prior to the below allows for
the removal of the extra preallocation.

> 
> > ---
> >  lib/maple_tree.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 7a8e7c467d7c..5a209d88c318 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -5534,7 +5534,12 @@ void mas_store_prealloc(struct ma_state *mas, void *entry)
> >  
> >  	mas_wr_store_setup(&wr_mas);
> >  	trace_ma_write(__func__, mas, 0, entry);
> > +
> > +retry:
> >  	mas_wr_store_entry(&wr_mas);
> > +	if (unlikely(mas_nomem(mas, GFP_ATOMIC | __GFP_NOFAIL)))
> > +		goto retry;
> > +
> >  	MAS_WR_BUG_ON(&wr_mas, mas_is_err(mas));
> >  	mas_destroy(mas);
> >  }
> > @@ -5550,9 +5555,10 @@ EXPORT_SYMBOL_GPL(mas_store_prealloc);
> >  int mas_preallocate(struct ma_state *mas, gfp_t gfp)
> >  {
> >  	int ret;
> > +	int count = 1 + mas_mt_height(mas) * 3;
> >  
> > -	mas_node_count_gfp(mas, 1 + mas_mt_height(mas) * 3, gfp);
> > -	mas->mas_flags |= MA_STATE_PREALLOC;
> > +	mas_node_count_gfp(mas, 1, gfp);
> > +	kmem_cache_prefill_percpu_array(maple_node_cache, count, gfp);
> >  	if (likely(!mas_is_err(mas)))
> >  		return 0;
> >  
> > -- 
> > 2.41.0
> > 

[-- Attachment #2: 0001-maple_tree-Remove-MA_STATE_PREALLOC.patch --]
[-- Type: text/x-diff, Size: 2823 bytes --]

From 5f1f230ed424ec37d65a3537f03c7e6e961cc713 Mon Sep 17 00:00:00 2001
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
Date: Tue, 8 Aug 2023 14:54:27 -0400
Subject: [PATCH] maple_tree: Remove MA_STATE_PREALLOC

MA_SATE_PREALLOC was added to catch any writes that try to allocate when
the maple state is being used in preallocation mode.  This can safely be
removed in favour of the percpu array of nodes.

Note that mas_expected_entries() still expects no allocations during
operation and so MA_STATE_BULK can be used in place of preallocations
for this case, which is primarily used for forking.

Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 lib/maple_tree.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 5a209d88c318..78796342caf0 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -68,11 +68,9 @@
  * Maple state flags
  * * MA_STATE_BULK		- Bulk insert mode
  * * MA_STATE_REBALANCE		- Indicate a rebalance during bulk insert
- * * MA_STATE_PREALLOC		- Preallocated nodes, WARN_ON allocation
  */
 #define MA_STATE_BULK		1
 #define MA_STATE_REBALANCE	2
-#define MA_STATE_PREALLOC	4
 
 #define ma_parent_ptr(x) ((struct maple_pnode *)(x))
 #define ma_mnode_ptr(x) ((struct maple_node *)(x))
@@ -1279,11 +1277,8 @@ static inline void mas_alloc_nodes(struct ma_state *mas, gfp_t gfp)
 		return;
 
 	mas_set_alloc_req(mas, 0);
-	if (mas->mas_flags & MA_STATE_PREALLOC) {
-		if (allocated)
-			return;
-		WARN_ON(!allocated);
-	}
+	if (mas->mas_flags & MA_STATE_BULK)
+		return;
 
 	if (!allocated || mas->alloc->node_count == MAPLE_ALLOC_SLOTS) {
 		node = (struct maple_alloc *)mt_alloc_one(gfp);
@@ -5601,7 +5596,7 @@ void mas_destroy(struct ma_state *mas)
 
 		mas->mas_flags &= ~MA_STATE_REBALANCE;
 	}
-	mas->mas_flags &= ~(MA_STATE_BULK|MA_STATE_PREALLOC);
+	mas->mas_flags &= ~MA_STATE_BULK;
 
 	total = mas_allocated(mas);
 	while (total) {
@@ -5650,9 +5645,6 @@ int mas_expected_entries(struct ma_state *mas, unsigned long nr_entries)
 	 * of nodes during the operation.
 	 */
 
-	/* Optimize splitting for bulk insert in-order */
-	mas->mas_flags |= MA_STATE_BULK;
-
 	/*
 	 * Avoid overflow, assume a gap between each entry and a trailing null.
 	 * If this is wrong, it just means allocation can happen during
@@ -5669,8 +5661,9 @@ int mas_expected_entries(struct ma_state *mas, unsigned long nr_entries)
 	/* Add working room for split (2 nodes) + new parents */
 	mas_node_count(mas, nr_nodes + 3);
 
-	/* Detect if allocations run out */
-	mas->mas_flags |= MA_STATE_PREALLOC;
+	/* Optimize splitting for bulk insert in-order */
+	mas->mas_flags |= MA_STATE_BULK;
+
 
 	if (!mas_is_err(mas))
 		return 0;
-- 
2.39.2