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

[Oleksandr <olekstysh@xxxxxxxxx>]

On 21.08.19 11:09, Paul Durrant wrote:

Hi, Paul

> >   }
> >
> > +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;
> > +    else
> > +    {
> > +        struct bhdr *b;
> > +        b = (struct bhdr *)((char *)ptr - BHDR_OVERHEAD);
> > +        curr_size = b->size & BLOCK_SIZE_MASK;
> > +    }
> That seconds clause is not going to give you the block size if the previous 
> allocation had alignment padding. You'll need to check the FREE_BLOCK bit to 
> tell whether it's a real block header or the 'fake' alignment header and then 
> maybe walk backwards onto the real header. See the code in xfree().
Indeed. Thank you for the pointer.


> 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?

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;
     }

@@ -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 */

     p = _xmalloc(size, align);
     if ( p )

(END)


Did I get your point correct?


--
Regards,

Oleksandr Tyshchenko


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