diff mbox series

multifd: Fix the number of channels ready

Message ID 20230426162307.11060-1-quintela@redhat.com
State New
Headers show
Series multifd: Fix the number of channels ready | expand

Commit Message

Juan Quintela April 26, 2023, 4:23 p.m. UTC
We don't wait in the sem when we are doing a sync_main.  Make it wait
there.  To make things clearer, we mark the channel ready at the
begining of the thread loop.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Fabiano Rosas April 26, 2023, 5:22 p.m. UTC | #1
Juan Quintela <quintela@redhat.com> writes:

> We don't wait in the sem when we are doing a sync_main.  Make it wait
> there.  To make things clearer, we mark the channel ready at the
> begining of the thread loop.

So in other words we're estabilishing that "channel ready" means ready
to send, regardless of having sent the sync packet. Is that it?
Juan Quintela April 26, 2023, 5:35 p.m. UTC | #2
Fabiano Rosas <farosas@suse.de> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> We don't wait in the sem when we are doing a sync_main.  Make it wait
>> there.  To make things clearer, we mark the channel ready at the
>> begining of the thread loop.
>
> So in other words we're estabilishing that "channel ready" means ready
> to send, regardless of having sent the sync packet. Is that it?

Yeap.

There was a bug (from the beggining) that made the counter always get
up and up.  This fixes it.

It was always supposed to work this way.

/me puts (second time in the week) a brown paper bag on head

Later, Juan.
Fabiano Rosas April 26, 2023, 5:58 p.m. UTC | #3
Juan Quintela <quintela@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> We don't wait in the sem when we are doing a sync_main.  Make it wait
>>> there.  To make things clearer, we mark the channel ready at the
>>> begining of the thread loop.
>>
>> So in other words we're estabilishing that "channel ready" means ready
>> to send, regardless of having sent the sync packet. Is that it?
>
> Yeap.
>
> There was a bug (from the beggining) that made the counter always get
> up and up.  This fixes it.
>
> It was always supposed to work this way.

Ah, great. I'm proposing a multifd variant without the sync packet in my
fixed-ram series and moving the channels_ready to the top of the loop
means I can stop issuing an extra qemu_sem_post(&p->sem) just to skip
the sync packet.
Fabiano Rosas April 26, 2023, 5:58 p.m. UTC | #4
Juan Quintela <quintela@redhat.com> writes:

> We don't wait in the sem when we are doing a sync_main.  Make it wait
> there.  To make things clearer, we mark the channel ready at the
> begining of the thread loop.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>
diff mbox series

Patch

diff --git a/migration/multifd.c b/migration/multifd.c
index 903df2117b..e625e8725e 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -635,6 +635,7 @@  int multifd_send_sync_main(QEMUFile *f)
     for (i = 0; i < migrate_multifd_channels(); i++) {
         MultiFDSendParams *p = &multifd_send_state->params[i];
 
+        qemu_sem_wait(&multifd_send_state->channels_ready);
         trace_multifd_send_sync_main_wait(p->id);
         qemu_sem_wait(&p->sem_sync);
 
@@ -668,6 +669,7 @@  static void *multifd_send_thread(void *opaque)
     p->num_packets = 1;
 
     while (true) {
+        qemu_sem_post(&multifd_send_state->channels_ready);
         qemu_sem_wait(&p->sem);
 
         if (qatomic_read(&multifd_send_state->exiting)) {
@@ -736,7 +738,6 @@  static void *multifd_send_thread(void *opaque)
             if (flags & MULTIFD_FLAG_SYNC) {
                 qemu_sem_post(&p->sem_sync);
             }
-            qemu_sem_post(&multifd_send_state->channels_ready);
         } else if (p->quit) {
             qemu_mutex_unlock(&p->mutex);
             break;