[Qemu-devel] QEMU question: is eventfd not thread safe?

[Alexey Kardashevskiy]

QEMU from qemu.org/master (not qemu-kvm), linux host 3.4, ppc64.

Shortly the problem is in the host kernel: closing a file in one thread does 
not interrupt select() waiting on the same file description in another thread.

Longer story is:
I'll use VFIO as an example as I hit this when I was debugging it but VFIO 
itself has nothing to do with the issue.

The bug looks like: I start the guest with MSI-capable PCI card passed via 
VFIO. The guest does dhclient from .bashrc and this dhclient does not receive 
anything till I press any key.
I did not see it for a while as I always start the guest and then typed 
"dhclient" manually in the guest console.

What happens:
VFIO initializes INTx eventfd via event_notifier_init() (returns fd=XX) and 
qemu_set_fd_handler() before the guest starts. Then QEMU starts the guest and 
enters a loop is os_host_main_loop_wait() which stays in select() until 
something happens.

>From the host kernel prospective, the XX fd was allocated, struct file* (P1) 
>with eventfd specific private_data allocated and initialized. select() added a 
>file refcounter (called get_file() in __pollwait()) and started waiting on 
>file* P1 but not on fd's number XX (what is mmm weird but ok).

The guest starts and tries to initialize MSI for the PCI card passed through. 
The guest does PCI config write and this write creates a second thread in QEMU. 
In this thread QEMU-VFIO closes fd XX which makes the host kernel release 
fd=XX, release the corresponding file* P1 (fput() in filp_close()) but this 
does not free this P1 as there is select() waiting on it.
eventfd_release() could wake it up but it is called when its refcounter becomes 
0 and this won't happen till select() interrupts.

Doing MSI init stuff, QEMU-VFIO calls the same event_notifier_init() (returns 
recycled fd=XX what is correct but confuses) and qemu_set_fd_handler() which 
adds a handler but select() does not pick it up.
The eventfd() (called by event_notifier_init()) allocates new struct file *P2 
in the kernel, QEMU-VFIO passes this fd=XX to the kernel's VFIO. When MSI 
interrupt comes to the host kernel, the VFIO interrupt handler calls 
eventfd_signal() on the correct file* P2. However do_select() in the kernel 
does not interrupt to deliver eventfd event as it is still waiting on P1 - 
nobody interrupted select(). It can only interrupt on stdin events (like typing 
keys) and INTx interrupt (which does not happen as MSI is enabled).

So we need to sync eventfd()/close() calls in one thread with select() in 
another. Below is the patch which simply sends SIGUSR2 to the main thread (the 
sample patch is below). Another solution could be adding new IO handler to wake 
select() up. Or to do something with the kernel but I am not sure what - 
implementing file_operations::flush for eventfd to do wakeup did not help and I 
did not dig any further.


Does it make sense? What do I miss? What would be the right solution?



---
 iohandler.c |    1 +
 main-loop.c |   17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/iohandler.c b/iohandler.c
index 3c74de6..54f4c48 100644
--- a/iohandler.c
+++ b/iohandler.c
@@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd,
         ioh->fd_write = fd_write;
         ioh->opaque = opaque;
         ioh->deleted = 0;
+        kill(getpid(), SIGUSR2);
     }
     return 0;
 }
diff --git a/main-loop.c b/main-loop.c
index eb3b6e6..f65e048 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -199,10 +199,27 @@ static int qemu_signal_init(void)
 }
 #endif
 
+static void sigusr2_print(int signal)
+{
+}
+
+static void sigusr2_init(void)
+{
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    sigfillset(&action.sa_mask);
+    action.sa_handler = sigusr2_print;
+    action.sa_flags = 0;
+    sigaction(SIGUSR2, &action, NULL);
+}
+
 int main_loop_init(void)
 {
     int ret;
 
+    sigusr2_init();
+
     qemu_mutex_lock_iothread();
     ret = qemu_signal_init();
     if (ret) {
-- 
1.7.10