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

[Avi Kivity]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/