Re: [Xen-devel] [PATCH 1 of 2] tools/libxc: Remus Checkpoint Compression

[Shriram Rajagopalan <rshriram@xxxxxxxxx>]

On Thu, Jun 16, 2011 at 1:57 PM, Brendan Cully <brendan@xxxxxxxxx> wrote:

On 2011-06-16, at 10:46 AM, Shriram Rajagopalan wrote:

On Thu, Jun 16, 2011 at 12:34 PM, Brendan Cully <brendan.cully@xxxxxxxxxx> wrote:

On Thursday, 16 June 2011 at 08:50, Shriram Rajagopalan wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@xxxxxxxxx>
> # Date 1308238095 25200
> # Node ID 5dbafaf24c7036f3e24e4387797b32e59d732ac6
> # Parent  23c068b109236657ededa3e3b7f180346a5cd9f9
> tools/libxc: Remus Checkpoint Compression
>
> Instead of sending dirty pages of guest memory as-is, use a simple compression
> algorithm that sends a RLE-encoded XOR of the page against its last sent copy.
> A small LRU cache is used to hold recently dirtied pages. Pagetable pages are
> sent as-is, as they are canonicalized at sender side and uncanonicalized at
> receiver.
>
> Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>

...

> @@ -1395,6 +1455,7 @@
>          }
>
>          pagebuf.nr_physpages = pagebuf.nr_pages = 0;
> +        pagebuf.compbuf_pos = pagebuf.compbuf_size = 0;
>
>          n += j; /* crude stats */
>

Is that zeroing compbuf_size at the end of every round? Does that mean
you're doing a realloc for every single page you ever receive? Ouch.

We have already talked about it offline and I dont mind explaining it "again" :).

Its a realloc for every compressed chunk. And a compressed chunk is
made of as many pages as the 16MB outbuf on sender side can hold.

Actually, I never received a reply yesterday when I asked this exact question, which is why I'm replying on-list. 

I still don't understand why you're zeroing this field while you still have a buffer allocated. It's misleading and can (and probably will) cause unnecessary reallocs. Glad to hear you're not doing this at every page though.

That code follows the same pattern as the original receive code.

Usual receive code:
buf->nr_physpages += countpages;
    if (!buf->pages) {
        if (!(buf->pages = malloc(buf->nr_physpages * PAGE_SIZE))) {

            ERROR("Could not allocate page buffer");
            return -1;
        }
    } else {
        if (!(ptmp = realloc(buf->pages, buf->nr_physpages * PAGE_SIZE))) {
            ERROR("Could not reallocate page buffer");

            return -1;
        }
        buf->pages = ptmp;
    }
    if ( RDEXACT(fd, buf->pages + oldcount * PAGE_SIZE, countpages * PAGE_SIZE) ) {
        PERROR("Error when reading pages");

        return -1;
    }

****
The compression code:
buf->compbuf_size += compbuf_size;
        if (!buf->pages) {
            if (!(buf->pages = malloc(buf->compbuf_size))) {
                ERROR("Could not allocate compression buffer");

                return -1;
            }
        } else {
            if (!(ptmp = realloc(buf->pages, buf->compbuf_size))) {
                ERROR("Could not reallocate compression buffer");

                return -1;
            }
            buf->pages = ptmp;
        }
        if ( RDEXACT(fd, buf->pages + (buf->compbuf_size - compbuf_size),
                     compbuf_size) ) {

            PERROR("Error when reading compression buffer");
            return -1;
        }
*****

In usual code, the nr_physpages is set to 0 in the pagebuf_get() and in the main restore loop.

Same logic is applied here. The original code receives pages in batches of 1024 max. The
compression code receives pages in compressed chunks.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel