Re: [PATCH 2/2] maple_tree: memset maple_big_node as a whole

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

* Wei Yang <richard.weiyang@gmail.com> [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@gmail.com>
> CC: Liam R. Howlett <Liam.Howlett@Oracle.com>
> 
> ---
> 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.

If you are sending more than one patch, it is better to make a
cover letter to explain your series.

It is probably worth re-spinning the series to fix the comment, but
these changes look good.  I'll have a closer look later.

> 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
>