Re: Ping: [PATCH v2 8/8] Arm/optee: don't open-code xzalloc_flex_struct()

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi Jan,

Jan Beulich writes:

> On 21.04.2021 16:59, Jan Beulich wrote:
>> The current use of xzalloc_bytes() in optee is nearly an open-coded
>> version  of xzalloc_flex_struct(), which was introduced after the driver
>> was merged.
>> 
>> The main difference is xzalloc_bytes() will also force the allocation to
>> be SMP_CACHE_BYTES aligned and therefore avoid sharing the cache line.
>> While sharing the cache line can have an impact on the performance, this
>> is also true for most of the other users of x*alloc(), x*alloc_array(),
>> and x*alloc_flex_struct(). So if we want to prevent sharing cache lines,
>> arranging for this should be done in the allocator itself.
>> 
>> In this case, we don't need stricter alignment than what the allocator 
>> provides. Hence replace the call to xzalloc_bytes() with one of
>> xzalloc_flex_struct().
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Signed-off-by: Julien Grall <julien@xxxxxxx>
>> ---
>> v2: Use commit message very close to what was suggested by Julien.
>
> I realize I forgot to Cc you on the v2 submission, but I didn't hear
> back on v1 from you either. May I ask for an ack or otherwise?

You already had discussion on v1, so I just wanted to see how it will
conclude before stepping in.

I'm fine with this change:

Acked-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>

>
>> --- a/xen/arch/arm/tee/optee.c
>> +++ b/xen/arch/arm/tee/optee.c
>> @@ -529,8 +529,7 @@ static struct optee_shm_buf *allocate_op
>>      while ( unlikely(old != atomic_cmpxchg(&ctx->optee_shm_buf_pages,
>>                                             old, new)) );
>>  
>> -    optee_shm_buf = xzalloc_bytes(sizeof(struct optee_shm_buf) +
>> -                                  pages_cnt * sizeof(struct page *));
>> +    optee_shm_buf = xzalloc_flex_struct(struct optee_shm_buf, pages, 
>> pages_cnt);
>>      if ( !optee_shm_buf )
>>      {
>>          err_code = -ENOMEM;
>> 
>> 


-- 
Volodymyr Babchuk at EPAM