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@xxxxxxxxxx> > > 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
