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

[Oleksandr <olekstysh@xxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel