On Tue, Oct 15, 2024 at 09:25:19PM -0400, Liam R. Howlett wrote: >* Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> [241015 20:42]: >> * Wei Yang <richard.weiyang@xxxxxxxxx> [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). Agree with your above statement, this depends how we want to handle this. The change here is to make the behavior consistent. Want to confirm with you: the fix in this patch is fine with your, right? > >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). Sure, will merge it. > >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. > Since user won't detect the difference, so a case to see whether the root is a node looks good to you? >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. > So I should drop the fix tag? >> >> >> > 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 >> > >> > -- Wei Yang Help you, Help me
