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

[Davide Libenzi]

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


--
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/