Re: [Xen-devel] Re: [PATCH 0 of 2 V5] libxc: checkpoint compression

[Shriram Rajagopalan <rshriram@xxxxxxxxx>]

On Tue, Nov 8, 2011 at 9:20 AM, Shriram Rajagopalan <rshriram@xxxxxxxxx> wrote:

On Tue, Nov 8, 2011 at 9:16 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

On Tue, 2011-11-08 at 17:13 +0000, Shriram Rajagopalan wrote:
> On Tue, Nov 8, 2011 at 9:02 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> wrote:
>         On Tue, 2011-11-08 at 16:51 +0000, Shriram Rajagopalan wrote:
>         > On Fri, Nov 4, 2011 at 12:21 PM, Shriram Rajagopalan
>         > <rshriram@xxxxxxxxx> wrote:
>         >         Why posix_memalign?
>         >
>         >         The compression code involves a lot of memcpys at 4K
>         >         granularity (dirty pages
>         >         copied from domU's memory to internal cache/page
>         buffers etc).
>         >         I would like to
>         >         keep these memcpys page aligned for purposes of
>         speed. The
>         >         source pages
>         >         (from domU) are already aligned. The destination
>         pages
>         >         allocated by the
>         >         compression code need to be page aligned.
>         >
>         >         correct me if I am wrong:
>         >          mallocing a huge buffer for this purpose is not
>         optimal.
>         >         malloc aligns allocations
>         >          on 16byte (or 8byte) granularity but if a 4K region
>         straddles
>         >         across two physical
>         >         memory frames, then the memcpy is going to be
>         suboptimal.
>         >         OTOH, memalign
>         >         ensures that we are dealing with just 2 memory
>         frames as
>         >         opposed
>         >         to 3 (possible) frames in malloc.
>         >
>         >         A simple 8Mb memcpy test shows an average of 500us
>         overhead
>         >         for malloc
>         >         based allocation compared to posix_memalign based
>         allocation.
>         >         While this
>         >         might seem low, the checkpoints are being taken at
>         high
>         >         frequency
>         >         (every 20ms for instance).
>         >
>         >         It is not okay to use malloc on other platforms. I
>         simply dont
>         >         have access to other
>         >         platforms to test their equivalent versions.  Short
>         of using
>         >         something
>         >         like qemu_memalign function.
>         >
>         >         I am open to suggestions :)
>
>
>         This is due to minios (aka stubdoms) not having
>         posix_memalign, right?
>
>         minios (or rather newlib) does appear to have memalign though,
>         which if
>         true would also work, right? You could potentially also
>         implement
>         posix_memalign in terms of memalign on minios and avoid the
>         ifdef.
>
>
> Sounds good. In that case, can I just post a patch to minios,
> implementing posix_memalign and will you then directly take the
> previous version V4 of this patch series (the one without #ifdefs) ?

Well, *I* won't be taking any version of the patch but that sounds like
a sane plan to me, assuming V4 builds after your minios patch.

oops. sorry.. I was referring to IanJ. 

Just realized i forgot to state why I had to the __linux__.

a. minios lacks posix_memalign
b. I looked up online. solaris has no posix_memalign. I am not sure about netbsd.
c. in tools/libxc/
xc_solaris.c uses memalign
xc_netbsd.c uses valloc
xc_minios.c uses memalign
xc_linux_osdep.c uses posix_memalign!

further posix_memalign manpage states that
"posix_memalign() verifies that alignment matches the requirements detailed above.
 memalign() may not check that the boundary argument is correct."

fortified by newlib-1.16.0's comments in mallocr.c (newlib-1.16.0/newlib/libc/stdlib/)
"The alignment argument must be a power of two. This property is not
checked by memalign, so misuse may result in random runtime errors."

Judging by all this mess, I thought i was better off doing a #ifdef __linux__ and
resorting to simple malloc for the other platforms.

One alternative would be to add the xc_memalign function alone, that was removed
by c/s 22520.

-void *xc_memalign(size_t alignment, size_t size)
-{
-#if defined(_POSIX_C_SOURCE) && !defined(__sun__)
-    int ret;
-    void *ptr;
-    ret = posix_memalign(&ptr, alignment, size);
-    if (ret != 0)
-        return NULL;
-    return ptr;
-#elif defined(__NetBSD__) || defined(__OpenBSD__)
-    return valloc(size);
-#else
-    return memalign(alignment, size);
-#endif
-}
-

shriram
 

>
> thanks
> shriram
>
>
>         Ian.
>
>         >
>         >         shriram
>         >
>         >
>         >
>         > Ping.
>         >
>         >
>         >         On Fri, Nov 4, 2011 at 5:14 AM, Ian Jackson
>         >         <Ian.Jackson@xxxxxxxxxxxxx> wrote:
>         >                 rshriram@xxxxxxxxx writes ("[PATCH 0 of 2
>         V5] libxc:
>         >                 checkpoint compression"):
>         >                 > This patch series adds checkpoint
>         compression
>         >                 functionality, while
>         >                 > running under Remus.
>         >
>         >                 ...
>         >                 > Changes since last version:
>         >                 > 1. use posix_memalign only on linux
>         platforms and
>         >                 switch to normal malloc for
>         >                 >    the rest. stubdom compiles
>         successfully.
>         >
>         >
>         >                 Looking at this in more detail, I don't
>         understand why
>         >                 you're using
>         >                 posix_memalign rather than just malloc,
>         anyway.  If
>         >                 it's necessary to
>         >                 use posix_memalign on Linux, why is it OK to
>         use
>         >                 malloc on other
>         >                 platforms ?
>         >
>         >                 Also this #ifdef is quite ugly.
>         >
>         >                 Ian.
>         >
>         >
>         >
>         >
>
>
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel