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

Gregory Haskins
2009-06-02T16:34:34Z

[-- Attachment #1: Type: text/plain, Size: 2616 bytes --]

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.

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 266 bytes --]