2022-12-20 14:51:59

by Vernon Yang

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

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

Signed-off-by: Vernon Yang <[email protected]>
---
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


2022-12-20 15:09:28

by Vernon Yang

[permalink] [raw]
Subject: [PATCH 8/8] maple_tree: refine mab_calc_split function

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 <[email protected]>
---
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

2022-12-20 15:10:11

by Vernon Yang

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

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 <[email protected]>
---
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

2022-12-20 15:15:42

by Vernon Yang

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

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 <[email protected]>
---
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

2022-12-20 15:16:45

by Liam R. Howlett

[permalink] [raw]
Subject: Re: [PATCH 8/8] maple_tree: refine mab_calc_split function

* Vernon Yang <[email protected]> [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 <[email protected]>
> ---
> 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
>
>

2022-12-21 05:24:35

by Vernon Yang

[permalink] [raw]
Subject: Re: [PATCH 8/8] maple_tree: refine mab_calc_split function

On Tue, Dec 20, 2022 at 03:05:52PM +0000, Liam Howlett wrote:
> * Vernon Yang <[email protected]> [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 <[email protected]>
> > ---
> > 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
> >
> >