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: Thu, 7 Jan 2010 08:45:03 +0200 Message-ID: <20100107064503.GA10379@redhat.com> References: <4A9CB318.7030401@redhat.com> <20100106193347.GG4001@redhat.com> <20100106205526.GJ4001@redhat.com> <20100106222920.GB29200@redhat.com> <20100106234521.GA30515@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]:14685 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752522Ab0AGGr4 (ORCPT ); Thu, 7 Jan 2010 01:47:56 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jan 06, 2010 at 03:59:11PM -0800, Davide Libenzi wrote: > On Thu, 7 Jan 2010, Michael S. Tsirkin wrote: > > > On Wed, Jan 06, 2010 at 02:46:06PM -0800, Davide Libenzi wrote: > > > On Thu, 7 Jan 2010, Michael S. Tsirkin wrote: > > > > > > > > On top of creating an interface which requires a lock, which noone can get > > > > > from the interface itself, since it's not exposed. > > > > > > > > I think here's how KVM gets it: the way it does is by calling poll with > > > > our own poll table, then in poll_queue_proc we get wait queue pointer, > > > > and we use the wait queue. Lock is in there :) > > > > > > Yes, I know you are called locked, but it does not lead to a clean > > > interface. > > > > True. > > > > > > > I could split the two and have a locked one, and an unlocked one, but that > > > > > looks shitty too (for the above reason). > > > > > > > > Yes, this will work. Thanks! > > > > > > This is a lot more complex than I thought. The wakeup code is already > > > enumerating the list, and doing a wakeup might trigger a secondary > > > enumeration/recursion. > > > > For KVM what you describe is I think is not a problem: we check wake type > > and ignore POLLOUT events. > > You seem to think in one dimension only ;) > The interface needs to be stable for everyone. > > > > > Maybe yes. I'll think it over and get back to you. Thanks! > > Let me know. > > > > - Davide OK. What I think we need is a way to remove ourselves from the eventfd wait queue and clear the counter atomically. We currently do remove_wait_queue(irqfd->wqh, &irqfd->wait); where wqh saves the eventfd wait queue head. If we do this before proposed eventfd_read_ctx, we can lose events. If we do this after, we can get spurious events. An unlocked read is one way to fix this. -- MST