LinuxLists.cc - [PATCH] async poll for 2.5

2002-10-14 22:41:40

Subject: [PATCH] async poll for 2.5

diff -ruN linux-2.5.41/fs/aio.c linux-2.5.41AIO/fs/aio.c
--- linux-2.5.41/fs/aio.c Mon Oct 7 11:24:13 2002
+++ linux-2.5.41AIO/fs/aio.c Mon Oct 14 12:27:05 2002
@@ -59,6 +59,8 @@
static spinlock_t fput_lock = SPIN_LOCK_UNLOCKED;
LIST_HEAD(fput_head);

+int async_poll(struct kiocb *iocb, int events);
+
/* aio_setup
* Creates the slab caches used by the aio routines, panic on
* failure as this is done early during the boot sequence.
@@ -893,6 +895,19 @@
return -EINVAL;
}

+ssize_t generic_aio_poll(struct file *file, struct kiocb *req, struct iocb *iocb)
+{
+ unsigned events = iocb->aio_buf;
+
+ /* Did the user set any bits they weren't supposed to? (The
+ * above is actually a cast.
+ */
+ if (unlikely(events != iocb->aio_buf))
+ return -EINVAL;
+
+ return async_poll(req, events);
+}
+
static int FASTCALL(io_submit_one(struct kioctx *ctx, struct iocb *user_iocb,
struct iocb *iocb));
static int io_submit_one(struct kioctx *ctx, struct iocb *user_iocb,
@@ -978,12 +993,15 @@
if (file->f_op->aio_fsync)
ret = file->f_op->aio_fsync(req, 0);
break;
+ case IOCB_CMD_POLL:
+ ret = generic_aio_poll(file, req, iocb);
+ break;
default:
dprintk("EINVAL: io_submit: no operation provided\n");
ret = -EINVAL;
}

