[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.