Re: [PATCH v2 05/20] qemu_file: Use a stat64 for qemu_file_transferred

[Juan Quintela <quintela@redhat.com>]

Peter Xu <peterx@redhat.com> wrote:
> On Tue, May 30, 2023 at 08:39:26PM +0200, Juan Quintela wrote:
>> This way we can read it from any thread.
>> I checked that it gives the same value than the current one.  We never
>> use to qemu_files at the same time.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> The follow up patch may be better to be squashed or it's very confusing..

As said before, there used to be a nice:

if (old_counter != new_counter)
   printf(old_counter and new counter)

> Why do we need to convert mostly everything into atomics?  Is it modified
> outside migration thread?

Because we have four users of this stuff:

- io thread: needs it for info migrate basically
- migration_thread: needs it to know when to stop.  qemu_file is
  supposed to only happen here.
- ... until someone called peter decided to create another qemu file for
  preempt.  Current code don't account for this.
- multifd bytes is a completelly diffrent story, that is why it use its
  own atomics.

After this series go in, basically we only update one atomic for each
write (ok two yet, downtime/precopy/postocpy_bytes are still not
integrated).

But there used to be three:
- transferred
- multifd_bytes (for multifd pages)
- downtime/-precopy/postocpy

And we are back to only one (now we use transferred or multifd, never both).

> AFAIR atomic ops are still expensive, and will get more expensive on larger
> hosts,

If you care for performance, you are using multifd.
one atomic every 64 pages is kind ok.

For rate limiting, we do it 10 times a second, and:
a - it shouldn't matter with so few times
b - it is not a regression, we were already updating some
c - correct vs fast any day

And the other big improvement is that after this series, you can use any
counter on any thread.  No need to contorsions to update the rate_limit
that we used to have.

> IOW it'll be good for us to keep non-atomic when possible (and
> that's why when I changed some counters in postcopy work I only changed the
> limited set because then the rest are still accessed in 1 single thread and
> keep running fast).

void ram_transferred_add(uint64_t bytes)
{
    if (runstate_is_running()) {
        stat64_add(&mig_stats.precopy_bytes, bytes);
    } else if (migration_in_postcopy()) {
        stat64_add(&mig_stats.postcopy_bytes, bytes);
    } else {
        stat64_add(&mig_stats.downtime_bytes, bytes);
    }
    stat64_add(&mig_stats.transferred, bytes);
}

That is used everywhere, and I am dropping this to a single atomic.

Later, Juan.