Re: [PATCH 0/2] fix mas_new_root()

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

* Liam R. Howlett <Liam.Howlett@oracle.com> [241015 20:42]:
> * Wei Yang <richard.weiyang@gmail.com> [241015 19:39]:
> > When overwriting the whole range with NULL, current behavior is not correct.
> > 
> 
> This is really strange.  You have changed the code to be wrong then
> removed it..  The second patch removes what you changed in the first.
> 
> It doesn't look right today but what you have done is also not right.

Looking at this again, the code that you have changed is correct.

I actually think the bug is the other way around.  If we are
represnenting 0 - ULONG_MAX => NULL, then it's an empty tree and we
don't need a node to store that, and shouldn't.

It's also not really a bug, but a missed optimisation.  The ranges are
stored correctly, we just use too much memory in one case.

The dump isn't clear, but since we merge NULL entries, if there is a 0-0
-> NULL and 1-ULONG_MAX => NULL, then they will be one and the same.
You could change the dump code as part of your fix.

It's like the init of a tree (tree->ma_root = NULL).

Please don't submit multiple patches to fix the same thing like this, it
makes it look like you are trying to pad your patch count.  I'm guessing
you did this to keep them logically separate, but when you completely
drop the entire block of code that was changed in the second patch it
becomes a bit much (and hard to follow, I was trying to figure out what
branch you were working off because it didn't look like the patch would
apply to my branch).

Please submit a testcase with any suspected bugs. If it is not possible
to do the fix first, then do them at the same time.  I often write the
fix for a bug, then recreate the bug in a testcase and ensure that it
fails without my fix.

I am not sure the fixes tag is correct in the patch either, since so
much has changed around this.  You could test the older code to see once
you write a testcase.  But the bug is using a node to store 0-ULONG_MAX
=> NULL.

> 
> 
> > Wei Yang (2):
> >   maple_tree: not necessary to check index/last again
> >   maple_tree: one single entry couldn't represent the whole range
> > 
> >  lib/maple_tree.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 
> >