From patchwork Sun Jul 1 11:06:20 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Kardashevskiy X-Patchwork-Id: 168377 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 086A02C01E0 for ; Sun, 1 Jul 2012 21:06:41 +1000 (EST) Received: from localhost ([::1]:58417 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlHzB-0005uT-MV for incoming@patchwork.ozlabs.org; Sun, 01 Jul 2012 07:06:37 -0400 Received: from eggs.gnu.org ([208.118.235.92]:46988) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlHz3-0005u4-SO for qemu-devel@nongnu.org; Sun, 01 Jul 2012 07:06:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SlHz1-0003OF-Is for qemu-devel@nongnu.org; Sun, 01 Jul 2012 07:06:29 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:49121) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlHz1-0003O7-9l for qemu-devel@nongnu.org; Sun, 01 Jul 2012 07:06:27 -0400 Received: by dadn2 with SMTP id n2so6886732dad.4 for ; Sun, 01 Jul 2012 04:06:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :content-type:content-transfer-encoding:x-gm-message-state; bh=iwKlZQ/TODyX1rA2o1LHnbXFFwAcX6ldUgw7PhfjiiY=; b=O536QuPVi6eIhas4UZ66ly+ArNYAzecnhaHekBmUaEwjpZ+W4oVM5HrKokZdbMQ+hE XwzREgZhN3JyyKMwvWJs8zwMxG6H7tYnhexgq3uFLh4w8qn4b6wHGt3ZLhu88yTnqyIB dwXWzvkCSoS7193E0/WqVb7wLcThT9BugdBrOqtpJ1bEcrEq6WlaJgr4fNWjoxoI5tmp AsYblTKTMrjwxYvrTxLg84JXkQjnqALIuKJv3iV8NoDXE4a35D7zpGyLDwYOa+d2am+2 m+iJ6cHL4spYcHnSTgqO5T+CBDATi2WPQ/rl23hVHrErkaSJD5gg9lnAH9S6NZSkElL1 vMhg== Received: by 10.66.86.165 with SMTP id q5mr13455843paz.72.1341140783583; Sun, 01 Jul 2012 04:06:23 -0700 (PDT) Received: from [192.168.1.50] (60-242-102-4.tpgi.com.au. [60.242.102.4]) by mx.google.com with ESMTPS id to1sm10324835pbc.27.2012.07.01.04.06.20 (version=SSLv3 cipher=OTHER); Sun, 01 Jul 2012 04:06:23 -0700 (PDT) Message-ID: <4FF02F2C.30006@ozlabs.ru> Date: Sun, 01 Jul 2012 21:06:20 +1000 From: Alexey Kardashevskiy User-Agent: Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20120428 Thunderbird/12.0.1 MIME-Version: 1.0 To: qemu-devel@nongnu.org X-Gm-Message-State: ALoCoQlDGVLvzB1cb6OsWRBOlRTxEVAl3yYz2OITZFTaTk21N2O3k0wgjtHVeNO459jJVdl7pWH1 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.210.45 Cc: Alex Williamson , David Gibson , mst@redhat.com Subject: [Qemu-devel] QEMU question: is eventfd not thread safe? X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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) {