* [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 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).