Re: [PATCH v7 1/2] kvm: Extend irqfd to support level interrupts

[Alex Williamson <alex.williamson@redhat.com>]

On Sun, 2012-07-29 at 18:01 +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 24, 2012 at 02:43:14PM -0600, Alex Williamson wrote:
> > In order to inject a level interrupt from an external source using an
> > irqfd, we need to allocate a new irq_source_id.  This allows us to
> > assert and (later) de-assert an interrupt line independently from
> > users of KVM_IRQ_LINE and avoid lost interrupts.
> > 
> > We also add what may appear like a bit of excessive infrastructure
> > around an object for storing this irq_source_id.  However, notice
> > that we only provide a way to assert the interrupt here.  A follow-on
> > interface will make use of the same irq_source_id to allow de-assert.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> 
> I think this tracking of source ids is the root of all the problems
> you see with this patchset.
> 
> A source ID is required for an irqfd to be created.
> But if source ID exists after irqfd is destroyed then
> the next create will fail.

Only if there are no available source IDs.

> So the only sane thing to do is to make irqfd manage this resource,
> clean it up completely when irqfd is gone.
> 
> Not to mention, the patch will be smaller :)

The only sane way to do that is to pull the eoifd into KVM_IRQFD and set
them up together.  That's actually what v1 of this endeavor did.  My
intention with splitting eoifd from irqfd is that I think EOI
notification is potentially useful outside of this usage with irqfds and
I wanted an interface that could be used independently.  Someday, an
irqfd may not be the only way to generate a key.  Userspace may also
wish to register to receive notification-only for the existing user
source ID.

I do not think it's sane to have an eoifd configured using KVM_EOIFD and
destroyed using KVM_IRQFD.  As for smaller patch, I'm not convinced.  We
still have to watch for POLLHUP, which pulls in the bulk of the code.
And using the above approach of pulling eoifd setup into irqfd we have
to address what happens to the combined set when either eventfd is
closed.  By your argument closing the irqfd closes the eoifd, but does
closing the eoifd necessarily close the irqfd.  If not then we end up
with the question of how can an eoifd be added to an existing irqfd.