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

Re: [Minios-devel] [UNIKRAFT PATCH] lib/ukalloc: uk_getmallocsize should return size not order

Hi Radu,

On 07/24/2018 11:47 AM, Radu Nicolau wrote:
        @@ -135,8 +135,24 @@ static void *uk_get_real_start(const void *ptr)
           static size_t uk_getmallocsize(const void *ptr)
                 size_t *intptr = uk_get_real_start(ptr);
        +       size_t mallocsize = __PAGE_SIZE << (*intptr);
           -     return *intptr;
        +       if (((uintptr_t) ptr & (~__PAGE_MASK)) == 0) {
        +               /*
        +                * special case: the memory was page-aligned
        +                * In this case the allocated size should not
        account for the
        +                * previous page which was used for storing the
        +                */
        +               mallocsize -= __PAGE_SIZE;
        +       } else {
        +               /*
        +                * If pointer is not page aligned it means the
        header is
        +                * on the same page. This will break if metadata
        size increases
        +                */
        +               mallocsize -= sizeof(*intptr);
        +       }
        +       return mallocsize;

    I agree that it probably makes sense for uk_getmallocsize() to
    return the size and not the page order. Plus, it indeed fixes a bug
    in the comparison in uk_realloc_ifpages(). However, I don't
    understand checks you do to change mallocsize. uk_get_real_start()
    already takes care of this, so you're overcompensating, aren't you?

uk_getmallocsize() takes the order from uk_get_real_start() and does (pagesize<<order). The problem is that the result of that shift will include the size of the metadata. The check I introduces determines the size of the metadata depending on whether the pointer is page aligned or not which it then subtracts from the mallocsize. You need to do this as realloc needs the size of the malloc (without metadata size). I agree that functions check for the same thing, but the actions differ - uk_get_real_start() determines location of the metadata, uk_getmallocsize() uses the metadata and determines the allocated size.

Oh, right. I misread your if-statement. This sounds correct indeed.

If you submit a v2 with the slightly updated commit message, I'll flag this as reviewed.


Minios-devel mailing list



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