Hi,Juan, On 2023/1/30 下午4:09, Juan Quintela wrote: > Current code asumes that all pages are whole. That is not true for > example for compression already. Fix it for creating a new field > ->sent_bytes that includes it. > > All ram_counters are used only from the migration thread, so we have > two options: > - put a mutex and fill everything when we sent it (not only > ram_counters, also qemu_file->xfer_bytes). > - Create a local variable that implements how much has been sent > through each channel. And when we push another packet, we "add" the > previous stats. > > I choose two due to less changes overall. On the previous code we > increase transferred and then we sent. Current code goes the other > way around. It sents the data, and after the fact, it updates the > counters. Notice that each channel can have a maximum of half a > megabyte of data without counting, so it is not very important. > > Signed-off-by: Juan Quintela <quint...@redhat.com> > --- > migration/multifd.h | 2 ++ > migration/multifd.c | 6 ++++-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/migration/multifd.h b/migration/multifd.h > index e2802a9ce2..36f899c56f 100644 > --- a/migration/multifd.h > +++ b/migration/multifd.h > @@ -102,6 +102,8 @@ typedef struct { > uint32_t flags; > /* global number of generated multifd packets */ > uint64_t packet_num; > + /* How many bytes have we sent on the last packet */ > + uint64_t sent_bytes; > /* thread has work to do */ > int pending_job; > /* array of pages to sent. > diff --git a/migration/multifd.c b/migration/multifd.c > index 61cafe4c76..cd26b2fda9 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -394,7 +394,6 @@ static int multifd_send_pages(QEMUFile *f) > static int next_channel; > MultiFDSendParams *p = NULL; /* make happy gcc */ > MultiFDPages_t *pages = multifd_send_state->pages; > - uint64_t transferred; > > if (qatomic_read(&multifd_send_state->exiting)) { > return -1; > @@ -429,7 +428,8 @@ static int multifd_send_pages(QEMUFile *f) > p->packet_num = multifd_send_state->packet_num++; > multifd_send_state->pages = p->pages; > p->pages = pages; > - transferred = ((uint64_t) pages->num) * p->page_size + p->packet_len; > + uint64_t transferred = p->sent_bytes; > + p->sent_bytes = 0; > qemu_file_acct_rate_limit(f, transferred); > qemu_mutex_unlock(&p->mutex); > stat64_add(&ram_atomic_counters.multifd_bytes, transferred); > @@ -719,6 +719,8 @@ static void *multifd_send_thread(void *opaque) > } > > qemu_mutex_lock(&p->mutex); > + p->sent_bytes += p->packet_len; > + p->sent_bytes += p->next_packet_size;
Consider a scenario where some normal pages are transmitted in the first round, followed by several consecutive rounds of zero pages. When zero pages are transmitted, next_packet_size of first round is still incorrectly added to sent_bytes. If we set a rate limiting for dirty page transmission, the transmission performance of multi zero check will degrade. Maybe we should set next_packet_size to 0 in multifd_send_pages()? > p->pending_job--; > qemu_mutex_unlock(&p->mutex); >