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

Michael S. Tsirkin
2009-06-02T17:03:07Z

On Tue, Jun 02, 2009 at 07:59:49PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 02, 2009 at 12:34:12PM -0400, Gregory Haskins wrote:
> > Michael S. Tsirkin wrote:
> > > On Tue, Jun 02, 2009 at 12:14:15PM -0400, Gregory Haskins wrote:
> > >
> > >> Michael S. Tsirkin wrote:
> > >>
> > >>> On Tue, Jun 02, 2009 at 11:15:28AM -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. I have it as an RFC
> > >>>> for now because it needs Davide's official submission/SOB for patch 1/2, and
> > >>>> it should get some eyeballs/acks on my SRCU usage before going in.
> > >>>>
> > >>>> I will submit the updated irqfd userspace which eschews the deassign() verb
> > >>>> since we can now just use the close(fd) method alone. I will also address
> > >>>> the userspace review comments from Avi.
> > >>>>
> > >>>>
> > >>> We are not killing the deassign though, do we?
> > >>>
> > >>>
> > >> Yes, it is not needed any more now that we have proper
> > >> release-notification from eventfd.
> > >>
> > >>
> > >>> It's good to have that option e.g. for when we pass
> > >>> the fd to another process.
> > >>>
> > >>>
> > >> Passing the fd to another app should up the underlying file reference
> > >> count. If the producer app wants to "deassign" it simply calls
> > >> close(fd) (as opposed to today where it calls DEASSIGN+close), but the
> > >> reference count will allow the consuming app to leave the eventfd's file
> > >> open. Or am I misunderstanding you?
> > >>
> > >> -Greg
> > >>
> > >>
> > >>
> > >
> > > I think we want to keep supporting the deassign ioctl. This, even though
> > > close overlaps with it functionally somewhat.
> > >
> > > This allows qemu to pass eventfd to another process/device, and then
> > > block/unblock interrupts as seen by that process by
> > > assigning/deassigning irq to it. This is much easier and lightweight
> > > than asking another process to close the fd and passing another fd
> > > later.
> > >
> > >
> > Perhaps, but if that is the case we should just ignore this series and
> > continue with the DEASSIGN+close methodology since it already provides
> > that separation. Trying to do a hybrid is just messy.
>
> As I see it, it's the least evil.
>
> One-way ioctl operations on file descriptors are messier still. What's
> another example of an ioctl that can't be undone without closing the fd?
> And having close not clean up the state unless you do an ioctl first is
> very messy IMO - I don't think you'll find any such examples in kernel.
>
> > But in any case, I think that approach is flawed. DEASSIGN shouldn't be
> > used as a mask in my opinion, and we shouldn't be reassigning a
> > channel's meaning under the covers like that. If this is in fact a
> > valid use case, we should have a separate "GSI_MASK" type operation that
> > is independent of irqfd.
> > Likewise, we really should pass a new fd if
> > the gsi-routing is changing. Today there is a tight coupling of
> > fd-to-gsi, and I think that makes sense to continue this association.
> >
> > -Greg
> >
>
> I'm not arguing that this use-case is not theoretical. Just that if you
> don't create the fd to connect to GSI, you shouln't ask the user to
> destroy it to disconnect. Who knows what else this eventfd descriptor
> can be used for?

As a follow-up, here's another example: imagine an application that
handles interrupt events from a thread by blocking on eventfd. To wake
up this thread, we could reuse the same eventfd just by writing there. I
might want to do this even after I don't get any interrupts anymore.

--
MST