[PATCH 1/8] maple_tree: remove extra space and blank line

[PATCH 1/8] maple_tree: remove extra space and blank line

[Vernon Yang]

These extra space and blank line is unnecessary, so drop it.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 include/linux/maple_tree.h |  2 --
 lib/maple_tree.c           | 14 ++++----------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
index e594db58a0f1..4ee5a969441c 100644
--- a/include/linux/maple_tree.h
+++ b/include/linux/maple_tree.h
@@ -517,7 +517,6 @@ static inline void mas_reset(struct ma_state *mas)
  * entry.
  *
  * Note: may return the zero entry.
- *
  */
 #define mas_for_each(__mas, __entry, __max) \
 	while (((__entry) = mas_find((__mas), (__max))) != NULL)
@@ -639,7 +638,6 @@ static inline void mt_set_in_rcu(struct maple_tree *mt)
 }
 
 static inline unsigned int mt_height(const struct maple_tree *mt)
-
 {
 	return (mt->ma_flags & MT_FLAGS_HEIGHT_MASK) >> MT_FLAGS_HEIGHT_OFFSET;
 }
diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index fe3947b80069..8ace65a5eea5 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -183,7 +183,6 @@ static void ma_free_rcu(struct maple_node *node)
 	call_rcu(&node->rcu, mt_free_rcu);
 }
 
