* [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync
@ 2019-10-26 0:45 Wei Yang
2019-10-26 0:45 ` [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet() Wei Yang
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
Current send thread could work while the sync mechanism has some problem:
* has spuriously wakeup
* number of channels_ready will *overflow* the number of real channels
The reason is:
* if MULTIFD_FLAG_SYNC is set in the middle of send thread running, there
is one more spurious wakeup
* if MULTIFD_FLAG_SYNC is set when send thread is not running, there is one
more channels_ready be triggered
To solve this situation, one new mechanism is introduced to synchronize send
threads. The idea is simple, a new field *sync* is introduced to indicate a
synchronization is required.
---
v2: rebase on latest code
Wei Yang (6):
migration/multifd: move Params update and pages cleanup into
multifd_send_fill_packet()
migration/multifd: notify channels_ready when send thread starts
migration/multifd: use sync field to synchronize send threads
migration/multifd: used must not be 0 for a pending job
migration/multifd: use boolean for pending_job is enough
migration/multifd: there is no spurious wakeup now
migration/ram.c | 74 +++++++++++++++++++++++++++++++------------------
1 file changed, 47 insertions(+), 27 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet()
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
@ 2019-10-26 0:45 ` Wei Yang
2019-11-19 10:57 ` Juan Quintela
2019-10-26 0:45 ` [PATCH v2 2/6] migration/multifd: notify channels_ready when send thread starts Wei Yang
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
Fill data and update/cleanup related field in one place. Also make the
code a little clean.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 5876054195..35f147388b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -789,15 +789,16 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
g_free(pages);
}
-static void multifd_send_fill_packet(MultiFDSendParams *p)
+static void multifd_send_fill_packet(MultiFDSendParams *p, uint32_t used)
{
MultiFDPacket_t *packet = p->packet;
+ uint32_t next_packet_size = used * qemu_target_page_size();
int i;
packet->flags = cpu_to_be32(p->flags);
packet->pages_alloc = cpu_to_be32(p->pages->allocated);
packet->pages_used = cpu_to_be32(p->pages->used);
- packet->next_packet_size = cpu_to_be32(p->next_packet_size);
+ packet->next_packet_size = cpu_to_be32(next_packet_size);
packet->packet_num = cpu_to_be64(p->packet_num);
if (p->pages->block) {
@@ -807,6 +808,13 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
for (i = 0; i < p->pages->used; i++) {
packet->offset[i] = cpu_to_be64(p->pages->offset[i]);
}
+
+ p->next_packet_size = next_packet_size;
+ p->flags = 0;
+ p->num_packets++;
+ p->num_pages += used;
+ p->pages->used = 0;
+ p->pages->block = NULL;
}
static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
@@ -1109,13 +1117,7 @@ static void *multifd_send_thread(void *opaque)
uint64_t packet_num = p->packet_num;
flags = p->flags;
- p->next_packet_size = used * qemu_target_page_size();
- multifd_send_fill_packet(p);
- p->flags = 0;
- p->num_packets++;
- p->num_pages += used;
- p->pages->used = 0;
- p->pages->block = NULL;
+ multifd_send_fill_packet(p, used);
qemu_mutex_unlock(&p->mutex);
trace_multifd_send(p->id, packet_num, used, flags,
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/6] migration/multifd: notify channels_ready when send thread starts
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
2019-10-26 0:45 ` [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet() Wei Yang
@ 2019-10-26 0:45 ` Wei Yang
2019-10-26 0:45 ` [PATCH v2 3/6] migration/multifd: use sync field to synchronize send threads Wei Yang
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
multifd_send_state->channels_ready is initialized to 0. It is proper to
let main thread know we are ready when thread start running.
Current implementation works since ram_save_setup() calls
multifd_send_sync_main() which wake up send thread and posts
channels_ready. This behavior will introduce some unpredictable
situation and disturb the semaphore value.
This is a preparation patch to use another mechanism to do send thread
synchronization to avoid post channels_ready in this case. So this patch
posts channels_ready when send threads start running.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 35f147388b..25d477796e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1107,6 +1107,8 @@ static void *multifd_send_thread(void *opaque)
}
/* initial packet */
p->num_packets = 1;
+ /* let main thread know we are ready */
+ qemu_sem_post(&multifd_send_state->channels_ready);
while (true) {
qemu_sem_wait(&p->sem);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/6] migration/multifd: use sync field to synchronize send threads
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
2019-10-26 0:45 ` [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet() Wei Yang
2019-10-26 0:45 ` [PATCH v2 2/6] migration/multifd: notify channels_ready when send thread starts Wei Yang
@ 2019-10-26 0:45 ` Wei Yang
2019-10-26 0:45 ` [PATCH v2 4/6] migration/multifd: used must not be 0 for a pending job Wei Yang
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
Add a field in MultiFDSendParams to indicate there is a request to
synchronize send threads.
By doing so, send_thread will just post sem_sync on synchronization
request and channels_ready will not *overflow*.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 25d477796e..62072b7a35 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -641,6 +641,8 @@ typedef struct {
QemuMutex mutex;
/* is this channel thread running */
bool running;
+ /* should sync this channel */
+ bool sync;
/* should this thread finish */
bool quit;
/* thread has work to do */
@@ -1074,8 +1076,7 @@ static void multifd_send_sync_main(RAMState *rs)
}
p->packet_num = multifd_send_state->packet_num++;
- p->flags |= MULTIFD_FLAG_SYNC;
- p->pending_job++;
+ p->sync = true;
qemu_file_update_transfer(rs->f, p->packet_len);
ram_counters.multifd_bytes += p->packet_len;
ram_counters.transferred += p->packet_len;
@@ -1143,10 +1144,27 @@ static void *multifd_send_thread(void *opaque)
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);
+ } else if (p->sync) {
+ uint64_t packet_num = p->packet_num;
+ uint32_t flags = p->flags;
+ assert(!p->pages->used);
+
+ p->flags |= MULTIFD_FLAG_SYNC;
+ multifd_send_fill_packet(p, 0);
+ p->sync = false;
+ qemu_mutex_unlock(&p->mutex);
+
+ trace_multifd_send(p->id, packet_num, 0, flags | MULTIFD_FLAG_SYNC,
+ p->next_packet_size);
+
+ ret = qio_channel_write_all(p->c, (void *)p->packet,
+ p->packet_len, &local_err);
+ if (ret != 0) {
+ break;
+ }
+
+ qemu_sem_post(&p->sem_sync);
} else if (p->quit) {
qemu_mutex_unlock(&p->mutex);
break;
@@ -1221,7 +1239,7 @@ int multifd_save_setup(void)
qemu_mutex_init(&p->mutex);
qemu_sem_init(&p->sem, 0);
qemu_sem_init(&p->sem_sync, 0);
- p->quit = false;
+ p->quit = p->sync = false;
p->pending_job = 0;
p->id = i;
p->pages = multifd_pages_init(page_count);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/6] migration/multifd: used must not be 0 for a pending job
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
` (2 preceding siblings ...)
2019-10-26 0:45 ` [PATCH v2 3/6] migration/multifd: use sync field to synchronize send threads Wei Yang
@ 2019-10-26 0:45 ` Wei Yang
2019-10-26 0:45 ` [PATCH v2 5/6] migration/multifd: use boolean for pending_job is enough Wei Yang
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
After thread synchronization request is handled in another case, this
means when we only get pending_job when there is used pages.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 62072b7a35..12c270e86d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1132,12 +1132,11 @@ static void *multifd_send_thread(void *opaque)
break;
}
- if (used) {
- ret = qio_channel_writev_all(p->c, p->pages->iov,
- used, &local_err);
- if (ret != 0) {
- break;
- }
+ assert(used);
+ ret = qio_channel_writev_all(p->c, p->pages->iov,
+ used, &local_err);
+ if (ret != 0) {
+ break;
}
qemu_mutex_lock(&p->mutex);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/6] migration/multifd: use boolean for pending_job is enough
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
` (3 preceding siblings ...)
2019-10-26 0:45 ` [PATCH v2 4/6] migration/multifd: used must not be 0 for a pending job Wei Yang
@ 2019-10-26 0:45 ` Wei Yang
2019-10-26 0:45 ` [PATCH v2 6/6] migration/multifd: there is no spurious wakeup now Wei Yang
2019-12-16 2:36 ` [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
6 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
After synchronization request is handled in another case, there only
could be one pending_job for one send thread at most.
This is fine to use boolean to represent this behavior.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 12c270e86d..fccdbfabc5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -646,7 +646,7 @@ typedef struct {
/* should this thread finish */
bool quit;
/* thread has work to do */
- int pending_job;
+ bool pending_job;
/* array of pages to sent */
MultiFDPages_t *pages;
/* packet allocated len */
@@ -933,7 +933,7 @@ static int multifd_send_pages(RAMState *rs)
return -1;
}
if (!p->pending_job) {
- p->pending_job++;
+ p->pending_job = true;
next_channel = (i + 1) % migrate_multifd_channels();
break;
}
@@ -1140,7 +1140,7 @@ static void *multifd_send_thread(void *opaque)
}
qemu_mutex_lock(&p->mutex);
- p->pending_job--;
+ p->pending_job = false;
qemu_mutex_unlock(&p->mutex);
qemu_sem_post(&multifd_send_state->channels_ready);
@@ -1238,8 +1238,7 @@ int multifd_save_setup(void)
qemu_mutex_init(&p->mutex);
qemu_sem_init(&p->sem, 0);
qemu_sem_init(&p->sem_sync, 0);
- p->quit = p->sync = false;
- p->pending_job = 0;
+ p->quit = p->sync = p->pending_job = false;
p->id = i;
p->pages = multifd_pages_init(page_count);
p->packet_len = sizeof(MultiFDPacket_t)
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 6/6] migration/multifd: there is no spurious wakeup now
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
` (4 preceding siblings ...)
2019-10-26 0:45 ` [PATCH v2 5/6] migration/multifd: use boolean for pending_job is enough Wei Yang
@ 2019-10-26 0:45 ` Wei Yang
2019-12-16 2:36 ` [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
6 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-10-26 0:45 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
The spurious wakeup is gone.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index fccdbfabc5..73ace40b1b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1168,8 +1168,8 @@ static void *multifd_send_thread(void *opaque)
qemu_mutex_unlock(&p->mutex);
break;
} else {
- qemu_mutex_unlock(&p->mutex);
- /* sometimes there are spurious wakeups */
+ /* no other case should trigger me */
+ g_assert_not_reached();
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet()
2019-10-26 0:45 ` [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet() Wei Yang
@ 2019-11-19 10:57 ` Juan Quintela
2019-11-29 8:30 ` Wei Yang
0 siblings, 1 reply; 10+ messages in thread
From: Juan Quintela @ 2019-11-19 10:57 UTC (permalink / raw)
To: Wei Yang; +Cc: dgilbert, qemu-devel
Wei Yang <richardw.yang@linux.intel.com> wrote:
> Fill data and update/cleanup related field in one place. Also make the
> code a little clean.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
right cleanup.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet()
2019-11-19 10:57 ` Juan Quintela
@ 2019-11-29 8:30 ` Wei Yang
0 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-11-29 8:30 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel, Wei Yang, dgilbert
On Tue, Nov 19, 2019 at 11:57:22AM +0100, Juan Quintela wrote:
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>> Fill data and update/cleanup related field in one place. Also make the
>> code a little clean.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Reviewed-by: Juan Quintela <quintela@redhat.com>
>
>right cleanup.
>
Hi, Juan
Do you have other comments on the following patches?
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
` (5 preceding siblings ...)
2019-10-26 0:45 ` [PATCH v2 6/6] migration/multifd: there is no spurious wakeup now Wei Yang
@ 2019-12-16 2:36 ` Wei Yang
6 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2019-12-16 2:36 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert, quintela
Ping for comment.
On Sat, Oct 26, 2019 at 08:45:14AM +0800, Wei Yang wrote:
>Current send thread could work while the sync mechanism has some problem:
>
> * has spuriously wakeup
> * number of channels_ready will *overflow* the number of real channels
>
>The reason is:
>
> * if MULTIFD_FLAG_SYNC is set in the middle of send thread running, there
> is one more spurious wakeup
> * if MULTIFD_FLAG_SYNC is set when send thread is not running, there is one
> more channels_ready be triggered
>
>To solve this situation, one new mechanism is introduced to synchronize send
>threads. The idea is simple, a new field *sync* is introduced to indicate a
>synchronization is required.
>
>---
>v2: rebase on latest code
>
>Wei Yang (6):
> migration/multifd: move Params update and pages cleanup into
> multifd_send_fill_packet()
> migration/multifd: notify channels_ready when send thread starts
> migration/multifd: use sync field to synchronize send threads
> migration/multifd: used must not be 0 for a pending job
> migration/multifd: use boolean for pending_job is enough
> migration/multifd: there is no spurious wakeup now
>
> migration/ram.c | 74 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 47 insertions(+), 27 deletions(-)
>
>--
>2.17.1
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-16 2:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-26 0:45 [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
2019-10-26 0:45 ` [PATCH v2 1/6] migration/multifd: move Params update and pages cleanup into multifd_send_fill_packet() Wei Yang
2019-11-19 10:57 ` Juan Quintela
2019-11-29 8:30 ` Wei Yang
2019-10-26 0:45 ` [PATCH v2 2/6] migration/multifd: notify channels_ready when send thread starts Wei Yang
2019-10-26 0:45 ` [PATCH v2 3/6] migration/multifd: use sync field to synchronize send threads Wei Yang
2019-10-26 0:45 ` [PATCH v2 4/6] migration/multifd: used must not be 0 for a pending job Wei Yang
2019-10-26 0:45 ` [PATCH v2 5/6] migration/multifd: use boolean for pending_job is enough Wei Yang
2019-10-26 0:45 ` [PATCH v2 6/6] migration/multifd: there is no spurious wakeup now Wei Yang
2019-12-16 2:36 ` [PATCH v2 0/6] migration/multifd: a new mechanism for send thread sync Wei Yang
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.