Re: [PATCH] xen/arm: Initialise memory type for struct kernel_info

[Michal Orzel <michal.orzel@xxxxxxx>]

On 23/12/2024 11:52, Luca Fancellu wrote:
> 
> 
> Hi Julien and Michal,
> 
>> On 23 Dec 2024, at 10:45, Julien Grall <julien@xxxxxxx> wrote:
>>
>> Hi,
>>
>> On 23/12/2024 10:42, Michal Orzel wrote:
>>> On 23/12/2024 11:06, Julien Grall wrote:
>>>>
>>>>
>>>> Hi Michal,
>>>>
>>>> On 23/12/2024 07:58, Michal Orzel wrote:
>>>>>
>>>>>
>>>>> On 20/12/2024 16:19, Luca Fancellu wrote:
>>>>>>
>>>>>>
>>>>>> Commit a14593e3995a ("xen/device-tree: Allow region overlapping with
>>>>>> /memreserve/ ranges") introduced a type in the 'struct membanks_hdr'
>>>>>> but forgot to update the 'struct kernel_info' initialiser, while
>>>>>> it doesn't lead to failures because the field is not currently
>>>>>> used while managing kernel_info structures, it's good to have it
>>>>>> for completeness.
>>>>> The last part "good to have it" does not sound like we need a Fixes tag.
>>>>
>>>> Reading the discussion, it sounds like ".type" is not always set and
>>>> this is not properly documented. This means that in the future we may
>>>> write a patch that requires to use ".type" and needs backported.
>>>>
>>>> But this would depend on this patch which was not tagged appropriately.
>>>> Therefore, I would argue it needs a fixes tag (even though this is a
>>>> latent bug) or at least a backport request.
>>> Setting explicitly a type for structures not requiring it, does not seem 
>>> beneficial for me.
>>
>> I have to disagree. If we set type everywhere, then the developer doesn't 
>> need to think whether ".type" is garbagge or not.
> 
> So, my thought was to at least initialise it on the structures that goes 
> around in the codebase,
> gnttab in find_host_extended_regions and shm_heap_banks in static-shmem.c 
> usage are less
> spreaded.
> 
> However I have no objection to always initialise them all, so that anyone 
> sending patches that
> use them, don’t need to think if the field is initialised or not.
> 
> I’m currently on leave, is it ok to wait until new year if any change is 
> required?
Hi Luca, yes, please send a new revision once you're back.

~Michal