Re: [PATCH 3/3] eventfd: add internal reference counting to fix notifier race conditions

Davide Libenzi
2009-06-22T18:09:32Z

On Mon, 22 Jun 2009, Gregory Haskins wrote:

> I am probably confused or perhaps have the wrong terminology, but isnt
> that "ok". I am concerned about the consumer (the guy getting the
> POLLINs) to be able to detect POLLHUP when the last producer
> (f_ops->write() from userspace, eventfd_signal() from kernel) goes away.
>
> Consider the following sequence:
>
> -------------------
>
> userspace calls "fd = eventfd()", and gives one to KVM as an irqfd, and
> the other to some PCI-passthrough device.
>
> The kvm/irqfd side acquires a kref, the pci side acquires a file. At
> this moment, userspace has the fd, and the pci device has the file (for
> eventfd_signal()). The fget() count is 2. Userspace closes the fd
> because its done with it, and the count drops to 1.
>
> Some time later, pci does an fput(), and KVM sees the POLLHUP and cleans up.
>
> -------------------
>
> In this new model, the POLLHUP would have gone out as soon as userspace
> closed the fd, even though the intended producer (the PCI device) and
> the consumer (the KVM guest) are still up and running. This doesnt seem
> right to me. Or am I missing something?

What you're doing there, is setting up a kernel-to-kernel (since
userspace only role is to create the eventfd) communication, using a file*
as accessory. That IMO is plain wrong.
If userspace is either the producer, or the consumer, and you need to
handle userspace leaving the building, you need to:

file = eventfd_fget(fd);
ctx = eventfd_ctx_get(file); /* Eventually, if producer */
eventfd_pollcb_register(file, ...);
fput(file);

In your case of kernel-to-kernel scenario, why would you need eventfd at
all, if userspace role in that model is simply to create it?
There are more effective ways to have in kernel communication channels,
than resorting to userspace link facilities like eventfd.

- Davide