Skip to contentSkip to conversation

Fix migration issue when the destination is loaded

PATCH Dor Laor 2 participants 4 messages Jul 15, 2009 – Jul 16, 2009 KVM

lore
Share & tools
lore

Dor Laor

author
#0 lore raw
Quoted 1 line

From abbe3b57af6a28bb81e5fb8b09b10802a8ccc3fe Mon Sep 17 00:00:00 2001

From: Dor Laor <[email protected]>
Date: Wed, 15 Jul 2009 17:53:16 +0300
Subject: [PATCH] Fix migration issue when the destination is loaded

If the migration socket is full, we get EAGAIN for the write. The set_fd_handler2 defers the write for later on. The function tries to wake up the iothread by qemu_kvm_notify_work.
Since this happens in a loop, multiple times, the pipe that emulates eventfd becomes full and we get a deadlock.

It is solved by checking for write-readiness using select. Note that I check for select only for full 8 byte write and not for partial writes. This is because we'll break the reader otherwise.

---
 qemu-kvm.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
qemu-kvm.c +16 -0
index ed7e466..0ea67a7 100644
@@ -2094,12 +2094,28 @@ void qemu_kvm_notify_work(void)
     uint64_t value = 1;
     char buffer[8];
     size_t offset = 0;
+    fd_set wfds;
+    struct timeval tv;
+    int retval;
 
     if (io_thread_fd == -1)
 	return;
 
     memcpy(buffer, &value, sizeof(value));
 
+    FD_ZERO(&wfds);
+    FD_SET(io_thread_fd, &wfds);
+    tv.tv_sec = tv.tv_usec = 0;
+    retval = select(io_thread_fd + 1, NULL, &wfds, NULL, &tv);
+    if (retval == -1) {
+        fprintf(stderr, "failed to notify io thread due to select error\n");
+        return;
+    } else if (retval == 0)
+        /* We probably ponded this pipe too much and it is full now */
+        return;
+
+    assert(FD_ISSET(io_thread_fd, &wfds));
+
     while (offset < 8) {
 	ssize_t len;
 
Signature · 1 line
1.6.2.5
#1 lore raw

Hi Dor,

Quoted 5 lines · Dor Laor

If the migration socket is full, we get EAGAIN for the write. The set_fd_handler2 defers the write for later on. The function tries to wake up the iothread by qemu_kvm_notify_work. Since this happens in a loop, multiple times, the pipe that emulates eventfd becomes full and we get a deadlock.

I'm not sure I follow:

- You're seeing qemu_kvm_notify_work() being called many times

- The call chain is migrate_fd_put_buffer(), qemu_set_fd_handler2(), main_loop_break()

- This happens when write() in migrate_fd_put_buffer() returns EAGAIN because the socket buffer has filled up

Correct?

That sounds like migrate_fd_put_buffer() is being called repeatedly while we know the socket isn't writable?

Shouldn't the buffered file could stop attempting to call put_buffer() until it has been notified that the underlying fd is writable?

Cheers,
Mark.

author
#2 lore raw
Quoted 19 lines · Mark McLoughlin

Hi Dor,

Quoted 5 lines · Dor Laor

If the migration socket is full, we get EAGAIN for the write. The set_fd_handler2 defers the write for later on. The function tries to wake up the iothread by qemu_kvm_notify_work. Since this happens in a loop, multiple times, the pipe that emulates eventfd becomes full and we get a deadlock.

I'm not sure I follow:

- You're seeing qemu_kvm_notify_work() being called many times

- The call chain is migrate_fd_put_buffer(), qemu_set_fd_handler2(), main_loop_break()

- This happens when write() in migrate_fd_put_buffer() returns EAGAIN because the socket buffer has filled up

Quoted 7 lines

Correct?

That sounds like migrate_fd_put_buffer() is being called repeatedly while we know the socket isn't writable?

Shouldn't the buffered file could stop attempting to call put_buffer() until it has been notified that the underlying fd is writable?

There are two fds here:
The one returning EAGAIN, is the migration socket. That's why migrate_fd_put_buffer is called and a call back is rescheduled by qemu_set_fd_handler2.

In the procedure of qemu_set_fd_handler2, the main_loop_break is called. It needs to notify the iothread. It does is by qemu_eventfd, since it is being emulated on older kernels, we use a pipe.

The pipe fd is the one that blocks.
I though of setting it to non-blocking but we might get partial writes with a non blocking fd (in theory) too.

Quoted 4 lines

Cheers,
Mark.

#3 lore raw
Quoted 33 lines · Dor Laor
Quoted 19 lines · Mark McLoughlin

Hi Dor,

Quoted 5 lines · Dor Laor

If the migration socket is full, we get EAGAIN for the write. The set_fd_handler2 defers the write for later on. The function tries to wake up the iothread by qemu_kvm_notify_work. Since this happens in a loop, multiple times, the pipe that emulates eventfd becomes full and we get a deadlock.

I'm not sure I follow:

- You're seeing qemu_kvm_notify_work() being called many times

- The call chain is migrate_fd_put_buffer(), qemu_set_fd_handler2(), main_loop_break()

- This happens when write() in migrate_fd_put_buffer() returns EAGAIN because the socket buffer has filled up

Quoted 7 lines

Correct?

That sounds like migrate_fd_put_buffer() is being called repeatedly while we know the socket isn't writable?

Shouldn't the buffered file could stop attempting to call put_buffer() until it has been notified that the underlying fd is writable?

There are two fds here:
The one returning EAGAIN, is the migration socket. That's why migrate_fd_put_buffer is called and a call back is rescheduled by qemu_set_fd_handler2.

Quoted 7 lines

In the procedure of qemu_set_fd_handler2, the main_loop_break is called. It needs to notify the iothread. It does is by qemu_eventfd, since it is being emulated on older kernels, we use a pipe.

The pipe fd is the one that blocks.
I though of setting it to non-blocking but we might get partial writes with a non blocking fd (in theory) too.

Yes, but if the pipe fd is blocking, that means we're writing a lot of events to it, which means the write to the migration socket is failing with EAGAIN a lot.

As soon as we get EAGAIN on the migration socket, we should stop trying to write to socket until the we get notification that its writable.

If we did that, we'd only be writing a very small number of events to the pipe and we wouldn't block.

Cheers,
Mark.