From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8Ym5-0006p2-9f for qemu-devel@nongnu.org; Tue, 27 Sep 2011 10:36:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R8Ylv-0004AN-GC for qemu-devel@nongnu.org; Tue, 27 Sep 2011 10:36:45 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:51667) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8Ylv-0004AG-Cg for qemu-devel@nongnu.org; Tue, 27 Sep 2011 10:36:35 -0400 Received: by yxl11 with SMTP id 11so6905976yxl.4 for ; Tue, 27 Sep 2011 07:36:34 -0700 (PDT) Message-ID: <4E81DF6F.8080905@codemonkey.ws> Date: Tue, 27 Sep 2011 09:36:31 -0500 From: Anthony Liguori 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> <4E81DDDF.8050201@siemens.com> In-Reply-To: <4E81DDDF.8050201@siemens.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: Jan Kiszka Cc: Kevin Wolf , Anthony Liguori , Stefan Hajnoczi , Marcelo Tosatti , qemu-devel , Avi Kivity On 09/27/2011 09:29 AM, Jan Kiszka wrote: > 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. uint64_t value; for (i = 0; i < 1 << 32; i++) { value = 1; write(fd, &value, sizeof(value)); } uint64_t count = 0; do { len = read(fd, &value, sizeof(value)); count += value; } while (len != -1); With eventfd, count == 2^32. With pipe, count == 8192. That's each '1' is stored in the pipe buffer whereas with eventfd, an index is just incremented. Not sure what you mean re: EFD_SEMAPHORE. EFD_SEMAPHORE basically means any non-zero value is returned as 1 and the counter is decremented by 1. Without EFD_SEMAPHORE, the count is returned and the counter is reset to 0. Regards, Anthony Liguori > > Jan >