[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/7] migration: do floating-point division
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 7/7] migration: do floating-point division |
Date: |
Mon, 26 Jan 2015 18:06:47 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Paolo Bonzini (address@hidden) wrote:
> Dividing integer expressions transferred_bytes and time_spent, and then
> converting
> the integer quotient to type double. Any remainder, or fractional part of the
> quotient, is ignored. Fix this.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> migration/migration.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b3adbc6..6db75b8 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -646,7 +646,7 @@ static void *migration_thread(void *opaque)
> if (current_time >= initial_time + BUFFER_DELAY) {
> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
> uint64_t time_spent = current_time - initial_time;
> - double bandwidth = transferred_bytes / time_spent;
> + double bandwidth = (double)transferred_bytes / time_spent;
> max_size = bandwidth * migrate_max_downtime() / 1000000;
>
> s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
This feels like it would be better to fix this by merging it into
the s->mbps calculation just off the bottom; we currently have:
uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
uint64_t time_spent = current_time - initial_time;
double bandwidth = transferred_bytes / time_spent;
max_size = bandwidth * migrate_max_downtime() / 1000000;
s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
Note that the mbps has a check for time_spent being 0 - if that can ever happen,
how come 'bandwidth' has never triggered it?
transferred_bytes - bytes
time_spent - ms
bandwidth - bytes/ms
migrate_max_downtime - in ns
s->mbps - mbit/s
giving
max_size = bytes/ms * time-in-ns / 1000000
= bytes/ms * time-in-ms
= bytes
so how about something like:
uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
uint64_t time_spent = current_time - initial_time;
double bytes_s = (double) transferred_bytes / ((double)
time_spent / 1000.0));
s->mbps = (bytes_s * 8.0) / 1000000.0;
max_size = bytes_s * (migrate_max_downtime() / 1000000000.0);
that also needs the trace fixing and the line a few lines below, I *think* we
have
dirty_bytes_rate is in bytes/second ? (arch_init.c)
expected_downtime in ms ?
s->expected_downtime = s->dirty_bytes_rate / bandwidth;
so, bytes/s / bytes/ms erm that's supposed to come out as time in ms
s->expected_downtime = (int64_t)(1000 *
(double)s->dirty_bytes_rate / bytes_s);
Yeuch.
Dave
> --
> 1.8.3.1
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 2/7] cpu-exec: simplify icount code, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 3/7] uri: avoid NULL arguments to strcmp, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 4/7] qemu-sockets: improve error reporting in unix_listen_opts, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 1/7] cpu-exec: drop dead assignment, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 6/7] aes: remove a dead return statement, Paolo Bonzini, 2015/01/26
- [Qemu-devel] [PATCH 7/7] migration: do floating-point division, Paolo Bonzini, 2015/01/26
- Re: [Qemu-devel] [PATCH 7/7] migration: do floating-point division,
Dr. David Alan Gilbert <=