Re: [Xen-devel] [PATCH V4 4/8] xen/common: Introduce _xrealloc function

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.09.2019 17:03, Oleksandr wrote:
> On 16.09.19 13:13, Jan Beulich wrote:
>> On 13.09.2019 17:35, Oleksandr Tyshchenko wrote:
>>> --- a/xen/common/xmalloc_tlsf.c
>>> +++ b/xen/common/xmalloc_tlsf.c
>>> @@ -598,6 +598,58 @@ void *_xzalloc(unsigned long size, unsigned long align)
>>>       return p ? memset(p, 0, size) : p;
>>>   }
>>>   
>>> +void *_xrealloc(void *ptr, unsigned long size, unsigned long align)
>>> +{
>>> +    unsigned long curr_size, tmp_size;
>>> +    void *p;
>>> +
>>> +    if ( !size )
>>> +    {
>>> +        xfree(ptr);
>>> +        return ZERO_BLOCK_PTR;
>>> +    }
>>> +
>>> +    if ( ptr == NULL || ptr == ZERO_BLOCK_PTR )
>>> +        return _xmalloc(size, align);
>>> +
>>> +    if ( !((unsigned long)ptr & (PAGE_SIZE - 1)) )
>>> +        curr_size = PFN_ORDER(virt_to_page(ptr)) << PAGE_SHIFT;
>> While the present MAX_ORDER setting will prevent allocations of
>> 4GiB or above from succeeding, may I ask that you don't introduce
>> latent issues in case MAX_ORDER would ever need bumping?
> Sure (I assume, you are talking about possible truncation):
> 
> if ( !((unsigned long)ptr & (PAGE_SIZE - 1)) )
>      curr_size = (unsigned long)PFN_ORDER(virt_to_page(ptr)) << PAGE_SHIFT;

Yes.

>>> +            ROUNDUP_SIZE(tmp_size);
>>> +
>>> +    if ( tmp_size <= curr_size && ((unsigned long)ptr & (align - 1)) == 0 )
>>> +        return ptr; /* the size and alignment fit in already allocated 
>>> space */
>> You also don't seem to ever update ptr in case you want to use the
>> (head) padding, i.e. you'd hand back a pointer to a block which the
>> caller would assume extends past its actual end. I think you want
>> to calculate the new tentative pointer (taking the requested
>> alignment into account), and only from that calculate curr_size
>> (which perhaps would better be named "usable" or "space" or some
>> such). Obviously the (head) padding block may need updating, too.
> 
> I am afraid I don't completely understand your point here. And sorry for 
> the maybe naive question, but what is the "(head) padding" here?

The very padding talked about earlier. I did add "(head)" to clarify
it's that specific case - after all tail padding is far more common.

Jan

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