From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 0/2] eventfd: new EFD_STATE flag Date: Wed, 6 Jan 2010 21:33:48 +0200 Message-ID: <20100106193347.GG4001@redhat.com> References: <1251363930-3916-1-git-send-email-pbonzini@redhat.com> <20090827143027.GA22720@redhat.com> <20090827144905.GA22728@redhat.com> <4A9CB318.7030401@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm@vger.kernel.org To: Davide Libenzi Return-path: Received: from mx1.redhat.com ([209.132.183.28]:63994 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754622Ab0AFTgm (ORCPT ); Wed, 6 Jan 2010 14:36:42 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Sep 01, 2009 at 07:24:24AM -0700, Davide Libenzi wrote: > On Tue, 1 Sep 2009, Avi Kivity wrote: > > > On 09/01/2009 02:45 AM, Davide Libenzi wrote: > > > On Thu, 27 Aug 2009, Davide Libenzi wrote: > > > > > > > > > > On Thu, 27 Aug 2009, Michael S. Tsirkin wrote: > > > > > > > > > > > > > Oh, I stopped pushing EFD_STATE since we have a solution. > > > > > > > > > Do you guys need the kernel-side eventfd_ctx_read() I posted or not? > > > > Because if nobody uses it, I'm not going to push it. > > > > > > > Guys, I did not get a reply on this. Do you need me to push it, or you're > > > not going to use it at the end? > > > > > > > We'll use it eventually, but we're still some ways from it. > > OK, then bug me when you're going to need it. I won't push it before that. > > > - Davide So, it turns out that we need this: be thought we don't because currently kvm does not zero eventfd counter when it polls eventfd. But this causes spurious interrupts when we disconnect irqfd from kvm and re-connect it back. However, since kvm does its own thing with the wait queue, and might read the counter from wait queue callback (which might be from interrupt context), a simpler, lower-level interface would be better for us. Does the following (build tested only) look palatable? Thanks! diff --git a/fs/eventfd.c b/fs/eventfd.c index d26402f..e350ffd 100644 --- a/fs/eventfd.c +++ b/fs/eventfd.c @@ -135,6 +135,17 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait) return events; } +/* Caller must have wait queue head lock. */ +ssize_t _eventfd_read_ctx(struct eventfd_ctx *ctx, u64 *ucnt) +{ + if (!ctx->count) + return -EAGAIN; + *ucnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count; + ctx->count -= *ucnt; + return sizeof *ucnt; +} +EXPORT_SYMBOL_GPL(_eventfd_read_ctx); + static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -146,17 +157,14 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count, if (count < sizeof(ucnt)) return -EINVAL; spin_lock_irq(&ctx->wqh.lock); - res = -EAGAIN; - if (ctx->count > 0) - res = sizeof(ucnt); - else if (!(file->f_flags & O_NONBLOCK)) { + res = _eventfd_read_ctx(ctx, &ucnt); + if (res < 0 && !(file->f_flags & O_NONBLOCK)) { __add_wait_queue(&ctx->wqh, &wait); for (res = 0;;) { set_current_state(TASK_INTERRUPTIBLE); - if (ctx->count > 0) { - res = sizeof(ucnt); + res = _eventfd_read_ctx(ctx, &ucnt); + if (res > 0) break; - } if (signal_pending(current)) { res = -ERESTARTSYS; break; @@ -169,8 +177,6 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count, __set_current_state(TASK_RUNNING); } if (likely(res > 0)) { - ucnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count; - ctx->count -= ucnt; if (waitqueue_active(&ctx->wqh)) wake_up_locked_poll(&ctx->wqh, POLLOUT); } diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h index 94dd103..a3d0ce9 100644 --- a/include/linux/eventfd.h +++ b/include/linux/eventfd.h @@ -34,6 +34,7 @@ struct file *eventfd_fget(int fd); struct eventfd_ctx *eventfd_ctx_fdget(int fd); struct eventfd_ctx *eventfd_ctx_fileget(struct file *file); int eventfd_signal(struct eventfd_ctx *ctx, int n); +ssize_t _eventfd_read_ctx(struct eventfd_ctx *ctx, u64 *ucnt); #else /* CONFIG_EVENTFD */ @@ -61,6 +62,11 @@ static inline void eventfd_ctx_put(struct eventfd_ctx *ctx) } +static inline ssize_t _eventfd_read_ctx(struct eventfd_ctx *ctx, u64 *ucnt) +{ + return -ENOSYS; +} + #endif #endif /* _LINUX_EVENTFD_H */ -- MST