On Thu, May 18, 2023 at 06:40:18PM +0200, Juan Quintela wrote: > Peter Xu <pet...@redhat.com> wrote: > > On Mon, May 08, 2023 at 03:09:09PM +0200, Juan Quintela wrote: > >> In the past, we had to put the in the main thread all the operations > >> related with sizes due to qemu_file not beeing thread safe. As now > >> all counters are atomic, we can update the counters just after the > >> do the write. As an aditional bonus, we are able to use the right > >> value for the compression methods. Right now we were assuming that > >> there were no compression at all. > > > > Maybe worth mention that initial packet is also accounted after this. > > Ok. > >> > >> + stat64_add(&mig_stats.multifd_bytes, p->next_packet_size); > >> + stat64_add(&mig_stats.transferred, p->next_packet_size); > > > > Two nits: > > > > Maybe merge the two so half atomic operations? > > On my tree, to send after this got in: > > 77fdd3475c migration: Remove transferred atomic counter > > O:-)
Ah this looks even better, indeed. :) What I meant was we can also do atomic update in one shot for both next_packet_size + packet_len. > > > Also maybe also worth having a inline helper for adding both multifd_bytes > > and transferred? > > I am removing it. > > After next set of packates: > > rate limit is calulated as: > > begining_period = migration_transferred_bytes(); > ... > > bytes_this_period = migration_transferred_bytes() - begining_period; > > transferred is calculated as: > - multifd_bytes + qemu_file_bytes; > > So things get really simple. As all counters are atomic, you do a > write and after the write to increse the write size to the qemu_file or > to the multifd_bytes. And that is it. Agreed. Thanks, -- Peter Xu
