Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

On Tue, 2024-02-27 at 07:02 +0000, Christophe Leroy wrote:
> > It could be possible to initialize the new field for each arch to
> > 0, but
> > instead simply inialize the field with a C99 struct inializing
> > syntax.
> 
> Why doing a full init of the struct when all fields are re-written a
> few 
> lines after ?
> 
> If I take the exemple of powerpc function slice_find_area_bottomup():
> 
>         struct vm_unmapped_area_info info;
> 
>         info.flags = 0;
>         info.length = len;
>         info.align_mask = PAGE_MASK & ((1ul << pshift) - 1);
>         info.align_offset = 0;
> 
> For me it looks better to just add:
> 
>         info.new_field = 0; /* or whatever value it needs to have */

Hi,

Thanks for taking a look. Yes, I guess that should have some
justification. I was thinking of two reasons:
1. No future additions of optional parameters would need to make tree
wide changes like this.
2. The change is easier to review and get correct because the necessary
context is within a single line. For example, in that function some of
members are set within a while loop. The place you pointed seems to be
the correct one, but a diff that had the new field set after:
   info.high_limit = addr;
...would look correct too, but not be.

What is the concern with C99 initialization? FWIW, the full series also
removes an indirect branch, and probably is a net win for performance
in this path.