Re: [PATCH 08/11] xen/arm: Reduce struct membank size on static shared memory

[Michal Orzel <michal.orzel@amd.com>]

Hi Luca,

On 04/04/2024 11:33, Luca Fancellu wrote:
> 
> 
>> On 22 Mar 2024, at 10:30, Michal Orzel <michal.orzel@amd.com> wrote:
>>
>> Hi Luca,
>>
>> On 12/03/2024 14:03, Luca Fancellu wrote:
>>>
>>>
>>> Currently the memory footprint of the static shared memory feature
>>> is impacting all the struct meminfo instances with memory space
>>> that is not going to be used.
>>>
>>> To solve this issue, rework the static shared memory extra
>>> information linked to the memory bank to another structure,
>>> struct shmem_membank_extra, and exploit the struct membank
>>> padding to host a pointer to that structure in a union with the
>> NIT: AFAICT the padding will be reused on Arm64 but on Arm32 there will still be 4B padding.
> 
> Yes, my purpose was to make clear that no additional space was needed
> for that pointer, should I rephrase it or it’s ok?
Since you are modifying this patch anyway, you can take the opportunity to rephrase it.
> 
> 
>>>
>>>
>>> +struct shared_meminfo {
>>> +    struct membanks common;
>>> +    struct membank bank[NR_SHMEM_BANKS];
>>> +    struct shmem_membank_extra extra[NR_SHMEM_BANKS];
>>> +};
>> Same as with meminfo, please add a BUILD_BUG_ON for padding between common and bank.
> 
> Sure
> 
>>>
>>>
>>> -static int __init append_shm_bank_to_domain(struct membanks *shm_mem,
>>> -                                            paddr_t start, paddr_t size,
>>> -                                            const char *shm_id)
>>> +static int __init
>>> +append_shm_bank_to_domain(struct shared_meminfo *kinfo_shm_mem, paddr_t start,
>> Is there any particular reason to prepend the shm_mem name with kinfo?
> 
> I think because usually kinfo is used to point to ’struct kernel_info’, instead here we point to
> 'struct shared_meminfo'
> 
>>
>> ~Michal
> 

~Michal