* [PATCH 0/2] multifd: cleanup some source code @ 2021-12-22 11:30 Li Zhang 2021-12-22 11:30 ` [PATCH 1/2] multifd: cleanup the function multifd_channel_connect Li Zhang 2021-12-22 11:30 ` [PATCH 2/2] multifd: cleanup the function multifd_send_thread Li Zhang 0 siblings, 2 replies; 7+ messages in thread From: Li Zhang @ 2021-12-22 11:30 UTC (permalink / raw) To: quintela, dgilbert, cfontana, qemu-devel; +Cc: Li Zhang When looking into the multifd source code, some of them can be improved. This patchset is to cleanup the functions multifd_channel_connect and multfd_send_thead. The functions are tested without any errors. Li Zhang (2): multifd: cleanup the function multifd_channel_connect multifd: cleanup the function multifd_send_thread migration/multifd.c | 131 ++++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 66 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] multifd: cleanup the function multifd_channel_connect 2021-12-22 11:30 [PATCH 0/2] multifd: cleanup some source code Li Zhang @ 2021-12-22 11:30 ` Li Zhang 2022-01-06 10:07 ` Li Zhang 2023-02-08 17:59 ` Juan Quintela 2021-12-22 11:30 ` [PATCH 2/2] multifd: cleanup the function multifd_send_thread Li Zhang 1 sibling, 2 replies; 7+ messages in thread From: Li Zhang @ 2021-12-22 11:30 UTC (permalink / raw) To: quintela, dgilbert, cfontana, qemu-devel; +Cc: Li Zhang Cleanup multifd_channel_connect Signed-off-by: Li Zhang <lizhang@suse.de> --- migration/multifd.c | 49 ++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 212be1ed04..4ec40739e0 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -801,33 +801,32 @@ static bool multifd_channel_connect(MultiFDSendParams *p, trace_multifd_set_outgoing_channel( ioc, object_get_typename(OBJECT(ioc)), p->tls_hostname, error); - if (!error) { - if (s->parameters.tls_creds && - *s->parameters.tls_creds && - !object_dynamic_cast(OBJECT(ioc), - TYPE_QIO_CHANNEL_TLS)) { - multifd_tls_channel_connect(p, ioc, &error); - if (!error) { - /* - * tls_channel_connect will call back to this - * function after the TLS handshake, - * so we mustn't call multifd_send_thread until then - */ - return true; - } else { - return false; - } - } else { - migration_ioc_register_yank(ioc); - p->registered_yank = true; - p->c = ioc; - qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, - QEMU_THREAD_JOINABLE); - } - return true; + if (error) { + return false; } - return false; + if (s->parameters.tls_creds && + *s->parameters.tls_creds && + !object_dynamic_cast(OBJECT(ioc), + TYPE_QIO_CHANNEL_TLS)) { + multifd_tls_channel_connect(p, ioc, &error); + if (error) { + return false; + } + /* + * tls_channel_connect will call back to this + * function after the TLS handshake, + * so we mustn't call multifd_send_thread until then + */ + return true; + } else { + migration_ioc_register_yank(ioc); + p->registered_yank = true; + p->c = ioc; + qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, + QEMU_THREAD_JOINABLE); + } + return true; } static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] multifd: cleanup the function multifd_channel_connect 2021-12-22 11:30 ` [PATCH 1/2] multifd: cleanup the function multifd_channel_connect Li Zhang @ 2022-01-06 10:07 ` Li Zhang 2023-02-08 17:59 ` Juan Quintela 1 sibling, 0 replies; 7+ messages in thread From: Li Zhang @ 2022-01-06 10:07 UTC (permalink / raw) To: quintela, dgilbert, cfontana, qemu-devel ping On 12/22/21 12:30 PM, Li Zhang wrote: > Cleanup multifd_channel_connect > > Signed-off-by: Li Zhang <lizhang@suse.de> > --- > migration/multifd.c | 49 ++++++++++++++++++++++----------------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/migration/multifd.c b/migration/multifd.c > index 212be1ed04..4ec40739e0 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -801,33 +801,32 @@ static bool multifd_channel_connect(MultiFDSendParams *p, > trace_multifd_set_outgoing_channel( > ioc, object_get_typename(OBJECT(ioc)), p->tls_hostname, error); > > - if (!error) { > - if (s->parameters.tls_creds && > - *s->parameters.tls_creds && > - !object_dynamic_cast(OBJECT(ioc), > - TYPE_QIO_CHANNEL_TLS)) { > - multifd_tls_channel_connect(p, ioc, &error); > - if (!error) { > - /* > - * tls_channel_connect will call back to this > - * function after the TLS handshake, > - * so we mustn't call multifd_send_thread until then > - */ > - return true; > - } else { > - return false; > - } > - } else { > - migration_ioc_register_yank(ioc); > - p->registered_yank = true; > - p->c = ioc; > - qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, > - QEMU_THREAD_JOINABLE); > - } > - return true; > + if (error) { > + return false; > } > > - return false; > + if (s->parameters.tls_creds && > + *s->parameters.tls_creds && > + !object_dynamic_cast(OBJECT(ioc), > + TYPE_QIO_CHANNEL_TLS)) { > + multifd_tls_channel_connect(p, ioc, &error); > + if (error) { > + return false; > + } > + /* > + * tls_channel_connect will call back to this > + * function after the TLS handshake, > + * so we mustn't call multifd_send_thread until then > + */ > + return true; > + } else { > + migration_ioc_register_yank(ioc); > + p->registered_yank = true; > + p->c = ioc; > + qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, > + QEMU_THREAD_JOINABLE); > + } > + return true; > } > > static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] multifd: cleanup the function multifd_channel_connect 2021-12-22 11:30 ` [PATCH 1/2] multifd: cleanup the function multifd_channel_connect Li Zhang 2022-01-06 10:07 ` Li Zhang @ 2023-02-08 17:59 ` Juan Quintela 1 sibling, 0 replies; 7+ messages in thread From: Juan Quintela @ 2023-02-08 17:59 UTC (permalink / raw) To: Li Zhang; +Cc: dgilbert, cfontana, qemu-devel Li Zhang <lizhang@suse.de> wrote: > Cleanup multifd_channel_connect > > Signed-off-by: Li Zhang <lizhang@suse.de> Reviewed-by: Juan Quintela <quintela@redhat.com> queued. > --- > migration/multifd.c | 49 ++++++++++++++++++++++----------------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/migration/multifd.c b/migration/multifd.c > index 212be1ed04..4ec40739e0 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -801,33 +801,32 @@ static bool multifd_channel_connect(MultiFDSendParams *p, > trace_multifd_set_outgoing_channel( > ioc, object_get_typename(OBJECT(ioc)), p->tls_hostname, error); > > - if (!error) { > - if (s->parameters.tls_creds && > - *s->parameters.tls_creds && > - !object_dynamic_cast(OBJECT(ioc), > - TYPE_QIO_CHANNEL_TLS)) { > - multifd_tls_channel_connect(p, ioc, &error); > - if (!error) { > - /* > - * tls_channel_connect will call back to this > - * function after the TLS handshake, > - * so we mustn't call multifd_send_thread until then > - */ > - return true; > - } else { > - return false; > - } > - } else { > - migration_ioc_register_yank(ioc); > - p->registered_yank = true; > - p->c = ioc; > - qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, > - QEMU_THREAD_JOINABLE); > - } > - return true; > + if (error) { > + return false; > } > > - return false; > + if (s->parameters.tls_creds && > + *s->parameters.tls_creds && > + !object_dynamic_cast(OBJECT(ioc), > + TYPE_QIO_CHANNEL_TLS)) { > + multifd_tls_channel_connect(p, ioc, &error); > + if (error) { > + return false; > + } > + /* > + * tls_channel_connect will call back to this > + * function after the TLS handshake, > + * so we mustn't call multifd_send_thread until then > + */ > + return true; > + } else { > + migration_ioc_register_yank(ioc); > + p->registered_yank = true; > + p->c = ioc; > + qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, > + QEMU_THREAD_JOINABLE); > + } > + return true; > } > > static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] multifd: cleanup the function multifd_send_thread 2021-12-22 11:30 [PATCH 0/2] multifd: cleanup some source code Li Zhang 2021-12-22 11:30 ` [PATCH 1/2] multifd: cleanup the function multifd_channel_connect Li Zhang @ 2021-12-22 11:30 ` Li Zhang 2022-01-06 10:07 ` Li Zhang 2023-02-08 18:11 ` Juan Quintela 1 sibling, 2 replies; 7+ messages in thread From: Li Zhang @ 2021-12-22 11:30 UTC (permalink / raw) To: quintela, dgilbert, cfontana, qemu-devel; +Cc: Li Zhang Cleanup multifd_send_thread Signed-off-by: Li Zhang <lizhang@suse.de> --- migration/multifd.c | 82 ++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 4ec40739e0..7888d71bfe 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -649,58 +649,58 @@ static void *multifd_send_thread(void *opaque) break; } qemu_mutex_lock(&p->mutex); - - if (p->pending_job) { - uint32_t used = p->pages->num; - uint64_t packet_num = p->packet_num; - uint32_t flags = p->flags; - - if (used) { - ret = multifd_send_state->ops->send_prepare(p, &local_err); - if (ret != 0) { - qemu_mutex_unlock(&p->mutex); - break; - } - } - multifd_send_fill_packet(p); - p->flags = 0; - p->num_packets++; - p->num_pages += used; - p->pages->num = 0; - p->pages->block = NULL; + if (!p->quit && !p->pending_job) { + /* sometimes there are spurious wakeups */ + qemu_mutex_unlock(&p->mutex); + continue; + } else if (!p->pending_job) { qemu_mutex_unlock(&p->mutex); + break; + } - trace_multifd_send(p->id, packet_num, used, flags, - p->next_packet_size); + uint32_t used = p->pages->num; + uint64_t packet_num = p->packet_num; + uint32_t flags = p->flags; - ret = qio_channel_write_all(p->c, (void *)p->packet, - p->packet_len, &local_err); + if (used) { + ret = multifd_send_state->ops->send_prepare(p, &local_err); if (ret != 0) { + qemu_mutex_unlock(&p->mutex); break; } + } + multifd_send_fill_packet(p); + p->flags = 0; + p->num_packets++; + p->num_pages += used; + p->pages->num = 0; + p->pages->block = NULL; + qemu_mutex_unlock(&p->mutex); - if (used) { - ret = multifd_send_state->ops->send_write(p, used, &local_err); - if (ret != 0) { - break; - } - } + trace_multifd_send(p->id, packet_num, used, flags, + p->next_packet_size); - qemu_mutex_lock(&p->mutex); - p->pending_job--; - qemu_mutex_unlock(&p->mutex); + ret = qio_channel_write_all(p->c, (void *)p->packet, + p->packet_len, &local_err); + if (ret != 0) { + break; + } - if (flags & MULTIFD_FLAG_SYNC) { - qemu_sem_post(&p->sem_sync); + if (used) { + ret = multifd_send_state->ops->send_write(p, used, &local_err); + if (ret != 0) { + break; } - qemu_sem_post(&multifd_send_state->channels_ready); - } else if (p->quit) { - qemu_mutex_unlock(&p->mutex); - break; - } else { - qemu_mutex_unlock(&p->mutex); - /* sometimes there are spurious wakeups */ } + + qemu_mutex_lock(&p->mutex); + p->pending_job--; + qemu_mutex_unlock(&p->mutex); + + if (flags & MULTIFD_FLAG_SYNC) { + qemu_sem_post(&p->sem_sync); + } + qemu_sem_post(&multifd_send_state->channels_ready); } out: -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] multifd: cleanup the function multifd_send_thread 2021-12-22 11:30 ` [PATCH 2/2] multifd: cleanup the function multifd_send_thread Li Zhang @ 2022-01-06 10:07 ` Li Zhang 2023-02-08 18:11 ` Juan Quintela 1 sibling, 0 replies; 7+ messages in thread From: Li Zhang @ 2022-01-06 10:07 UTC (permalink / raw) To: qemu-devel ping On 12/22/21 12:30 PM, Li Zhang wrote: > Cleanup multifd_send_thread > > Signed-off-by: Li Zhang <lizhang@suse.de> > --- > migration/multifd.c | 82 ++++++++++++++++++++++----------------------- > 1 file changed, 41 insertions(+), 41 deletions(-) > > diff --git a/migration/multifd.c b/migration/multifd.c > index 4ec40739e0..7888d71bfe 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -649,58 +649,58 @@ static void *multifd_send_thread(void *opaque) > break; > } > qemu_mutex_lock(&p->mutex); > - > - if (p->pending_job) { > - uint32_t used = p->pages->num; > - uint64_t packet_num = p->packet_num; > - uint32_t flags = p->flags; > - > - if (used) { > - ret = multifd_send_state->ops->send_prepare(p, &local_err); > - if (ret != 0) { > - qemu_mutex_unlock(&p->mutex); > - break; > - } > - } > - multifd_send_fill_packet(p); > - p->flags = 0; > - p->num_packets++; > - p->num_pages += used; > - p->pages->num = 0; > - p->pages->block = NULL; > + if (!p->quit && !p->pending_job) { > + /* sometimes there are spurious wakeups */ > + qemu_mutex_unlock(&p->mutex); > + continue; > + } else if (!p->pending_job) { > qemu_mutex_unlock(&p->mutex); > + break; > + } > > - trace_multifd_send(p->id, packet_num, used, flags, > - p->next_packet_size); > + uint32_t used = p->pages->num; > + uint64_t packet_num = p->packet_num; > + uint32_t flags = p->flags; > > - ret = qio_channel_write_all(p->c, (void *)p->packet, > - p->packet_len, &local_err); > + if (used) { > + ret = multifd_send_state->ops->send_prepare(p, &local_err); > if (ret != 0) { > + qemu_mutex_unlock(&p->mutex); > break; > } > + } > + multifd_send_fill_packet(p); > + p->flags = 0; > + p->num_packets++; > + p->num_pages += used; > + p->pages->num = 0; > + p->pages->block = NULL; > + qemu_mutex_unlock(&p->mutex); > > - if (used) { > - ret = multifd_send_state->ops->send_write(p, used, &local_err); > - if (ret != 0) { > - break; > - } > - } > + trace_multifd_send(p->id, packet_num, used, flags, > + p->next_packet_size); > > - qemu_mutex_lock(&p->mutex); > - p->pending_job--; > - qemu_mutex_unlock(&p->mutex); > + ret = qio_channel_write_all(p->c, (void *)p->packet, > + p->packet_len, &local_err); > + if (ret != 0) { > + break; > + } > > - if (flags & MULTIFD_FLAG_SYNC) { > - qemu_sem_post(&p->sem_sync); > + if (used) { > + ret = multifd_send_state->ops->send_write(p, used, &local_err); > + if (ret != 0) { > + break; > } > - qemu_sem_post(&multifd_send_state->channels_ready); > - } else if (p->quit) { > - qemu_mutex_unlock(&p->mutex); > - break; > - } else { > - qemu_mutex_unlock(&p->mutex); > - /* sometimes there are spurious wakeups */ > } > + > + qemu_mutex_lock(&p->mutex); > + p->pending_job--; > + qemu_mutex_unlock(&p->mutex); > + > + if (flags & MULTIFD_FLAG_SYNC) { > + qemu_sem_post(&p->sem_sync); > + } > + qemu_sem_post(&multifd_send_state->channels_ready); > } > > out: > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] multifd: cleanup the function multifd_send_thread 2021-12-22 11:30 ` [PATCH 2/2] multifd: cleanup the function multifd_send_thread Li Zhang 2022-01-06 10:07 ` Li Zhang @ 2023-02-08 18:11 ` Juan Quintela 1 sibling, 0 replies; 7+ messages in thread From: Juan Quintela @ 2023-02-08 18:11 UTC (permalink / raw) To: Li Zhang; +Cc: dgilbert, cfontana, qemu-devel Li Zhang <lizhang@suse.de> wrote: > Cleanup multifd_send_thread > > Signed-off-by: Li Zhang <lizhang@suse.de> Hi Zhang First of all, sorry for the late review. This other patch is wrong. > --- > migration/multifd.c | 82 ++++++++++++++++++++++----------------------- > 1 file changed, 41 insertions(+), 41 deletions(-) > > diff --git a/migration/multifd.c b/migration/multifd.c > index 4ec40739e0..7888d71bfe 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -649,58 +649,58 @@ static void *multifd_send_thread(void *opaque) > break; > } > qemu_mutex_lock(&p->mutex); > - > - if (p->pending_job) { > - uint32_t used = p->pages->num; > - uint64_t packet_num = p->packet_num; > - uint32_t flags = p->flags; > - > - if (used) { > - ret = multifd_send_state->ops->send_prepare(p, &local_err); > - if (ret != 0) { > - qemu_mutex_unlock(&p->mutex); > - break; > - } > - } > - multifd_send_fill_packet(p); > - p->flags = 0; > - p->num_packets++; > - p->num_pages += used; > - p->pages->num = 0; > - p->pages->block = NULL; > + if (!p->quit && !p->pending_job) { > + /* sometimes there are spurious wakeups */ > + qemu_mutex_unlock(&p->mutex); > + continue; > + } else if (!p->pending_job) { Here it should be } else if (p->quit) { And in this case, I preffer the previous code, as the first case is the common one. I still have to see if we ever enter through the spurious case anymore. Thanks, Juan. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-08 18:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-22 11:30 [PATCH 0/2] multifd: cleanup some source code Li Zhang 2021-12-22 11:30 ` [PATCH 1/2] multifd: cleanup the function multifd_channel_connect Li Zhang 2022-01-06 10:07 ` Li Zhang 2023-02-08 17:59 ` Juan Quintela 2021-12-22 11:30 ` [PATCH 2/2] multifd: cleanup the function multifd_send_thread Li Zhang 2022-01-06 10:07 ` Li Zhang 2023-02-08 18:11 ` Juan Quintela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).