-
 static void mas_set_height(struct ma_state *mas)
 {
 	unsigned int new_flags = mas->tree->ma_flags;
@@ -468,7 +467,7 @@ static inline
 void mte_set_parent(struct maple_enode *enode, const struct maple_enode *parent,
 		    unsigned char slot)
 {
-	unsigned long val = (unsigned long) parent;
+	unsigned long val = (unsigned long)parent;
 	unsigned long shift;
 	unsigned long type;
 	enum maple_type p_type = mte_node_type(parent);
@@ -502,7 +501,7 @@ void mte_set_parent(struct maple_enode *enode, const struct maple_enode *parent,
  */
 static inline unsigned int mte_parent_slot(const struct maple_enode *enode)
 {
-	unsigned long val = (unsigned long) mte_to_node(enode)->parent;
+	unsigned long val = (unsigned long)mte_to_node(enode)->parent;
 
 	/* Root. */
 	if (val & 1)
@@ -1278,7 +1277,6 @@ static inline void mas_alloc_nodes(struct ma_state *mas, gfp_t gfp)
 		mas->alloc->total = success;
 	mas_set_err(mas, -ENOMEM);
 	return;
-
 }
 
 /*
@@ -2946,7 +2944,7 @@ static inline void *mtree_range_walk(struct ma_state *mas)
 	mas->min = prev_min;
 	mas->max = prev_max;
 	mas->node = last;
-	return (void *) next;
+	return (void *)next;
 
 dead_node:
 	mas_reset(mas);
@@ -3464,7 +3462,6 @@ static inline bool mas_push_data(struct ma_state *mas, int height,
  */
 static int mas_split(struct ma_state *mas, struct maple_big_node *b_node)
 {
-
 	struct maple_subtree_state mast;
 	int height = 0;
 	unsigned char mid_split, split = 0;
@@ -3890,7 +3887,7 @@ static inline void *mtree_lookup_walk(struct ma_state *mas)
 			goto dead_node;
 	} while (!ma_is_leaf(type));
 
-	return (void *) next;
+	return (void *)next;
 
 dead_node:
 	mas_reset(mas);
@@ -4708,7 +4705,6 @@ static inline void *mas_next_nentry(struct ma_state *mas,
 
 static inline void mas_rewalk(struct ma_state *mas, unsigned long index)
 {
-
 retry:
 	mas_set(mas, index);
 	mas_state_walk(mas);
@@ -4716,7 +4712,6 @@ static inline void mas_rewalk(struct ma_state *mas, unsigned long index)
 		goto retry;
 
 	return;
-
 }
 
 /*
@@ -5618,7 +5613,6 @@ static void mas_wr_store_setup(struct ma_wr_state *wr_mas)
 				mas_reset(wr_mas->mas);
 		}
 	}
-
 }
 
 /* Interface */
-- 
2.34.1

[PATCH 2/8] maple_tree: remove extra return statement

[Vernon Yang]

For functions with a return type of void, it is unnecessary to
add a reurn statement at the end of the function, so drop it.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 lib/maple_tree.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 8ace65a5eea5..34aa93c85d6e 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -1276,7 +1276,6 @@ static inline void mas_alloc_nodes(struct ma_state *mas, gfp_t gfp)
 	if (mas->alloc && !(((unsigned long)mas->alloc & 0x1)))
 		mas->alloc->total = success;
 	mas_set_err(mas, -ENOMEM);
-	return;
 }
 
 /*
@@ -4710,8 +4709,6 @@ static inline void mas_rewalk(struct ma_state *mas, unsigned long index)
 	mas_state_walk(mas);
 	if (mas_is_start(mas))
 		goto retry;
-
-	return;
 }
 
 /*
-- 
2.34.1

[PATCH 3/8] maple_tree: use mt_node_max() instead of direct operations mt_max[]

[Vernon Yang]

Use mt_node_max() to get the maximum number of slots for a node,
rather than direct operations mt_max[], makes it better portability.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 lib/maple_tree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 34aa93c85d6e..3d45c515ed42 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -6723,7 +6723,7 @@ static void mt_dump_range64(const struct maple_tree *mt, void *entry,
 
 		if (i < (MAPLE_RANGE64_SLOTS - 1))
 			last = node->pivot[i];
-		else if (!node->slot[i] && max != mt_max[mte_node_type(entry)])
+		else if (!node->slot[i] && max != mt_node_max(entry))
 			break;
 		if (last == 0 && i > 0)
 			break;
@@ -6830,7 +6830,7 @@ void mt_dump(const struct maple_tree *mt)
 	if (!xa_is_node(entry))
 		mt_dump_entry(entry, 0, 0, 0);
 	else if (entry)
-		mt_dump_node(mt, entry, 0, mt_max[mte_node_type(entry)], 0);
+		mt_dump_node(mt, entry, 0, mt_node_max(entry), 0);
 }
 EXPORT_SYMBOL_GPL(mt_dump);
 
-- 
2.34.1

[PATCH 4/8] maple_tree: use macro MA_ROOT_PARENT instead of number

[Vernon Yang]

When you need to compare whether node->parent is parent of the
root node, using macro MA_ROOT_PARENT is easier to understand
and for better readability.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 lib/maple_tree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 3d45c515ed42..b3a215dd961e 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -503,8 +503,7 @@ static inline unsigned int mte_parent_slot(const struct maple_enode *enode)
 {
 	unsigned long val = (unsigned long)mte_to_node(enode)->parent;
 
-	/* Root. */
-	if (val & 1)
+	if (val & MA_ROOT_PARENT)
 		return 0;
 
 	/*
-- 
2.34.1

[PATCH 5/8] maple_tree: remove the redundant code

[Vernon Yang]

The macros CONFIG_DEBUG_MAPLE_TREE_VERBOSE no one uses, functions
mas_dup_tree() and mas_dup_store() are not implemented, just
function declaration, so drop it.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 include/linux/maple_tree.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
index 4ee5a969441c..815a27661517 100644
--- a/include/linux/maple_tree.h
+++ b/include/linux/maple_tree.h
@@ -12,7 +12,6 @@
 #include <linux/rcupdate.h>
 #include <linux/spinlock.h>
 /* #define CONFIG_MAPLE_RCU_DISABLED */
-/* #define CONFIG_DEBUG_MAPLE_TREE_VERBOSE */
 
 /*
  * Allocated nodes are mutable until they have been inserted into the tree,
@@ -483,9 +482,6 @@ static inline bool mas_is_paused(struct ma_state *mas)
 	return mas->node == MAS_PAUSE;
 }
 
-void mas_dup_tree(struct ma_state *oldmas, struct ma_state *mas);
-void mas_dup_store(struct ma_state *mas, void *entry);
-
 /*
  * This finds an empty area from the highest address to the lowest.
  * AKA "Topdown" version,
-- 
2.34.1

[PATCH 6/8] maple_tree: change return type of mas_commit_b_node()

[Vernon Yang]

The return value of mas_commit_b_node() function represents whether
the submit b_node is successful, and can only be false and true, so
the return value type is bool is more appropriate and for better
readability.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 lib/maple_tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index b3a215dd961e..e7dde4a1d6cb 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -3578,7 +3578,7 @@ static inline bool mas_reuse_node(struct ma_wr_state *wr_mas,
  * @b_node: The maple big node
  * @end: The end of the data.
  */
-static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
+static inline bool mas_commit_b_node(struct ma_wr_state *wr_mas,
 			    struct maple_big_node *b_node, unsigned char end)
 {
 	struct maple_node *node;
@@ -3598,7 +3598,7 @@ static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
 
 	mas_node_count(wr_mas->mas, 1);
 	if (mas_is_err(wr_mas->mas))
-		return 0;
+		return false;
 
 	node = mas_pop_node(wr_mas->mas);
 	node->parent = mas_mn(wr_mas->mas)->parent;
@@ -3607,7 +3607,7 @@ static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
 	mas_replace(wr_mas->mas, false);
 reuse_node:
 	mas_update_gap(wr_mas->mas);
-	return 1;
+	return true;
 }
 
 /*
-- 
2.34.1

[PATCH 7/8] maple_tree: refine ma_state init from mas_start()

[Vernon Yang]

If mas->node is an MAS_START, there are three cases, and they
all assign different values to mas->node and mas->offset. So
there is no need to set them to a default value before updating.

Update them directly to make them easier to understand and for
better readability.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 lib/maple_tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index e7dde4a1d6cb..16cdcf309e97 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -1329,7 +1329,7 @@ static void mas_node_count(struct ma_state *mas, int count)
  * mas_start() - Sets up maple state for operations.
  * @mas: The maple state.
  *
- * If mas->node == MAS_START, then set the min, max, depth, and offset to
+ * If mas->node == MAS_START, then set the min, max and depth to
  * defaults.
  *
  * Return:
@@ -1343,22 +1343,22 @@ static inline struct maple_enode *mas_start(struct ma_state *mas)
 	if (likely(mas_is_start(mas))) {
 		struct maple_enode *root;
 
-		mas->node = MAS_NONE;
 		mas->min = 0;
 		mas->max = ULONG_MAX;
 		mas->depth = 0;
-		mas->offset = 0;
 
 		root = mas_root(mas);
 		/* Tree with nodes */
 		if (likely(xa_is_node(root))) {
 			mas->depth = 1;
 			mas->node = mte_safe_root(root);
+			mas->offset = 0;
 			return NULL;
 		}
 
 		/* empty tree */
 		if (unlikely(!root)) {
+			mas->node = MAS_NONE;
 			mas->offset = MAPLE_NODE_SLOTS;
 			return NULL;
 		}
-- 
2.34.1

[PATCH 8/8] maple_tree: refine mab_calc_split function

[Vernon Yang]

Invert the conditional judgment of the mid_split, to focus
the return statement in the last statement, which is easier
to understand and for better readability.

Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
---
 lib/maple_tree.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 16cdcf309e97..d147669fb99c 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -1882,10 +1882,9 @@ static inline int mab_calc_split(struct ma_state *mas,
 
 	/* Avoid ending a node on a NULL entry */
 	split = mab_no_null_split(bn, split, slot_count);
-	if (!(*mid_split))
-		return split;
 
-	*mid_split = mab_no_null_split(bn, *mid_split, slot_count);
+	if (*mid_split)
+		*mid_split = mab_no_null_split(bn, *mid_split, slot_count);
 
 	return split;
 }
-- 
2.34.1

Re: [PATCH 6/8] maple_tree: change return type of mas_commit_b_node()

[Liam Howlett]

* Vernon Yang <vernon2gm@gmail.com> [221220 09:26]:
> The return value of mas_commit_b_node() function represents whether
> the submit b_node is successful, and can only be false and true, so
> the return value type is bool is more appropriate and for better
> readability.
> 
> Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
> ---
>  lib/maple_tree.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index b3a215dd961e..e7dde4a1d6cb 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -3578,7 +3578,7 @@ static inline bool mas_reuse_node(struct ma_wr_state *wr_mas,
>   * @b_node: The maple big node
>   * @end: The end of the data.
>   */
> -static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
> +static inline bool mas_commit_b_node(struct ma_wr_state *wr_mas,
>  			    struct maple_big_node *b_node, unsigned char end)

mas_commit_b_node() can also return the ints from mas_split() and
mas_rebalance().  I'm fine with changing the return, but it seems odd to
only half-change it?

Initially I had a different intention for the return type of int, but it
seems the return isn't used at all from this function.  We should
just change mas_commit_b_node(), mas_split(), and mas_rebalance() to
return type void if we're going to clean it up.

>  {
>  	struct maple_node *node;
> @@ -3598,7 +3598,7 @@ static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
>  
>  	mas_node_count(wr_mas->mas, 1);
>  	if (mas_is_err(wr_mas->mas))
> -		return 0;
> +		return false;
>  
>  	node = mas_pop_node(wr_mas->mas);
>  	node->parent = mas_mn(wr_mas->mas)->parent;
> @@ -3607,7 +3607,7 @@ static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
>  	mas_replace(wr_mas->mas, false);
>  reuse_node:
>  	mas_update_gap(wr_mas->mas);
> -	return 1;
> +	return true;
>  }
>  
>  /*
> -- 
> 2.34.1
> 
> 

Re: [PATCH 8/8] maple_tree: refine mab_calc_split function

[Liam Howlett]

* Vernon Yang <vernon2gm@gmail.com> [221220 09:26]:
> Invert the conditional judgment of the mid_split, to focus
> the return statement in the last statement, which is easier
> to understand and for better readability.
> 
> Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
> ---
>  lib/maple_tree.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 16cdcf309e97..d147669fb99c 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -1882,10 +1882,9 @@ static inline int mab_calc_split(struct ma_state *mas,
>  
>  	/* Avoid ending a node on a NULL entry */
>  	split = mab_no_null_split(bn, split, slot_count);
> -	if (!(*mid_split))
> -		return split;
>  
> -	*mid_split = mab_no_null_split(bn, *mid_split, slot_count);
> +	if (*mid_split)
> +		*mid_split = mab_no_null_split(bn, *mid_split, slot_count);

The function is written this way because mid_split is almost always
zero.  If you want to change this, then we should add an unlikely() to
the if statement.

>  
>  	return split;
>  }
> -- 
> 2.34.1
> 
> 

Re: [PATCH 6/8] maple_tree: change return type of mas_commit_b_node()

[Vernon Yang]

On Tue, Dec 20, 2022 at 03:03:36PM +0000, Liam Howlett wrote:
> * Vernon Yang <vernon2gm@gmail.com> [221220 09:26]:
> > The return value of mas_commit_b_node() function represents whether
> > the submit b_node is successful, and can only be false and true, so
> > the return value type is bool is more appropriate and for better
> > readability.
> >
> > Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
> > ---
> >  lib/maple_tree.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index b3a215dd961e..e7dde4a1d6cb 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -3578,7 +3578,7 @@ static inline bool mas_reuse_node(struct ma_wr_state *wr_mas,
> >   * @b_node: The maple big node
> >   * @end: The end of the data.
> >   */
> > -static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
> > +static inline bool mas_commit_b_node(struct ma_wr_state *wr_mas,
> >  			    struct maple_big_node *b_node, unsigned char end)
>
> mas_commit_b_node() can also return the ints from mas_split() and
> mas_rebalance().  I'm fine with changing the return, but it seems odd to
> only half-change it?
Oh, sorry, I forgot to changes the return value type of the mas_split()
and mas_rebalance()

>
> Initially I had a different intention for the return type of int, but it
> seems the return isn't used at all from this function.  We should
> just change mas_commit_b_node(), mas_split(), and mas_rebalance() to
> return type void if we're going to clean it up.
Yes, the return isn't used at all from this function, have noticed.

Initially, I also wanted to change the return type void, but these
functions have an error condition that returns early, so I'm more
inclined to have an error value return so that it's clearer and
for better readable.

en... I temporarily remove this patch 6

>
> >  {
> >  	struct maple_node *node;
> > @@ -3598,7 +3598,7 @@ static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
> >
> >  	mas_node_count(wr_mas->mas, 1);
> >  	if (mas_is_err(wr_mas->mas))
> > -		return 0;
> > +		return false;
> >
> >  	node = mas_pop_node(wr_mas->mas);
> >  	node->parent = mas_mn(wr_mas->mas)->parent;
> > @@ -3607,7 +3607,7 @@ static inline int mas_commit_b_node(struct ma_wr_state *wr_mas,
> >  	mas_replace(wr_mas->mas, false);
> >  reuse_node:
> >  	mas_update_gap(wr_mas->mas);
> > -	return 1;
> > +	return true;
> >  }
> >
> >  /*
> > --
> > 2.34.1
> >
> >

Re: [PATCH 8/8] maple_tree: refine mab_calc_split function

[Vernon Yang]

On Tue, Dec 20, 2022 at 03:05:52PM +0000, Liam Howlett wrote:
> * Vernon Yang <vernon2gm@gmail.com> [221220 09:26]:
> > Invert the conditional judgment of the mid_split, to focus
> > the return statement in the last statement, which is easier
> > to understand and for better readability.
> >
> > Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
> > ---
> >  lib/maple_tree.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 16cdcf309e97..d147669fb99c 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -1882,10 +1882,9 @@ static inline int mab_calc_split(struct ma_state *mas,
> >
> >  	/* Avoid ending a node on a NULL entry */
> >  	split = mab_no_null_split(bn, split, slot_count);
> > -	if (!(*mid_split))
> > -		return split;
> >
> > -	*mid_split = mab_no_null_split(bn, *mid_split, slot_count);
> > +	if (*mid_split)
> > +		*mid_split = mab_no_null_split(bn, *mid_split, slot_count);
>
> The function is written this way because mid_split is almost always
> zero.  If you want to change this, then we should add an unlikely() to
> the if statement.
Okay，thank you very much for the suggestion, I will add
an unlikely() to the if statement.

>
> >
> >  	return split;
> >  }
> > --
> > 2.34.1
> >
> >