2002-11-07 22:46:58

by Jay Vosburgh

[permalink] [raw]
Subject: [PATCH] 2.5.46: epoll ep_insert doesn't wake waiters if events exist


The logic in fs/eventpoll.c:ep_insert() checks to see if events are
already present when processing an EP_CTL_ADD operation, and, if any are,
adds them to the list. It does not wake up any waiters, so, e.g., another
thread waiting in epoll_wait() will not be awakened for these events. The
fix is to have ep_insert() do wakeups, just as ep_poll_callback() does when
it adds events.

-J

diff -ur linux-2.5.46.orig/fs/eventpoll.c linux-2.5.46/fs/eventpoll.c
--- linux-2.5.46.orig/fs/eventpoll.c Thu Nov 7 14:31:21 2002
+++ linux-2.5.46/fs/eventpoll.c Thu Nov 7 14:32:16 2002
@@ -838,8 +838,13 @@
list_add(&dpi->llink, ep_hash_entry(ep, ep_hash_index(ep, tfile)));

/* If the file is already "ready" we drop it inside the ready list */
- if ((revents & pfd->events) && !EP_IS_LINKED(&dpi->rdllink))
+ if ((revents & pfd->events) && !EP_IS_LINKED(&dpi->rdllink)) {
list_add(&dpi->rdllink, &ep->rdllist);
+ if (waitqueue_active(&ep->wq))
+ wake_up(&ep->wq);
+ if (waitqueue_active(&ep->poll_wait))
+ wake_up(&ep->poll_wait);
+ }

write_unlock_irqrestore(&ep->lock, flags);



(See attached file: epoll-2.5.46-insert.patch)


Attachments:
epoll-2.5.46-insert.patch (703.00 B)

2002-11-07 23:08:01

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] 2.5.46: epoll ep_insert doesn't wake waiters if events exist

On Thu, 7 Nov 2002, Jay Vosburgh wrote:

>
> The logic in fs/eventpoll.c:ep_insert() checks to see if events are
> already present when processing an EP_CTL_ADD operation, and, if any are,
> adds them to the list. It does not wake up any waiters, so, e.g., another
> thread waiting in epoll_wait() will not be awakened for these events. The
> fix is to have ep_insert() do wakeups, just as ep_poll_callback() does when
> it adds events.

Matthew Kirk already made me notice this yesterday and it's in my code ( 0.37 ).
I was ashame to make another post to Linus :) so I delayed the post.
Linus, I have a few bits. Let me know if you want them now or we will
delay to 2.5.47 ...



- Davide


2002-11-08 15:43:46

by Mark Hamblin

[permalink] [raw]
Subject: Re: [PATCH] 2.5.46: epoll ep_insert doesn't wake waiters if events exist

Looking at the documentation at
http://www.xmailserver.org/linux-patches/epoll_wait.txt, I noticed that the
description for epoll_wait states: "On success, epoll_wait(2) returns the
number of file descriptors ready for the requested I/O." My question is if
I have, for example, 1000 sockets registered with epoll and 100 of them have
received data and I call epoll_wait with max_events set to 10, will
epoll_wait return 10 or 100? Furthermore, does the edge-triggered nature of
epoll "eat the edge" for the other 90 sockets even though they didn't get
returned? Finally, if those 10 sockets get more data before I call
epoll_wait with max_events = 10 again, will those same 10 sockets get
returned?

- Mark


2002-11-08 17:27:04

by Davide Libenzi

[permalink] [raw]
Subject: Re: [PATCH] 2.5.46: epoll ep_insert doesn't wake waiters if events exist

On Fri, 8 Nov 2002, Mark Hamblin wrote:

> Looking at the documentation at
> http://www.xmailserver.org/linux-patches/epoll_wait.txt, I noticed that the
> description for epoll_wait states: "On success, epoll_wait(2) returns the
> number of file descriptors ready for the requested I/O." My question is if
> I have, for example, 1000 sockets registered with epoll and 100 of them have
> received data and I call epoll_wait with max_events set to 10, will
> epoll_wait return 10 or 100? Furthermore, does the edge-triggered nature of
> epoll "eat the edge" for the other 90 sockets even though they didn't get
> returned? Finally, if those 10 sockets get more data before I call
> epoll_wait with max_events = 10 again, will those same 10 sockets get
> returned?

epoll will return 10 and the remaining 90 will remain available for future
epoll_wait() calls. The last question will trigger a change in my code,
from list_add() to list_add_tail(). Since I don't want to stress Linus
with 24590 patches, I'll wait for 2.5.47 to merge the new code ...



- Davide