* [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
* [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 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 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 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
* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.