| Message ID | 1363856971-4601-11-git-send-email-owasserm@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 21/03/2013 10:09, Orit Wasserman ha scritto: > + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; > + f->iov[f->iovcnt++].iov_len = size; > + > + f->is_write = 1; > + f->bytes_xfer += size; > + > + if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { > + qemu_fflush(f); > } It should not be complex to check if f->iov[f->iovcnt - 1] can be extended? This could remove many system calls when you have many consecutive zero pages. Paolo
Can you add a "flag" or something to indicate that the iov pointer belongs to RAM and not to device state? That way, I could re-use this code for RDMA - if I see this flag, I will know to send to RDMA..... - Michael On 03/21/2013 05:09 AM, Orit Wasserman wrote: > This allow us to add a buffer to the iovec to send without copying it > into the static buffer. > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > --- > include/migration/qemu-file.h | 5 +++++ > savevm.c | 42 ++++++++++++++++++++++++------------------ > 2 files changed, 29 insertions(+), 18 deletions(-) > > diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h > index 8d3da9b..5168be2 100644 > --- a/include/migration/qemu-file.h > +++ b/include/migration/qemu-file.h > @@ -75,6 +75,11 @@ int qemu_fclose(QEMUFile *f); > int64_t qemu_ftell(QEMUFile *f); > void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size); > void qemu_put_byte(QEMUFile *f, int v); > +/* > + * put_buffer without copying the buffer. > + * The buffer should be available till it is sent. > + */ > +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size); > > static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) > { > diff --git a/savevm.c b/savevm.c > index 40d96f4..32a506e 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -629,6 +629,22 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size) > { > int l; > > + while (size > 0) { > + l = IO_BUF_SIZE - f->buf_index; > + if (l > size) { > + l = size; > + } > + memcpy(f->buf + f->buf_index, buf, l); > + f->buf_index += l; > + f->is_write = 1; > + qemu_put_buffer_no_copy(f, f->buf + (f->buf_index - l), l); > + buf += l; > + size -= l; > + } > +} > + > +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) > +{ > if (f->last_error) { > return; > } > @@ -639,24 +655,14 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size) > abort(); > } > > - while (size > 0) { > - l = IO_BUF_SIZE - f->buf_index; > - if (l > size) > - l = size; > - memcpy(f->buf + f->buf_index, buf, l); > - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; > - f->iov[f->iovcnt++].iov_len = l; > - f->is_write = 1; > - f->buf_index += l; > - f->bytes_xfer += l; > - buf += l; > - size -= l; > - if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { > - qemu_fflush(f); > - if (qemu_file_get_error(f)) { > - break; > - } > - } > + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; > + f->iov[f->iovcnt++].iov_len = size; > + > + f->is_write = 1; > + f->bytes_xfer += size; > + > + if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { > + qemu_fflush(f); > } > } >
On 03/23/2013 06:27 PM, Michael R. Hines wrote: > Can you add a "flag" or something to indicate that the iov pointer belongs to RAM and not to device state? > > That way, I could re-use this code for RDMA - if I see this flag, I will know to send to RDMA..... This function is called only for ram pages so no need for flag. Orit > > - Michael > > > On 03/21/2013 05:09 AM, Orit Wasserman wrote: >> This allow us to add a buffer to the iovec to send without copying it >> into the static buffer. >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> --- >> include/migration/qemu-file.h | 5 +++++ >> savevm.c | 42 ++++++++++++++++++++++++------------------ >> 2 files changed, 29 insertions(+), 18 deletions(-) >> >> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h >> index 8d3da9b..5168be2 100644 >> --- a/include/migration/qemu-file.h >> +++ b/include/migration/qemu-file.h >> @@ -75,6 +75,11 @@ int qemu_fclose(QEMUFile *f); >> int64_t qemu_ftell(QEMUFile *f); >> void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size); >> void qemu_put_byte(QEMUFile *f, int v); >> +/* >> + * put_buffer without copying the buffer. >> + * The buffer should be available till it is sent. >> + */ >> +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size); >> >> static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) >> { >> diff --git a/savevm.c b/savevm.c >> index 40d96f4..32a506e 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -629,6 +629,22 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size) >> { >> int l; >> >> + while (size > 0) { >> + l = IO_BUF_SIZE - f->buf_index; >> + if (l > size) { >> + l = size; >> + } >> + memcpy(f->buf + f->buf_index, buf, l); >> + f->buf_index += l; >> + f->is_write = 1; >> + qemu_put_buffer_no_copy(f, f->buf + (f->buf_index - l), l); >> + buf += l; >> + size -= l; >> + } >> +} >> + >> +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) >> +{ >> if (f->last_error) { >> return; >> } >> @@ -639,24 +655,14 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size) >> abort(); >> } >> >> - while (size > 0) { >> - l = IO_BUF_SIZE - f->buf_index; >> - if (l > size) >> - l = size; >> - memcpy(f->buf + f->buf_index, buf, l); >> - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; >> - f->iov[f->iovcnt++].iov_len = l; >> - f->is_write = 1; >> - f->buf_index += l; >> - f->bytes_xfer += l; >> - buf += l; >> - size -= l; >> - if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { >> - qemu_fflush(f); >> - if (qemu_file_get_error(f)) { >> - break; >> - } >> - } >> + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; >> + f->iov[f->iovcnt++].iov_len = size; >> + >> + f->is_write = 1; >> + f->bytes_xfer += size; >> + >> + if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { >> + qemu_fflush(f); >> } >> } >> >
----- Messaggio originale ----- > Da: "Michael R. Hines" <mrhines@linux.vnet.ibm.com> > A: "Orit Wasserman" <owasserm@redhat.com> > Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com, "chegu vinod" <chegu_vinod@hp.com>, > quintela@redhat.com > Inviato: Sabato, 23 marzo 2013 17:27:49 > Oggetto: Re: [Qemu-devel] [RFC 10/12] Add qemu_put_buffer_no_copy > > Can you add a "flag" or something to indicate that the iov pointer > belongs to RAM and not to device state? > > That way, I could re-use this code for RDMA - if I see this flag, I > will know to send to RDMA..... I am not sure you can, because the function will be preceded by a qemu_put_be64 to store the header. That header is not sent in the RDMA case, isn't it? Paolo
Right, the header's not used - but, are we certain that put_buffer_copy() will *always* be used for RAM in the future? - Michael On 03/25/2013 09:05 AM, Paolo Bonzini wrote: > > ----- Messaggio originale ----- >> Da: "Michael R. Hines" <mrhines@linux.vnet.ibm.com> >> A: "Orit Wasserman" <owasserm@redhat.com> >> Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com, "chegu vinod" <chegu_vinod@hp.com>, >> quintela@redhat.com >> Inviato: Sabato, 23 marzo 2013 17:27:49 >> Oggetto: Re: [Qemu-devel] [RFC 10/12] Add qemu_put_buffer_no_copy >> >> Can you add a "flag" or something to indicate that the iov pointer >> belongs to RAM and not to device state? >> >> That way, I could re-use this code for RDMA - if I see this flag, I >> will know to send to RDMA..... > I am not sure you can, because the function will be preceded by > a qemu_put_be64 to store the header. That header is not sent in > the RDMA case, isn't it? > > Paolo >
> Right, the header's not used - but, are we certain that > put_buffer_copy() will *always* be used for RAM in the future? I think you should not make any assumption and proceed as if this series didn't exist. Paolo
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h index 8d3da9b..5168be2 100644 --- a/include/migration/qemu-file.h +++ b/include/migration/qemu-file.h @@ -75,6 +75,11 @@ int qemu_fclose(QEMUFile *f); int64_t qemu_ftell(QEMUFile *f); void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size); void qemu_put_byte(QEMUFile *f, int v); +/* + * put_buffer without copying the buffer. + * The buffer should be available till it is sent. + */ +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size); static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) { diff --git a/savevm.c b/savevm.c index 40d96f4..32a506e 100644 --- a/savevm.c +++ b/savevm.c @@ -629,6 +629,22 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size) { int l; + while (size > 0) { + l = IO_BUF_SIZE - f->buf_index; + if (l > size) { + l = size; + } + memcpy(f->buf + f->buf_index, buf, l); + f->buf_index += l; + f->is_write = 1; + qemu_put_buffer_no_copy(f, f->buf + (f->buf_index - l), l); + buf += l; + size -= l; + } +} + +void qemu_put_buffer_no_copy(QEMUFile *f, const uint8_t *buf, int size) +{ if (f->last_error) { return; } @@ -639,24 +655,14 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size) abort(); } - while (size > 0) { - l = IO_BUF_SIZE - f->buf_index; - if (l > size) - l = size; - memcpy(f->buf + f->buf_index, buf, l); - f->iov[f->iovcnt].iov_base = f->buf + f->buf_index; - f->iov[f->iovcnt++].iov_len = l; - f->is_write = 1; - f->buf_index += l; - f->bytes_xfer += l; - buf += l; - size -= l; - if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { - qemu_fflush(f); - if (qemu_file_get_error(f)) { - break; - } - } + f->iov[f->iovcnt].iov_base = (uint8_t *)buf; + f->iov[f->iovcnt++].iov_len = size; + + f->is_write = 1; + f->bytes_xfer += size; + + if (f->buf_index >= IO_BUF_SIZE || f->iovcnt >= MAX_IOV_SIZE) { + qemu_fflush(f); } }
This allow us to add a buffer to the iovec to send without copying it into the static buffer. Signed-off-by: Orit Wasserman <owasserm@redhat.com> --- include/migration/qemu-file.h | 5 +++++ savevm.c | 42 ++++++++++++++++++++++++------------------ 2 files changed, 29 insertions(+), 18 deletions(-)