From patchwork Tue Feb 1 17:03:11 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 81363 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E6D11B7110 for ; Wed, 2 Feb 2011 06:38:56 +1100 (EST) Received: from localhost ([127.0.0.1]:33073 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PkJsS-00033Z-M5 for incoming@patchwork.ozlabs.org; Tue, 01 Feb 2011 12:18:52 -0500 Received: from [140.186.70.92] (port=59196 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PkJlj-0008Vu-1a for qemu-devel@nongnu.org; Tue, 01 Feb 2011 12:12:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PkJdL-0002l9-CF for qemu-devel@nongnu.org; Tue, 01 Feb 2011 12:03:16 -0500 Received: from thoth.sbs.de ([192.35.17.2]:34661) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PkJdL-0002kt-1F for qemu-devel@nongnu.org; Tue, 01 Feb 2011 12:03:15 -0500 Received: from mail1.siemens.de (localhost [127.0.0.1]) by thoth.sbs.de (8.13.6/8.13.6) with ESMTP id p11H3CKL017297; Tue, 1 Feb 2011 18:03:12 +0100 Received: from mchn199C.mchp.siemens.de ([139.25.109.49]) by mail1.siemens.de (8.13.6/8.13.6) with ESMTP id p11H3BrT010111; Tue, 1 Feb 2011 18:03:11 +0100 Message-ID: <4D483CCF.60009@siemens.com> Date: Tue, 01 Feb 2011 18:03:11 +0100 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: Anthony Liguori References: <20110201155414.GF28968@x200.localdomain> <4D48367D.2060802@siemens.com> <4D483A9B.9000205@codemonkey.ws> In-Reply-To: <4D483A9B.9000205@codemonkey.ws> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 192.35.17.2 Cc: Chris Wright , "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" Subject: [Qemu-devel] Re: KVM call minutes for Feb 1 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 2011-02-01 17:53, Anthony Liguori wrote: > On 02/01/2011 10:36 AM, Jan Kiszka wrote: >> On 2011-02-01 16:54, Chris Wright wrote: >> >>> KVM upstream merge: status, plans, coordination >>> - Jan has a git tree, consolidating >>> - qemu-kvm io threading is still an issue >>> - Anthony wants to just merge >>> - concerns with non-x86 arch and merge >>> - concerns with big-bang patch merge and following stability >>> - post 0.14 conversion to glib mainloop, non-upstreamed qemu-kvm will be >>> a problem if it's not there by then >>> - testing and nuances are still an issue (e.g. stefan berger's mmio read issue) >>> - qemu-kvm still evolving, needs to get sync'd or it will keep diverging >>> - 2 implementations of main init, cpu init, Jan has merged them into one >>> - qemu-kvm-x86.c file that's only a few hundred lines >>> - review as one patch to see the fundamental difference >>> >> More precisely, my current work flow is to pick some function(s), e.g. >> kvm_cpu_exec/kvm_run, and start wondering "What needs to be done to >> upstream so that qemu-kvm could use that implementation?". If they >> differ, the reasons need to be understood and patched away, either by >> fixing/enhancing upstream or simplifying qemu-kvm. Once the upstream >> changes are merged back, a qemu-kvm patch is posted to switch to that >> version. >> >> Any help will be welcome, either via review of my subtle regressions or >> on resolving concrete differences. >> >> E.g. posix-aio-compat.c: Why does qemu-kvm differ here? If it's because >> of its own iothread code, can we wrap that away or do we need to >> consolidate the threading code first? Or do we need to fix something in >> upstream? >> > > I bet it's the eventfd thing. It's arbitrary. If you've got a small > diff post your series, I'd be happy to take a look at it and see what I > can explain. > Looks like it's around signalfd and its emulation: [git diff qemu/master..master posix-aio-compat.c] Jan diff --git a/posix-aio-compat.c b/posix-aio-compat.c index fa5494d..0704064 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -28,6 +28,7 @@ #include "qemu-common.h" #include "trace.h" #include "block_int.h" +#include "compatfd.h" #include "block/raw-posix-aio.h" @@ -55,7 +56,7 @@ struct qemu_paiocb { }; typedef struct PosixAioState { - int rfd, wfd; + int fd; struct qemu_paiocb *first_aio; } PosixAioState; @@ -474,18 +475,29 @@ static int posix_aio_process_queue(void *opaque) static void posix_aio_read(void *opaque) { PosixAioState *s = opaque; - ssize_t len; + union { + struct qemu_signalfd_siginfo siginfo; + char buf[128]; + } sig; + size_t offset; - /* read all bytes from signal pipe */ - for (;;) { - char bytes[16]; + /* try to read from signalfd, don't freak out if we can't read anything */ + offset = 0; + while (offset < 128) { + ssize_t len; - len = read(s->rfd, bytes, sizeof(bytes)); + len = read(s->fd, sig.buf + offset, 128 - offset); if (len == -1 && errno == EINTR) - continue; /* try again */ - if (len == sizeof(bytes)) - continue; /* more to read */ - break; + continue; + if (len == -1 && errno == EAGAIN) { + /* there is no natural reason for this to happen, + * so we'll spin hard until we get everything just + * to be on the safe side. */ + if (offset > 0) + continue; + } + + offset += len; } posix_aio_process_queue(s); @@ -499,20 +511,6 @@ static int posix_aio_flush(void *opaque) static PosixAioState *posix_aio_state; -static void aio_signal_handler(int signum) -{ - if (posix_aio_state) { - char byte = 0; - ssize_t ret; - - ret = write(posix_aio_state->wfd, &byte, sizeof(byte)); - if (ret < 0 && errno != EAGAIN) - die("write()"); - } - - qemu_service_io(); -} - static void paio_remove(struct qemu_paiocb *acb) { struct qemu_paiocb **pacb; @@ -616,9 +614,8 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, int paio_init(void) { - struct sigaction act; + sigset_t mask; PosixAioState *s; - int fds[2]; int ret; if (posix_aio_state) @@ -626,24 +623,21 @@ int paio_init(void) s = qemu_malloc(sizeof(PosixAioState)); - sigfillset(&act.sa_mask); - act.sa_flags = 0; /* do not restart syscalls to interrupt select() */ - act.sa_handler = aio_signal_handler; - sigaction(SIGUSR2, &act, NULL); + /* Make sure to block AIO signal */ + sigemptyset(&mask); + sigaddset(&mask, SIGUSR2); + sigprocmask(SIG_BLOCK, &mask, NULL); s->first_aio = NULL; - if (qemu_pipe(fds) == -1) { - fprintf(stderr, "failed to create pipe\n"); + s->fd = qemu_signalfd(&mask); + if (s->fd == -1) { + fprintf(stderr, "failed to create signalfd\n"); return -1; } - s->rfd = fds[0]; - s->wfd = fds[1]; - - fcntl(s->rfd, F_SETFL, O_NONBLOCK); - fcntl(s->wfd, F_SETFL, O_NONBLOCK); + fcntl(s->fd, F_SETFL, O_NONBLOCK); - qemu_aio_set_fd_handler(s->rfd, posix_aio_read, NULL, posix_aio_flush, + qemu_aio_set_fd_handler(s->fd, posix_aio_read, NULL, posix_aio_flush, posix_aio_process_queue, s); ret = pthread_attr_init(&attr);