Re: [Xen-devel] [PATCH V3 3/8] xen/common: Introduce _xrealloc function

[Oleksandr <olekstysh@gmail.com>]

On 21.08.19 18:47, Paul Durrant wrote:

Hi

>>
>> You should also check whether the new requested alignment is compatible with the existing block
>> alignment
>>
>>
>> I am wondering:
>>
>> In case when we use only type-safe construction for the basic allocation
>> (xmalloc) and type-safe construction for the re-allocation
>> (xrealloc_flex_struct) over the same pointer of the correct type, will
>> we get a possible alignment incompatibility?
> You shouldn't but you're trusting that no-one will want to call the function directly with a higher alignment.

Yes, it is best to be on the safe side.


>   
>>
>> This is how I see it:
>>
>> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
>> index eecae2e..f90f453 100644
>> --- a/xen/common/xmalloc_tlsf.c
>> +++ b/xen/common/xmalloc_tlsf.c
>> @@ -616,8 +616,15 @@ void *_xrealloc(void *ptr, unsigned long size,
>> unsigned long align)
>>            curr_size = PFN_ORDER(virt_to_page(ptr)) << PAGE_SHIFT;
>>        else
>>        {
>> -        struct bhdr *b;
>> -        b = (struct bhdr *)((char *)ptr - BHDR_OVERHEAD);
>> +        struct bhdr *b = (struct bhdr *)((char *)ptr - BHDR_OVERHEAD);
>> +
>> +        if ( b->size & FREE_BLOCK )
>> +        {
>> +            p = (char *)ptr - (b->size & ~FREE_BLOCK);
>> +            b = (struct bhdr *)((char *)p - BHDR_OVERHEAD);
>> +            ASSERT(!(b->size & FREE_BLOCK));
>> +        }
>> +
>>            curr_size = b->size & BLOCK_SIZE_MASK;
> Yes, that looks ok for getting the size right...
>
>>        }
>>
>> @@ -630,8 +637,8 @@ void *_xrealloc(void *ptr, unsigned long size,
>> unsigned long align)
>>            tmp_size = ( tmp_size < MIN_BLOCK_SIZE ) ? MIN_BLOCK_SIZE :
>>                ROUNDUP_SIZE(tmp_size);
>>
>> -    if ( tmp_size <= curr_size ) /* fits in current block */
>> -        return ptr;
>> +    if ( tmp_size <= curr_size && ((unsigned long)ptr & (align - 1)) == 0 )
>> +        return ptr; /* fits in current block */
> ...and that should deal with the alignment too (although you may want to mention the alignment part in the comment too)
>
>>        p = _xmalloc(size, align);
>>        if ( p )
>>
>> (END)
>>
>>
>> Did I get your point correct?
>>
> Yes, with those changes I think it will look ok.

Thank you.


-- 
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel