On Sun, 10 Jan 2010, Michael S. Tsirkin wrote:

> > What is you do (under proper irqfd locking) something like:
> > 
> >     eventfd_ctx_read(ctx, 1, &cnt);
> >     if (irqfd->cnt != cnt) {
> >             irqfd->cnt = cnt;
> >             schedule_work(&irqfd->inject);
> >     }
> > 
> > 
> > 
> > 
> > > And deactivation deep down does this (from irqfd_cleanup_wq workqueue,
> > > so this is not under the spinlock):
> > > 
> > >         /*
> > >          * Synchronize with the wait-queue and unhook ourselves to
> > >          * prevent
> > >          * further events.
> > >          */
> > > B.
> > >         remove_wait_queue(irqfd->wqh, &irqfd->wait);
> > > 
> > >   ....
> > > 
> > >         /*
> > >          * It is now safe to release the object's resources
> > >          */
> > >         eventfd_ctx_put(irqfd->eventfd);
> > >         kfree(irqfd);
> > 
> > And:
> > 
> >     eventfd_ctx_read(ctx, 1, &irqfd->cnt);
> 
> 
> ->
> 
> >     remove_wait_queue(irqfd->wqh, &irqfd->wait);
> > 
> > 
> > 
> > 
> > - Davide
> 
> Yes, this is exactly what I wanted to do.  So, here's the issue: if an
> event is signalled at point ->: after eventfd_ctx_read but before
> remove_wait_queue, then we inject interrupt but counter will be left
> non-zero and then when we unmask, we inject antoher, spurious interrupt.
> 
> This is why I wanted to have eventfd_ctx_read not take wait queue head
> lock: then I could do:
> 
>       spin_lock_irqsave(&ctx->wqh.lock, flags);
>       eventfd_ctx_read(ctx, 1, &irqfd->cnt);
>       __remove_wait_queue(irqfd->wqh, &irqfd->wait);
>       spin_lock_irqrestore(&ctx->wqh.lock, flags);

This is why I said "under proper irqfd locking".


- Davide


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to