* Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> [241018 13:57]:
> * Wei Yang <richard.weiyang@xxxxxxxxx> [241017 22:40]:
> > Currently, when storing NULL on mas_store_root(), the behavior could be
> > improved.
> >
> > For example possible cases are:
> >
> > * store NULL at any range result a new node
> > * store NULL at range [m, n] where m > 0 to a single entry tree result
> > a new node with range [m, n] set to NULL
> > * store NULL at range [m, n] where m > 0 to an empty tree result
> > consecutive NULL slot
> >
> > This patch tries to improve in:
> >
> > * memory efficient by setting to empty tree instead of using a node
>
> > * remove the possibility of consecutive NULL slot which will prohibit
> > extended null in later operation
>
> I don't understand this. Do we actually store consecutive NULLs now?
>
> This is a very odd change log for fixing an optimisation. Maybe start
> by explaining how we end up with a node with a single value now, then
> state how this code changes that?
>
> >
> > Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> > CC: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> > CC: Sidhartha Kumar <sidhartha.kumar@xxxxxxxxxx>
> > CC: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> >
> > ---
> > v3: move change into mas_store_root()
> > ---
> > lib/maple_tree.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index db8b89487c98..03fbee9880eb 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -3439,7 +3439,11 @@ static inline void mas_root_expand(struct ma_state *mas, void *entry)
> >
> > static inline void mas_store_root(struct ma_state *mas, void *entry)
> > {
> > - if (likely((mas->last != 0) || (mas->index != 0)))
> > + if (!entry) {
> > + void *contents = mas_root_locked(mas);
> > +
> > + if (!mas->index && contents)
> > + rcu_assign_pointer(mas->tree->ma_root, NULL);
>
> You are changing what used to handle any range that wasn't 0 to handle
> storing NULL.
>
> This seems really broken.
>
> > + } else if (likely((mas->last != 0) || (mas->index != 0)))
>
> Isn't this exactly what you have above in the if statement?
Oh, I see. It's the same as the line you deleted above.
>
> > mas_root_expand(mas, entry);
> > else if (((unsigned long) (entry) & 3) == 2)
> > mas_root_expand(mas, entry);
> > --
> > 2.34.1
> >
