If p->quit is true for any channel, we know that it has finished for
any reason.  So don't wait for it, just continue.

Signed-off-by: Juan Quintela <quintela@redhat.com>

---

I could be convinced that the right thing to do in that case is to
just do a break instead of a continue.  Each option has its own
advantages/disadvantanges.
---
 migration/ram.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 44ca56e1ea..bc918ef28d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1118,6 +1118,12 @@ static void multifd_send_sync_main(RAMState *rs)
         MultiFDSendParams *p = &multifd_send_state->params[i];
 
         trace_multifd_send_sync_main_wait(p->id);
+        qemu_mutex_lock(&p->mutex);
+        if (p->quit) {
+            qemu_mutex_unlock(&p->mutex);
+            continue;
+        }
+        qemu_mutex_unlock(&p->mutex);
         qemu_sem_wait(&p->sem_sync);
     }
     trace_multifd_send_sync_main(multifd_send_state->packet_num);
-- 
2.24.1

* Juan Quintela (quintela@redhat.com) wrote:
> If p->quit is true for any channel, we know that it has finished for
> any reason.  So don't wait for it, just continue.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> ---
> 
> I could be convinced that the right thing to do in that case is to
> just do a break instead of a continue.  Each option has its own
> advantages/disadvantanges.
> ---
>  migration/ram.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 44ca56e1ea..bc918ef28d 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1118,6 +1118,12 @@ static void multifd_send_sync_main(RAMState *rs)
>          MultiFDSendParams *p = &multifd_send_state->params[i];
>  
>          trace_multifd_send_sync_main_wait(p->id);
> +        qemu_mutex_lock(&p->mutex);
> +        if (p->quit) {
> +            qemu_mutex_unlock(&p->mutex);
> +            continue;
> +        }
> +        qemu_mutex_unlock(&p->mutex);

Why is this needed/helps?
You can't depend on the p->quit happening before the 
sem_wait, so the main thread still has to do a post on sem_sync before
the join, even with the addition of the check for p->quit.

Dave

>          qemu_sem_wait(&p->sem_sync);
>      }
>      trace_multifd_send_sync_main(multifd_send_state->packet_num);
> -- 
> 2.24.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> If p->quit is true for any channel, we know that it has finished for
>> any reason.  So don't wait for it, just continue.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> 
>> ---
>> 
>> I could be convinced that the right thing to do in that case is to
>> just do a break instead of a continue.  Each option has its own
>> advantages/disadvantanges.
>> ---
>>  migration/ram.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 44ca56e1ea..bc918ef28d 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1118,6 +1118,12 @@ static void multifd_send_sync_main(RAMState *rs)
>>          MultiFDSendParams *p = &multifd_send_state->params[i];
>>  
>>          trace_multifd_send_sync_main_wait(p->id);
>> +        qemu_mutex_lock(&p->mutex);
>> +        if (p->quit) {
>> +            qemu_mutex_unlock(&p->mutex);
>> +            continue;
>> +        }
>> +        qemu_mutex_unlock(&p->mutex);
>
> Why is this needed/helps?
> You can't depend on the p->quit happening before the 
> sem_wait, so the main thread still has to do a post on sem_sync before
> the join, even with the addition of the check for p->quit.

if we have asked the thread to quit, it is inside posibility that it has
already quit, so it is not going to be able to do the ->post() for this
sem.

if ->quit == true, then we know that we are exiting.  On _normal_ exit,
we know that everything is ok.  On cancel/error, we don't really know,
it deppends how lucky we are.

Later, Juan.