Re: [patch] eventfd - revised interface and cleanups (2nd rev)

From: Andrew Morton
Date: Tue Jun 23 2009 - 15:48:42 EST


On Tue, 23 Jun 2009 12:25:36 -0700 (PDT)
Davide Libenzi <davidel@xxxxxxxxxxxxxxx> wrote:

> The following patch changes the eventfd interface to de-couple the eventfd
> memory context, from the file pointer instance.
> Without such change, there is no clean way to racely free handle the
> POLLHUP event sent when the last instance of the file* goes away.
> Also, now the internal eventfd APIs are using the eventfd context instead
> of the file*.
> Another cleanup this patch does, is making AIO select EVENTFD, instead of
> adding a bunch of empty function stubs inside eventfd.h in order to
> handle the (AIO && !EVENTFD) case.
>

If we really want to squeeze this into 2.6.31 then it would be helpful
to have justification in the changelog, please. I see that some KVM
feature needs it, but what and why and why now, etc?

The patch conflicts with similar changes int he KVM tree, but that'll
just ruin sfr's life for a day.


Your patch has:


> ...
> static int eventfd_release(struct inode *inode, struct file *file)
> {
> - kfree(file->private_data);
> + struct eventfd_ctx *ctx = file->private_data;
> +
> + wake_up_poll(&ctx->wqh, POLLHUP);
> + eventfd_ctx_put(ctx);
> return 0;
> }

but the patch in linux-next has

static int eventfd_release(struct inode *inode, struct file *file)
{
struct eventfd_ctx *ctx = file->private_data;

/*
* No need to hold the lock here, since we are on the file cleanup
* path and the ones still attached to the wait queue will be
* serialized by wake_up_locked_poll().
*/
wake_up_locked_poll(&ctx->wqh, POLLHUP);
kfree(ctx);
return 0;
}

which looks quite different (and has a nice comment!)

What's going on here?
--
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/