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

[Avi Kivity <avi@redhat.com>]

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