Re: [KVM PATCH v5 0/4] irqfd fixes and enhancements

[Gregory Haskins]

Gregory Haskins wrote:
> (Applies to kvm.git/master:4631e094)
>
> The following is the latest attempt to fix the races in irqfd/eventfd, as
> well as restore DEASSIGN support.  For more details, please read the patch
> headers.
>
> This series has been tested against the kvm-eventfd unit test, and
> appears to be functioning properly.  You can download this test here:
>
> ftp://ftp.novell.com/dev/ghaskins/kvm-eventfd.tar.bz2
>
> I've included version 4 of Davide's eventfd patch (ported to kvm.git) so
> that its a complete reviewable series.  Note, however, that there may be
> later versions of his patch to consider for merging, so we should
> coordinate with him.
>   

So I know we talked yesterday in the review session about whether it was
actually worth all this complexity to deal with the POLLHUP or if we
should just revert to the prior "two syscall" model and be done with
it.  Rusty reflected these same sentiments this morning in response to
Davide's patch in a different thread.

I am a bit torn myself, tbh.  I do feel as though I have a good handle
on the issue and that it is indeed now fixed (at least, if this series
is applied and the slow-work issue is fixed, still pending upstream
ACK).  I have a lot invested in going the POLLHUP direction having spent
so much time thinking about the problem and working on the patches, so I
a bit of a biased opinion, I know.

The reason why I am pushing this series out now is at least partly so we
can tie up these loose ends.  We have both solutions in front of us and
can make a decision either way.  At least the solution is formally
documented in the internet archives forever this way ;)

I took the review comments to heart that the shutdown code was
substantially larger and more complex than the actual fast-path code.  I
went though last night and simplified and clarified it.  I think the
latest result is leaner and clearer, so please give it another review
(particularly for races) before dismissing it.

Ultimately, I think the concept of a release notification for eventfd is
a good thing for all eventfd users, so I don't think this thing should
go away per se even if irqfd decides to not use it. 

-Greg

Attachment: signature.asc
Description: OpenPGP digital signature