- if (likely(EIOCBQUEUED == ret))
+ if (likely(-EIOCBQUEUED == ret))
return 0;
if (ret >= 0) {
aio_complete(req, ret, 0);
diff -ruN linux-2.5.41/fs/select.c linux-2.5.41AIO/fs/select.c
--- linux-2.5.41/fs/select.c Mon Oct 7 11:23:21 2002
+++ linux-2.5.41AIO/fs/select.c Mon Oct 14 13:39:58 2002
@@ -20,6 +20,8 @@
#include <linux/personality.h> /* for STICKY_TIMEOUTS */
#include <linux/file.h>
#include <linux/fs.h>
+#include <linux/aio.h>
+#include <linux/init.h>

#include <asm/uaccess.h>

@@ -27,19 +29,34 @@
#define DEFAULT_POLLMASK (POLLIN | POLLOUT | POLLRDNORM | POLLWRNORM)

struct poll_table_entry {
- struct file * filp;
wait_queue_t wait;
wait_queue_head_t * wait_address;
+ struct file * filp;
+ poll_table *p;
};

struct poll_table_page {
+ unsigned long size;
struct poll_table_page * next;
struct poll_table_entry * entry;
struct poll_table_entry entries[0];
};

#define POLL_TABLE_FULL(table) \
- ((unsigned long)((table)->entry+1) > PAGE_SIZE + (unsigned long)(table))
+ ((unsigned long)((table)->entry+1) > \
+ (table)->size + (unsigned long)(table))
+
+/* async poll uses only one entry per poll table as it is linked to an iocb */
+typedef struct async_poll_table_struct {
+ poll_table pt;
+ int events; /* event mask for async poll */
+ int wake;
+ long sync;
+ struct poll_table_page pt_page; /* one poll table page hdr */
+ struct poll_table_entry entries[1]; /* space for a single entry */
+} async_poll_table;
+
+static kmem_cache_t *async_poll_table_cache;

/*
* Ok, Peter made a complicated, but straightforward multiple_wait() function.
@@ -53,8 +70,7 @@
* as all select/poll functions have to call it to add an entry to the
* poll table.
*/
-
-void poll_freewait(poll_table* pt)
+void __poll_freewait(poll_table* pt, wait_queue_t *wait)
{
struct poll_table_page * p = pt->table;
while (p) {
@@ -62,15 +78,141 @@
struct poll_table_page *old;

entry = p->entry;
+ if (entry == p->entries) /* may happen with async poll */
+ break;
do {
entry--;
- remove_wait_queue(entry->wait_address,&entry->wait);
+ if (wait != &entry->wait)
+ remove_wait_queue(entry->wait_address,&entry->wait);
+ else
+ __remove_wait_queue(entry->wait_address,&entry->wait);
fput(entry->filp);
} while (entry > p->entries);
old = p;
p = p->next;
- free_page((unsigned long) old);
+ if (old->size == PAGE_SIZE)
+ free_page((unsigned long) old);
}
+ if (pt->iocb)
+ kmem_cache_free(async_poll_table_cache, pt);
+}
+
+void poll_freewait(poll_table* pt)
+{
+ __poll_freewait(pt, NULL);
+}
+
+void async_poll_complete(void *data)
+{
+ async_poll_table *pasync = data;
+ poll_table *p = data;
+ struct kiocb *iocb = p->iocb;
+ unsigned int mask;
+
+ pasync->wake = 0;
+ wmb();
+ do {
+ mask = iocb->ki_filp->f_op->poll(iocb->ki_filp, p);
+ mask &= pasync->events | POLLERR | POLLHUP;
+ if (mask) {
+ poll_table *p2 = xchg(&iocb->ki_data, NULL);
+ if (p2) {
+ poll_freewait(p2);
+ aio_complete(iocb, mask, 0);
+ }
+ return;
+ }
+ pasync->sync = 0;
+ wmb();
+ } while (pasync->wake);
+}
+
+static int async_poll_waiter(wait_queue_t *wait, unsigned mode, int sync)
+{
+ struct poll_table_entry *entry = (struct poll_table_entry *)wait;
+ async_poll_table *pasync = (async_poll_table *)(entry->p);
+ struct kiocb *iocb;
+ unsigned int mask;
+
+ iocb = pasync->pt.iocb;
+ mask = iocb->ki_filp->f_op->poll(iocb->ki_filp, NULL);
+ mask &= pasync->events | POLLERR | POLLHUP;
+ if (mask) {
+ poll_table *p2 = xchg(&iocb->ki_data, NULL);
+ if (p2) {
+ __poll_freewait(p2, wait);
+ aio_complete(iocb, mask, 0);
+ return 1;
+ }
+ }
+ return 0;
+}
+
+int async_poll_cancel(struct kiocb *iocb, struct io_event *res)
+{
+ poll_table *p;
+
+ /* FIXME: almost right */
+ p = xchg(&iocb->ki_data, NULL);
+ if (p) {
+ poll_freewait(p);
+ aio_complete(iocb, 0, 0);
+ aio_put_req(iocb);
+ return 0;
+ }
+ aio_put_req(iocb);
+ return -EAGAIN;
+}
+
+int async_poll(struct kiocb *iocb, int events)
+{
+ unsigned int mask;
+ async_poll_table *pasync;
+ poll_table *p;
+
+ /* Fast path */
+ if (iocb->ki_filp->f_op && iocb->ki_filp->f_op->poll) {
+ mask = iocb->ki_filp->f_op->poll(iocb->ki_filp, NULL);
+ mask &= events | POLLERR | POLLHUP;
+ if (mask & events)
+ return events;
+ }
+
+ pasync = kmem_cache_alloc(async_poll_table_cache, SLAB_KERNEL);
+ if (!pasync)
+ return -ENOMEM;
+
+ p = (poll_table *)pasync;
+ poll_initwait(p);
+ p->iocb = iocb;
+ pasync->wake = 0;
+ pasync->sync = 0;
+ pasync->events = events;
+ pasync->pt_page.entry = pasync->pt_page.entries;
+ pasync->pt_page.size = sizeof(pasync->pt_page) + sizeof(pasync->entries);
+ pasync->pt_page.next = 0;
+ p->table = &pasync->pt_page;
+
+ iocb->ki_data = p;
+ wmb();
+ iocb->ki_cancel = async_poll_cancel;
+
+ mask = DEFAULT_POLLMASK;
+#warning broken
+ iocb->ki_users ++;
+ if (iocb->ki_filp->f_op && iocb->ki_filp->f_op->poll)
+ mask = iocb->ki_filp->f_op->poll(iocb->ki_filp, p);
+ mask &= events | POLLERR | POLLHUP;
+ if (mask && !test_and_set_bit(0, &pasync->sync))
+ aio_complete(iocb, mask, 0);
+
+ if (aio_put_req(iocb))
+ /* Must be freed after aio_complete to synchronise with
+ * cancellation of the request.
+ */
+ poll_freewait(p);
+
+ return -EIOCBQUEUED;
}

void __pollwait(struct file * filp, wait_queue_head_t * wait_address, poll_table *p)
@@ -86,6 +228,7 @@
__set_current_state(TASK_RUNNING);
return;
}
+ new_table->size = PAGE_SIZE;
new_table->entry = new_table->entries;
new_table->next = table;
p->table = new_table;
@@ -99,7 +242,11 @@
get_file(filp);
entry->filp = filp;
entry->wait_address = wait_address;
- init_waitqueue_entry(&entry->wait, current);
+ entry->p = p;
+ if (p->iocb) /* async poll */
+ init_waitqueue_func_entry(&entry->wait, async_poll_waiter);
+ else
+ init_waitqueue_entry(&entry->wait, current);
add_wait_queue(wait_address,&entry->wait);
}
}
@@ -495,3 +642,14 @@
poll_freewait(&table);
return err;
}
+
+static int __init async_poll_init(void)
+{
+ async_poll_table_cache = kmem_cache_create("async poll table",
+ sizeof(async_poll_table), 0, 0, NULL, NULL);
+ if (!async_poll_table_cache)
+ panic("unable to alloc poll_table_cache");
+ return 0;
+}
+
+module_init(async_poll_init);
diff -ruN linux-2.5.41/include/linux/aio_abi.h linux-2.5.41AIO/include/linux/aio_abi.h
--- linux-2.5.41/include/linux/aio_abi.h Mon Oct 7 11:23:28 2002
+++ linux-2.5.41AIO/include/linux/aio_abi.h Mon Oct 7 16:33:36 2002
@@ -40,6 +40,7 @@
* IOCB_CMD_PREADX = 4,
* IOCB_CMD_POLL = 5,
*/
+ IOCB_CMD_POLL = 5,
IOCB_CMD_NOOP = 6,
};

