From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38785) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8YfY-0003Hv-R0 for qemu-devel@nongnu.org; Tue, 27 Sep 2011 10:30:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R8YfU-0002fy-HP for qemu-devel@nongnu.org; Tue, 27 Sep 2011 10:30:00 -0400 Received: from goliath.siemens.de ([192.35.17.28]:33877) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8YfU-0002fr-52 for qemu-devel@nongnu.org; Tue, 27 Sep 2011 10:29:56 -0400 Message-ID: <4E81DDDF.8050201@siemens.com> Date: Tue, 27 Sep 2011 16:29:51 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <4E78C42D.5030207@siemens.com> <20110921080600.GA9847@stefanha-thinkpad.localdomain> <4E80B50B.9000301@siemens.com> <4E80B55F.5020203@redhat.com> <4E80BFF3.8000907@us.ibm.com> <4E8190BE.3000801@redhat.com> <4E81D609.1060203@siemens.com> <4E81D88B.4020504@codemonkey.ws> In-Reply-To: <4E81D88B.4020504@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Use qemu_eventfd for POSIX AIO List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , Anthony Liguori , Stefan Hajnoczi , Marcelo Tosatti , qemu-devel , Avi Kivity On 2011-09-27 16:07, Anthony Liguori wrote: > On 09/27/2011 08:56 AM, Jan Kiszka wrote: >> Move qemu_eventfd unmodified to oslib-posix and use it for signaling >> POSIX AIO completions. If native eventfd suport is available, this >> avoids multiple read accesses to drain multiple pending signals. As >> before we use a pipe if eventfd is not supported. >> >> Signed-off-by: Jan Kiszka >> --- >> os-posix.c | 32 -------------------------------- >> oslib-posix.c | 32 +++++++++++++++++++++++++++++++- >> posix-aio-compat.c | 12 ++++++++---- >> 3 files changed, 39 insertions(+), 37 deletions(-) >> >> diff --git a/os-posix.c b/os-posix.c >> index dbf3b24..a918895 100644 >> --- a/os-posix.c >> +++ b/os-posix.c >> @@ -45,10 +45,6 @@ >> #include >> #endif >> >> -#ifdef CONFIG_EVENTFD >> -#include >> -#endif >> - >> static struct passwd *user_pwd; >> static const char *chroot_dir; >> static int daemonize; >> @@ -333,34 +329,6 @@ void os_set_line_buffering(void) >> setvbuf(stdout, NULL, _IOLBF, 0); >> } >> >> -/* >> - * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set. >> - */ >> -int qemu_eventfd(int fds[2]) >> -{ >> -#ifdef CONFIG_EVENTFD >> - int ret; >> - >> - ret = eventfd(0, 0); >> - if (ret>= 0) { >> - fds[0] = ret; >> - qemu_set_cloexec(ret); >> - if ((fds[1] = dup(ret)) == -1) { >> - close(ret); >> - return -1; >> - } >> - qemu_set_cloexec(fds[1]); >> - return 0; >> - } >> - >> - if (errno != ENOSYS) { >> - return -1; >> - } >> -#endif >> - >> - return qemu_pipe(fds); >> -} >> - >> int qemu_create_pidfile(const char *filename) >> { >> char buffer[128]; >> diff --git a/oslib-posix.c b/oslib-posix.c >> index a304fb0..8ef7bd7 100644 >> --- a/oslib-posix.c >> +++ b/oslib-posix.c >> @@ -47,7 +47,9 @@ extern int daemon(int, int); >> #include "trace.h" >> #include "qemu_socket.h" >> >> - >> +#ifdef CONFIG_EVENTFD >> +#include >> +#endif >> >> int qemu_daemon(int nochdir, int noclose) >> { >> @@ -139,6 +141,34 @@ int qemu_pipe(int pipefd[2]) >> return ret; >> } >> >> +/* >> + * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set. >> + */ >> +int qemu_eventfd(int fds[2]) >> +{ >> +#ifdef CONFIG_EVENTFD >> + int ret; >> + >> + ret = eventfd(0, 0); >> + if (ret>= 0) { >> + fds[0] = ret; >> + qemu_set_cloexec(ret); >> + if ((fds[1] = dup(ret)) == -1) { >> + close(ret); >> + return -1; >> + } >> + qemu_set_cloexec(fds[1]); >> + return 0; >> + } >> + >> + if (errno != ENOSYS) { >> + return -1; >> + } >> +#endif >> + >> + return qemu_pipe(fds); >> +} >> + > > I think it's a bit dangerous to implement eventfd() in terms of pipe(). > > You don't expect to handle EAGAIN with eventfd() whereas you have to handle it > with pipe(). EAGAIN is returned on eventfd read if no event is pending and the fd is non-blocking - just as we configure it. > > Moreover, the eventfd() counter is not lossy (practically speaking) whereas if > you use pipe() as a counter, it will be lossy in practice. > > This is why posix aio uses pipe() and not eventfd(). I don't get this yet. eventfd is lossy by default. It only decreases the counter on read if you specify EFD_SEMAPHORE - which we do not do. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux