Set the iothread's eventfd/pipe descriptors to non-blocking
It replaces the previous fix of using select.
Quoted 1 line
From
ab5ae4bb69f8ab6c9a476f7823cb8d6729d31594Mon Sep 17 00:00:00 2001
From: Dor Laor <[email protected]>
Date: Wed, 15 Jul 2009 17:53:16 +0300
Subject: [PATCH] Set the iothread's eventfd/pipe descriptors to non-blocking.
It fixes 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.
Mark McLoughlin suggested to remove spurious wake-up of the migration code when we get EAGAIN and wait for the socket to become writeable. (+1)
Nevertheless, the pipe descriptors shouldn't be blocking and the reader can also read several chunks in a time.
--- qemu-kvm.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)
qemu-kvm.c +14 -10
@@ -2107,14 +2107,17 @@ void qemu_kvm_notify_work(void)if (len ==-1 && errno ==EINTR )continue ; -if (len <=0 ) +/* In case we have a pipe, there is not reason to insist writing +* 8 bytes +*/ +if (len ==-1 && errno ==EAGAIN )break ; +if (len <=0 ) +break ; + offset += len; } - -if (offset !=8 ) -fprintf (stderr,"failed to notify io thread\n" ); }/* If we have signalfd, we mask out the signals we want to handle and then
@@ -2153,20 +2156,18 @@ static void sigfd_handler(void *opaque)static void io_thread_wakeup (void *opaque) {int fd = (unsigned long )opaque; -char buffer[8 ]; -size_t offset =0 ; +char buffer[4096 ]; -while (offset <8 ) { +/* Drain the pipe/(eventfd) */ +while (1 ) {ssize_t len; - len =read (fd, buffer + offset,8 - offset); + len =read (fd, buffer,sizeof (buffer));if (len ==-1 && errno ==EINTR )continue ;if (len <=0 )break ; - - offset += len; } }
@@ -2184,6 +2185,9 @@ int kvm_main_loop(void)return -errno; } +fcntl (fds[0 ],F_SETFL ,O_NONBLOCK ); +fcntl (fds[1 ],F_SETFL ,O_NONBLOCK ); +qemu_set_fd_handler2 (fds[0 ],NULL , io_thread_wakeup,NULL , (void *)(unsigned long )fds[0 ]);
Signature · 1 line
1.6.2.5
Quoted 78 lines · Dor Laor
It replaces the previous fix of using select.
Quoted 1 line
From
ab5ae4bb69f8ab6c9a476f7823cb8d6729d31594Mon Sep 17 00:00:00 2001From: Dor Laor <[email protected]>
Date: Wed, 15 Jul 2009 17:53:16 +0300
Subject: [PATCH] Set the iothread's eventfd/pipe descriptors to non-blocking.It fixes migration issue when the destination is loaded.
If the migration socket is full, we get EAGAIN for the write. The
set_fd_handler2defers the write for later on. The function tries to wake up the iothread byqemu_kvm_notify_work.
Since this happens in a loop, multiple times, the pipe that emulates eventfd becomes full and we get a deadlock.Mark McLoughlin suggested to remove spurious wake-up of the migration code when we get EAGAIN and wait for the socket to become writeable. (+1)
Nevertheless, the pipe descriptors shouldn't be blocking and the reader can also read several chunks in a time.
Signed-off-by Dor Laor <[email protected]>--- qemu-kvm.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)qemu-kvm.c +11
-10@@ -2107,14 +2107,17 @@ void qemu_kvm_notify_work(void)if (len ==-1 && errno ==EINTR )continue ; -if (len <=0 ) +/* In case we have a pipe, there is not reason to insist writing +* 8 bytes +*/ +if (len ==-1 && errno ==EAGAIN )break ; +if (len <=0 ) +break ; + offset += len; } - -if (offset !=8 ) -fprintf (stderr,"failed to notify io thread\n" ); }/* If we have signalfd, we mask out the signals we want to handle and then @@ -2153,20 +2156,18 @@ static void sigfd_handler(void *opaque)static void io_thread_wakeup (void *opaque) {int fd = (unsigned long )opaque; -char buffer[8 ]; -size_t offset =0 ; +char buffer[4096 ]; -while (offset <8 ) { +/* Drain the pipe/(eventfd) */ +while (1 ) {ssize_t len; - len =read (fd, buffer + offset,8 - offset); + len =read (fd, buffer,sizeof (buffer));if (len ==-1 && errno ==EINTR )continue ;if (len <=0 )break ; - - offset += len; } }
qemu-kvm.c int kvm_main_loop(void) · line 2193
Quoted 9 lines
qemu-kvm.c +3
-0@@ -2184,6 +2185,9 @@ int kvm_main_loop(void)return -errno; } +fcntl (fds[0 ],F_SETFL ,O_NONBLOCK ); +fcntl (fds[1 ],F_SETFL ,O_NONBLOCK ); +qemu_set_fd_handler2 (fds[0 ],NULL , io_thread_wakeup,NULL , (void *)(unsigned long )fds[0 ]);
Looks fine to me. Anthony, can you ACK?
Marcelo Tosatti
AppliedQuoted 78 lines · Dor Laor
It replaces the previous fix of using select.
Quoted 1 line
From
ab5ae4bb69f8ab6c9a476f7823cb8d6729d31594Mon Sep 17 00:00:00 2001From: Dor Laor <[email protected]>
Date: Wed, 15 Jul 2009 17:53:16 +0300
Subject: [PATCH] Set the iothread's eventfd/pipe descriptors to non-blocking.It fixes migration issue when the destination is loaded.
If the migration socket is full, we get EAGAIN for the write. The
set_fd_handler2defers the write for later on. The function tries to wake up the iothread byqemu_kvm_notify_work.
Since this happens in a loop, multiple times, the pipe that emulates eventfd becomes full and we get a deadlock.Mark McLoughlin suggested to remove spurious wake-up of the migration code when we get EAGAIN and wait for the socket to become writeable. (+1)
Nevertheless, the pipe descriptors shouldn't be blocking and the reader can also read several chunks in a time.
Signed-off-by Dor Laor <[email protected]>--- qemu-kvm.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)qemu-kvm.c +11
-10@@ -2107,14 +2107,17 @@ void qemu_kvm_notify_work(void)if (len ==-1 && errno ==EINTR )continue ; -if (len <=0 ) +/* In case we have a pipe, there is not reason to insist writing +* 8 bytes +*/ +if (len ==-1 && errno ==EAGAIN )break ; +if (len <=0 ) +break ; + offset += len; } - -if (offset !=8 ) -fprintf (stderr,"failed to notify io thread\n" ); }/* If we have signalfd, we mask out the signals we want to handle and then @@ -2153,20 +2156,18 @@ static void sigfd_handler(void *opaque)static void io_thread_wakeup (void *opaque) {int fd = (unsigned long )opaque; -char buffer[8 ]; -size_t offset =0 ; +char buffer[4096 ]; -while (offset <8 ) { +/* Drain the pipe/(eventfd) */ +while (1 ) {ssize_t len; - len =read (fd, buffer + offset,8 - offset); + len =read (fd, buffer,sizeof (buffer));if (len ==-1 && errno ==EINTR )continue ;if (len <=0 )break ; - - offset += len; } }
qemu-kvm.c int kvm_main_loop(void) · line 2192
Quoted 8 lines
qemu-kvm.c +3
-0@@ -2184,6 +2185,9 @@ int kvm_main_loop(void)return -errno; } +fcntl (fds[0 ],F_SETFL ,O_NONBLOCK ); +fcntl (fds[1 ],F_SETFL ,O_NONBLOCK ); +qemu_set_fd_handler2 (fds[0 ],NULL , io_thread_wakeup,NULL , (void *)(unsigned long )fds[0 ]);
Applied, thanks.