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<jan.kis...@siemens.com>
>> ---
>> 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<sys/syscall.h>
>> #endif
>>
>> -#ifdef CONFIG_EVENTFD
>> -#include<sys/eventfd.h>
>> -#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<sys/eventfd.h>
>> +#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
