| Message ID | 1363881940-27505-9-git-send-email-owasserm@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 21, 2013 at 06:05:39PM +0200, Orit Wasserman wrote: > This will remove an unneeded copy of guest memory pages. > For the page header and device state we still copy the data to the > static buffer the other option is to allocate the memory on demand > which is more expensive. > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> Okay so with this, the _nocopy can be later rewritten to do vmsplice to save another copy? Cool. > --- > arch_init.c | 2 +- > savevm.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 98e2bc6..27b53eb 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -481,7 +481,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > /* XBZRLE overflow or normal page */ > if (bytes_sent == -1) { > bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); > - qemu_put_buffer(f, p, TARGET_PAGE_SIZE); > + qemu_put_buffer_no_copy(f, p, TARGET_PAGE_SIZE); > bytes_sent += TARGET_PAGE_SIZE; > acct_info.norm_pages++; > } > diff --git a/savevm.c b/savevm.c > index fbfb9e3..50e8fb2 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -634,7 +634,7 @@ void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) > abort(); > } > > - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; > + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; > f->iov[f->iovcnt++].iov_len = size; > > f->is_write = 1; > -- > 1.7.11.7
Orit Wasserman <owasserm@redhat.com> wrote: > This will remove an unneeded copy of guest memory pages. > For the page header and device state we still copy the data to the > static buffer the other option is to allocate the memory on demand > which is more expensive. > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > --- > arch_init.c | 2 +- > savevm.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 98e2bc6..27b53eb 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -481,7 +481,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > /* XBZRLE overflow or normal page */ > if (bytes_sent == -1) { > bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); > - qemu_put_buffer(f, p, TARGET_PAGE_SIZE); > + qemu_put_buffer_no_copy(f, p, TARGET_PAGE_SIZE); > bytes_sent += TARGET_PAGE_SIZE; > acct_info.norm_pages++; > } Once here, shouldn't we also change: block-migration.c::blk_send() qemu_put_buffer(f, blk->buf, BLOCK_SIZE); to nocopy? Again, this can be an additional patch. > diff --git a/savevm.c b/savevm.c > index fbfb9e3..50e8fb2 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -634,7 +634,7 @@ void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) > abort(); > } > > - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; > + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; > f->iov[f->iovcnt++].iov_len = size; This bit should be in the previous patch, the error that I pointed? Later, Juan.
On 03/21/2013 07:37 PM, Juan Quintela wrote: > Orit Wasserman <owasserm@redhat.com> wrote: >> This will remove an unneeded copy of guest memory pages. >> For the page header and device state we still copy the data to the >> static buffer the other option is to allocate the memory on demand >> which is more expensive. >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> --- >> arch_init.c | 2 +- >> savevm.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch_init.c b/arch_init.c >> index 98e2bc6..27b53eb 100644 >> --- a/arch_init.c >> +++ b/arch_init.c >> @@ -481,7 +481,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) >> /* XBZRLE overflow or normal page */ >> if (bytes_sent == -1) { >> bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); >> - qemu_put_buffer(f, p, TARGET_PAGE_SIZE); >> + qemu_put_buffer_no_copy(f, p, TARGET_PAGE_SIZE); >> bytes_sent += TARGET_PAGE_SIZE; >> acct_info.norm_pages++; >> } > > Once here, shouldn't we also change: > > block-migration.c::blk_send() > > qemu_put_buffer(f, blk->buf, BLOCK_SIZE); > > to nocopy? Sadly no, I looked at the code and I saw the buffer is freed immediately after call blk_send :(. look at mig_save_device_dirty. Orit > > Again, this can be an additional patch. > > >> diff --git a/savevm.c b/savevm.c >> index fbfb9e3..50e8fb2 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -634,7 +634,7 @@ void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) >> abort(); >> } >> >> - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; >> + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; >> f->iov[f->iovcnt++].iov_len = size; > > This bit should be in the previous patch, the error that I pointed? > > Later, Juan. >
On Thu, Mar 21, 2013 at 08:08:58PM +0200, Orit Wasserman wrote: > On 03/21/2013 07:37 PM, Juan Quintela wrote: > > Orit Wasserman <owasserm@redhat.com> wrote: > >> This will remove an unneeded copy of guest memory pages. > >> For the page header and device state we still copy the data to the > >> static buffer the other option is to allocate the memory on demand > >> which is more expensive. > >> > >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> > >> --- > >> arch_init.c | 2 +- > >> savevm.c | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch_init.c b/arch_init.c > >> index 98e2bc6..27b53eb 100644 > >> --- a/arch_init.c > >> +++ b/arch_init.c > >> @@ -481,7 +481,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > >> /* XBZRLE overflow or normal page */ > >> if (bytes_sent == -1) { > >> bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); > >> - qemu_put_buffer(f, p, TARGET_PAGE_SIZE); > >> + qemu_put_buffer_no_copy(f, p, TARGET_PAGE_SIZE); > >> bytes_sent += TARGET_PAGE_SIZE; > >> acct_info.norm_pages++; > >> } > > > > Once here, shouldn't we also change: > > > > block-migration.c::blk_send() > > > > qemu_put_buffer(f, blk->buf, BLOCK_SIZE); > > > > to nocopy? > Sadly no, I looked at the code and I saw the buffer is freed immediately > after call blk_send :(. look at mig_save_device_dirty. > > Orit Hmm if _nocopy also implies it is not safe to free the buffer immediately, then I'd rather put this in a name e.g. _async instead of _nocopy. > > > > Again, this can be an additional patch. > > > > > >> diff --git a/savevm.c b/savevm.c > >> index fbfb9e3..50e8fb2 100644 > >> --- a/savevm.c > >> +++ b/savevm.c > >> @@ -634,7 +634,7 @@ void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) > >> abort(); > >> } > >> > >> - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; > >> + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; > >> f->iov[f->iovcnt++].iov_len = size; > > > > This bit should be in the previous patch, the error that I pointed? > > > > Later, Juan. > >
diff --git a/arch_init.c b/arch_init.c index 98e2bc6..27b53eb 100644 --- a/arch_init.c +++ b/arch_init.c @@ -481,7 +481,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) /* XBZRLE overflow or normal page */ if (bytes_sent == -1) { bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); - qemu_put_buffer(f, p, TARGET_PAGE_SIZE); + qemu_put_buffer_no_copy(f, p, TARGET_PAGE_SIZE); bytes_sent += TARGET_PAGE_SIZE; acct_info.norm_pages++; } diff --git a/savevm.c b/savevm.c index fbfb9e3..50e8fb2 100644 --- a/savevm.c +++ b/savevm.c @@ -634,7 +634,7 @@ void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) abort(); } - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; f->iov[f->iovcnt++].iov_len = size; f->is_write = 1;
This will remove an unneeded copy of guest memory pages. For the page header and device state we still copy the data to the static buffer the other option is to allocate the memory on demand which is more expensive. Signed-off-by: Orit Wasserman <owasserm@redhat.com> --- arch_init.c | 2 +- savevm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)