Message ID | 1353429936-29180-4-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Nov 20, 2012 at 4:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > buffered_rate_limit is called to prevent the RAM migration callback > from putting too much data in the buffer. So it has to check against > the amount of data currently in the buffer, not against the amount > of data that has been transferred so far. > > s->bytes_xfer is used to communicate between successive calls of > buffered_put_buffer. Buffered_rate_tick resets it every now and > then to prevent moving too much buffered data to the socket at > once. However, its value does not matter for the producer of the > data. > > Here is the result for migrating an idle guest with 3GB of memory > and ~360MB of non-zero memory: > > migrate_set_speed Before After > ------------------------------------------------------------ > default (32MB/sec) ~3 sec ~13 sec > infinite (10GB/sec) ~3 sec ~3 sec > > Note that before this patch, QEMU is transferring of 100 MB/sec > despite the rate limiting. > > Also fix an off-by-one error, where > was used instead of >=. With this > fix, the condition in buffered_put_buffer is really the opposite of > "rate limit reached", so write it explicitly like that. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > buffered_file.c | 7 ++----- > 1 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/buffered_file.c b/buffered_file.c > index edead5c..2dac99a 100644 > --- a/buffered_file.c > +++ b/buffered_file.c > @@ -125,7 +125,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in > > if (pos == 0 && size == 0) { > DPRINTF("file is ready\n"); > - if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) { > + if (!qemu_file_rate_limit(s->file)) { > DPRINTF("notifying client\n"); > migrate_fd_put_ready(s->migration_state); > } > @@ -190,10 +190,7 @@ static int buffered_rate_limit(void *opaque) > if (ret) { > return ret; > } > - if (s->freeze_output) > - return 1; > - > - if (s->bytes_xfer > s->xfer_limit) > + if (s->buffer_size >= s->xfer_limit) Please add braces. > return 1; > > return 0; > -- > 1.7.1 > > >
Il 24/11/2012 20:53, Blue Swirl ha scritto: >> > - if (s->bytes_xfer > s->xfer_limit) >> > + if (s->buffer_size >= s->xfer_limit) > Please add braces. Frankly I don't care much. This code is going to disappear at the beginning of 1.4. Paolo >> > return 1; >> >
diff --git a/buffered_file.c b/buffered_file.c index edead5c..2dac99a 100644 --- a/buffered_file.c +++ b/buffered_file.c @@ -125,7 +125,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in if (pos == 0 && size == 0) { DPRINTF("file is ready\n"); - if (!s->freeze_output && s->bytes_xfer < s->xfer_limit) { + if (!qemu_file_rate_limit(s->file)) { DPRINTF("notifying client\n"); migrate_fd_put_ready(s->migration_state); } @@ -190,10 +190,7 @@ static int buffered_rate_limit(void *opaque) if (ret) { return ret; } - if (s->freeze_output) - return 1; - - if (s->bytes_xfer > s->xfer_limit) + if (s->buffer_size >= s->xfer_limit) return 1; return 0;
buffered_rate_limit is called to prevent the RAM migration callback from putting too much data in the buffer. So it has to check against the amount of data currently in the buffer, not against the amount of data that has been transferred so far. s->bytes_xfer is used to communicate between successive calls of buffered_put_buffer. Buffered_rate_tick resets it every now and then to prevent moving too much buffered data to the socket at once. However, its value does not matter for the producer of the data. Here is the result for migrating an idle guest with 3GB of memory and ~360MB of non-zero memory: migrate_set_speed Before After ------------------------------------------------------------ default (32MB/sec) ~3 sec ~13 sec infinite (10GB/sec) ~3 sec ~3 sec Note that before this patch, QEMU is transferring of 100 MB/sec despite the rate limiting. Also fix an off-by-one error, where > was used instead of >=. With this fix, the condition in buffered_put_buffer is really the opposite of "rate limit reached", so write it explicitly like that. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- buffered_file.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-)