On Sat, Sep 07, 2024 at 09:29:01AM -0400, Liam R. Howlett wrote:
>* Wei Yang <richard.weiyang@xxxxxxxxx> [240907 04:50]:
>> In maple_big_node, we define slot and padding/gap in a union. And based
>> on current definition of MAPLE_BIG_NODE_SLOTS/GAPS, padding is always
>> less then slot and part of the gap is overlapped by slot.
> ^^^^- than
>
>>
>> For example on 64bit system:
>>
>> MAPLE_BIG_NODE_SLOT is 34
>> MAPLE_BIG_NODE_GAP is 21
>>
>> With this knowledge, current code actually clear the whole
>> maple_big_node and even clear some space twice.
>>
>> Instead of clear maple_big_node each field separately, let's clear it in
>> one memset.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>> CC: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
>>
>> ---
>> Liam:
>>
>> This looks obvious, so I just run the ./maple test to see it doesn't
>> break anything.
>>
>
>The big node also includes the type, which isn't cleared. However it is
>unconditionally set below, so your change log is not correct in the
>statement that it is fully cleared, but the code will work and it is
>worth fixing what you have found.
>
I will mention this next time.
>If you are sending more than one patch, it is better to make a
>cover letter to explain your series.
>
Ok, I will add a cover letter next time.
>It is probably worth re-spinning the series to fix the comment, but
>these changes look good. I'll have a closer look later.
>
Thank,
I will re-arrange the change log and add a cover letter for v2.
>> Do you think I need to add a benchmark and run a perf for this kind of
>> change?
>
>No, you are doing less work so this should be better as long as it is
>correct. Zeroing can be expensive on some archs so this is good.
>
>> ---
>> lib/maple_tree.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>> index d8f10773e451..911c5e04e634 100644
>> --- a/lib/maple_tree.c
>> +++ b/lib/maple_tree.c
>> @@ -3134,10 +3134,7 @@ static inline void mast_fill_bnode(struct maple_subtree_state *mast,
>> bool cp = true;
>> unsigned char split;
>>
>> - memset(mast->bn->gap, 0, sizeof(unsigned long) * ARRAY_SIZE(mast->bn->gap));
>> - memset(mast->bn->slot, 0, sizeof(unsigned long) * ARRAY_SIZE(mast->bn->slot));
>> - memset(mast->bn->pivot, 0, sizeof(unsigned long) * ARRAY_SIZE(mast->bn->pivot));
>> - mast->bn->b_end = 0;
>> + memset(mast->bn, 0, sizeof(struct maple_big_node));
>>
>> if (mte_is_root(mas->node)) {
>> cp = false;
>> --
>> 2.34.1
>>
--
Wei Yang
Help you, Help me