diff -ruN linux-2.5.41/include/linux/poll.h linux-2.5.41AIO/include/linux/poll.h
--- linux-2.5.41/include/linux/poll.h Mon Oct 7 11:24:12 2002
+++ linux-2.5.41AIO/include/linux/poll.h Tue Oct 8 12:06:44 2002
@@ -9,12 +9,14 @@
#include <linux/string.h>
#include <linux/mm.h>
#include <asm/uaccess.h>
+#include <linux/workqueue.h>

struct poll_table_page;

typedef struct poll_table_struct {
- int error;
- struct poll_table_page * table;
+ int error;
+ struct poll_table_page *table;
+ struct kiocb *iocb; /* iocb for async poll */
} poll_table;

extern void __pollwait(struct file * filp, wait_queue_head_t * wait_address, poll_table *p);
@@ -29,6 +31,7 @@
{
pt->error = 0;
pt->table = NULL;
+ pt->iocb = NULL;
}
extern void poll_freewait(poll_table* pt);

Attachments:

aiopoll-2.5.41-5.patch (8.49 kB)

2002-10-14 22:48:55

by Charlie Krasic

[permalink] [raw]

Subject: Re: [PATCH] async poll for 2.5

John Gardiner Myers <[email protected]> writes:

> The epoll API is deficient--it is subtly error prone and it forces
> work on user space that is better done in the kernel. That the API
> is specified in a deficient way does not make it any less deficient.

You can argue that any API is subtly error prone. The whole sockets
API is that way. That's why the W. Richard Stevens network
programming books are such gems. That's why having access to kernel
source is invaluable. You have to pay attention to details to avoid
errors.

With /dev/epoll, it is perfectly feasible to write user level
wrapper libraries that help avoid the potential pitfalls.

I think it was Dan Kegel who has already mentioned one.

I've written one myself, and I'm very confident in it. I've written a
traffic generator application on top of my library that stresses the
Linux kernel protocol stack to the extreme. It generates the
proverbial 10k cps, saturates gigabit networks, etc.

It has no problem running over /dev/epoll.

IMHO, the code inside my wrapper library for the epoll case is
significantly easier to understand than the code for the case that
uses the legacy poll() interface.

If /dev/epoll were so error prone as you say it is, I think I would
have noticed it.

-- Buck

ps If anybody cares. I can give them a pointer to my code.

> Again, there is no rational justification for the kernel to not test
> the condition upon registration. There is ample justification for
> it to do so.

2002-10-16 14:11:47

by Davide Libenzi

[permalink] [raw]

Subject: Re: [PATCH] async poll for 2.5

On Tue, 15 Oct 2002, John Myers wrote:

> Davide Libenzi wrote:
>
> >Just a simple question : Have you ever used RT-Signal API ? Is it the API
> >"deficent" [...] ?
> >
> No. Yes. The (fixed) size of the signal queue is far too small. One
> either gets catastrophic failure on overload or one has to pay to do
> redundant accounting of interest.
>
> >Do you know the
> >difference between level triggered ( poll() - select() - /dev/poll ) and
> >edge triggered ( /dev/epoll - RT-Signal ) interfaces ?
> >
> >
> Yes. The registration of interest can itself be considered an edge
> condition.

I knew you were going there, aka you do not understand how edge triggered
API have to be used. Even if the API will drop an event at registration
time you still cannot use this code scheme :

int my_io(...) {

if (event_wait(...))
do_io(...);

}

You CAN NOT. And look, it is not an API problem, it's your problem that
you want to use the API like a poll()-like API. The code scheme for an
edge triggered API is :

int my_io(...) {

while (do_io(...) == EAGAIN)
event_wait(...);

}

This because you have to consume the I/O space to push the level to 0 so
that a transaction 0->1 can happen and you can happily receive your
events.

- Davide

2002-10-16 14:14:45

by Davide Libenzi

On Wed, 2002-10-16 at 19:29, John Gardiner Myers wrote:
> Better to fix the API. The kernel has more information than user space
> and can do a better job. In the kernel, the problem can be fixed once
> and for all, not over and over again in each different wrapper library.
> It's not even as if the change would break programs correctly written
> to the old API, not that we particularly care about programs written to
> the old API.

I think a chunk of the poll scaling problem is better addressed by
futexes. If I can say "this futex list for this fd for events X Y and Z"
I can construct almost all the efficient stuff I need out of the futex
interfaces, much like doing it with SIGIO setting flags but a lot less
clocks

Alan