Re: [PATCH 0/2] eventfd: new EFD_STATE flag

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jan 06, 2010 at 12:43:07PM -0800, Davide Libenzi wrote:
> On Wed, 6 Jan 2010, Michael S. Tsirkin wrote:
> 
> > 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?
> 
> What is wrong with you? :/

I was trying to be helpful.

> There was a patch attached with this email, and yet you did your own,

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?

Thanks!


-- 
MST