"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> We now send several pages at a time each time that we wakeup a thread.
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>>
>> --
>>
>> Use iovec's insead of creating the equivalent.
>> ---
>> migration/ram.c | 46 ++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 40 insertions(+), 6 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 2bf3fa7..90e1bcb 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -362,6 +362,13 @@ static void compress_threads_save_setup(void)
>>
>> /* Multiple fd's */
>>
>> +
>> +typedef struct {
>> + int num;
>> + int size;
>
> size_t ?
Done.
>
>> + struct iovec *iov;
>> +} multifd_pages_t;
>> +
>> struct MultiFDSendParams {
>> /* not changed */
>> uint8_t id;
>> @@ -371,7 +378,7 @@ struct MultiFDSendParams {
>> QemuMutex mutex;
>> /* protected by param mutex */
>> bool quit;
>> - uint8_t *address;
>> + multifd_pages_t pages;
>> /* protected by multifd mutex */
>> bool done;
>> };
>> @@ -459,8 +466,8 @@ static void *multifd_send_thread(void *opaque)
>> qemu_mutex_unlock(&p->mutex);
>> break;
>> }
>> - if (p->address) {
>> - p->address = 0;
>> + if (p->pages.num) {
>> + p->pages.num = 0;
>> qemu_mutex_unlock(&p->mutex);
>> qemu_mutex_lock(&multifd_send_state->mutex);
>> p->done = true;
>> @@ -475,6 +482,13 @@ static void *multifd_send_thread(void *opaque)
>> return NULL;
>> }
>>
>> +static void multifd_init_group(multifd_pages_t *pages)
>> +{
>> + pages->num = 0;
>> + pages->size = migrate_multifd_group();
>> + pages->iov = g_malloc0(pages->size * sizeof(struct iovec));
>
> Does that get freed anywhere?
Ooops. Now it does.
>> +}
>> +
>> int multifd_save_setup(void)
>> {
>> int thread_count;
>> @@ -498,7 +512,7 @@ int multifd_save_setup(void)
>> p->quit = false;
>> p->id = i;
>> p->done = true;
>> - p->address = 0;
>> + multifd_init_group(&p->pages);
>> p->c = socket_send_channel_create();
>> if (!p->c) {
>> error_report("Error creating a send channel");
>> @@ -515,8 +529,23 @@ int multifd_save_setup(void)
>>
>> static int multifd_send_page(uint8_t *address)
>> {
>> - int i;
>> + int i, j;
>> MultiFDSendParams *p = NULL; /* make happy gcc */
>> + static multifd_pages_t pages;
>> + static bool once;
>> +
>> + if (!once) {
>> + multifd_init_group(&pages);
>> + once = true;
>> + }
>> +
>> + pages.iov[pages.num].iov_base = address;
>> + pages.iov[pages.num].iov_len = TARGET_PAGE_SIZE;
>> + pages.num++;
>> +
>> + if (pages.num < (pages.size - 1)) {
>> + return UINT16_MAX;
>
> That's a very odd magic constant to return.
> What's your intention?
>
>> + }
>>
>> qemu_sem_wait(&multifd_send_state->sem);
>> qemu_mutex_lock(&multifd_send_state->mutex);
>> @@ -530,7 +559,12 @@ static int multifd_send_page(uint8_t *address)
>> }
>> qemu_mutex_unlock(&multifd_send_state->mutex);
>> qemu_mutex_lock(&p->mutex);
>> - p->address = address;
>> + p->pages.num = pages.num;
>> + for (j = 0; j < pages.size; j++) {
>> + p->pages.iov[j].iov_base = pages.iov[j].iov_base;
>> + p->pages.iov[j].iov_len = pages.iov[j].iov_len;
>> + }
>
> It would seem more logical to update p->pages.num last
>
> This is also a little odd in that iov_len is never really used,
> it's always TARGET_PAGE_SIZE.
changed by peter suggestion to iov_copy(). And iov_len is used in the
qio send functions, so we have a right value.
>> + pages.num = 0;
>> qemu_mutex_unlock(&p->mutex);
>> qemu_sem_post(&p->sem);
>
> What makes sure that any final chunk of pages that was less
> than the group size is sent at the end?
See last_page boolean in a following patch. It was the wrong place.
Thanks, Juan.
