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 00:29:20 +0200 Message-ID: <20100106222920.GB29200@redhat.com> References: <20090827144905.GA22728@redhat.com> <4A9CB318.7030401@redhat.com> <20100106193347.GG4001@redhat.com> <20100106205526.GJ4001@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]:51689 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755955Ab0AFWcO (ORCPT ); Wed, 6 Jan 2010 17:32:14 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jan 06, 2010 at 01:17:02PM -0800, Davide Libenzi wrote: > On Wed, 6 Jan 2010, Michael S. Tsirkin wrote: > > > I tried to explain, no? That patch was taking wait queue spinlock and > > was assuming that eventfd_read_ctx is called from a task that can block. > > KVM attaches its own poller so this is not a good fit for us. Hope this > > clarifies. > > > > > and yet again you managed to add an underscore at the beginning of the > > > API name, in an API set where there are no leading underscores. Even > > > if KVM does that for its own reasons, you should be able to fit your > > > naming "style" to the interface you're adding your droplets into. I > > > will post an eventfd_read_ctx() to Andrew ASAP. > > > > > > > > > > > > - Davide > > > > Sorry about the underscore - it's easy to fix but I won't bore you with > > more patch revisions before I understand what makes you unhappy. > > > > Before KVM starts creating workqueues and doing schedule_work calls just > > to work around the API, can we discuss this please? I am not hung on my > > patch, but could we have it work so that we can call eventfd_read_ctx > > from wait queue callback directly? > > The "read" needs to wake possible OUT waiters, *cannot* be done your way. Right. Now I see. Thanks! > 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 :) > 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! > I thought you *already* do your own stuff from a work queue, why would you > need to read from IRQ context? > > > > - Davide This was just work around while interrupt delivery was using mutex locks, so we had a workqueue to work around that limitation. Now that it has been all switched to RCU, we'll be getting getting rid of this. -- MST