Re: [KVM PATCH v2 0/2] irqfd: use POLLHUP notification for close()

Avi Kivity
2009-06-14T12:52:06Z

Gregory Haskins wrote:
> Michael S. Tsirkin wrote:
>
>> [ Resending with correct address for Davide. Pls don't reply
>> to the original one, you'll get bounces. ]
>>
>> On Thu, Jun 04, 2009 at 08:48:02AM -0400, Gregory Haskins wrote:
>>
>>
>>> (Applies to kvm.git/master:25deed73)
>>>
>>> Please see the header for 2/2 for a description. This patch series has been
>>> fully tested and appears to be working correctly.
>>>
>>> [Review notes:
>>> *) Paul has looked at the SRCU design and, to my knowledge, didn't find
>>> any holes.
>>> *) Michael, Avi, and myself agree that while the removal of the DEASSIGN
>>> vector is not desirable, the fix on close() is more important in
>>> the short-term. We can always add DEASSIGN support again in the
>>> future with a CAP bit.
>>> ]
>>>
>>>
>> So, I've been thinking about this, and this approach has another
>> problem: it depends on pollhup on close which is AFAIK an
>> eventfd-specific feature.
>>
>
> Thats ok with me, as we are already married to eventfd for other reasons
> (see eventfd_fget()).
>
>
>> This will prevent us from supporting polling
>> other useful file types, such as sockets and pipes, down the road, with
>> this interface.
>>
>>
> I am thinking that we would add explicit support in the future if there
> are other fd types that might want to also inject interrupts. For
> instance, perhaps POLLHUP is added to pipes if/when they are patched as
> a valid transport for irqfd. Or perhaps irqfd is abstracted such that
> eventfd_fget/POLLHUP are eventfd specific assign/deassign implementation
> details.
>
> Another option is that we s/irqfd/irq-eventfd to leave room for other
> interfaces like irq-pollfd, irq-socketfd, etc. IOW, there is no reason
> to make the current irqfd code "one-fd-interface to rule them all" per
> se. The real abstraction is the kvm_set_irq() + gsi interface anyway.
> The current irqfd code is a thin shim in front of that. Perhaps each fd
> type would be better served with code to specifically handle each type,
> for its hard to predict what the requirements for translating, say, a
> pipe-write into a gsi-inject will be apriori.
>
>

I don't see a reason to avoid a monogamous relationship with eventfd as
it exactly captures the essence of an raising an interrupt: events are
coalesced and it doesn't block. Since irqfd will rarely work by itself
(need a separate data channel), having things like a tcp socket inject
an interrupt are, while exotic, fairly useless.

>> I didn't realise these implications when I suggested deassign on close.
>> To me, it now looks like we are better off reverting this patch.
>> We can later add 'deassign on close' support with CAP bit after all :)
>>
>> Avi, Gregory, what's your take?
>>
>>
>>
> I like the design with the single-call close in place, so my vote is to
> keep it as it is now.
>

We could work around it by allocating a gsi private to the eventfd, and
when we want to mask the gsi, simply drop all its routes. Hacky.

--
error compiling committee.c: too many arguments to function