On 10/30/12 11:32 PM, Michael Wang wrote:
On 10/26/2012 08:08 AM, Paton J. Lewis wrote:
quoted
From: "Paton J. Lewis" <redacted>
It is not currently possible to reliably delete epoll items when using the
same epoll set from multiple threads. After calling epoll_ctl with
EPOLL_CTL_DEL, another thread might still be executing code related to an
event for that epoll item (in response to epoll_wait). Therefore the deleting
thread does not know when it is safe to delete resources pertaining to the
associated epoll item because another thread might be using those resources.
The deleting thread could wait an arbitrary amount of time after calling
epoll_ctl with EPOLL_CTL_DEL and before deleting the item, but this is
inefficient and could result in the destruction of resources before another
thread is done handling an event returned by epoll_wait.
This patch enhances epoll_ctl to support EPOLL_CTL_DISABLE, which disables an
epoll item. If epoll_ctl returns -EBUSY in this case, then another thread may
handling a return from epoll_wait for this item. Otherwise if epoll_ctl
returns 0, then it is safe to delete the epoll item. This allows multiple
threads to use a mutex to determine when it is safe to delete an epoll item
and its associated resources, which allows epoll items to be deleted both
efficiently and without error in a multi-threaded environment. Note that
EPOLL_CTL_DISABLE is only useful in conjunction with EPOLLONESHOT, and using
EPOLL_CTL_DISABLE on an epoll item without EPOLLONESHOT returns -EINVAL.
This patch also adds a new test_epoll self-test program to both demonstrate
the need for this feature and test it.
Hi, Paton
I'm just think about may be we could use this way.
Seems like currently we are depending on the epoll_ctl() to indicate the
start point of safe section and epoll_wait() for the end point, like:
while () {
epoll_wait() --------------
fd event arrived safe section
35 more quoted lines
The pseudecode you provide below (for "DISABLE") seems to indicate that this "epi ref count" must be maintained by the kernel. Therefore any userspace modification of a ref count associated with an epoll item will require a new or changed kernel API.
send event
And what DISABLE do is:
set epi stop flag
if epi ref count is not 0
wait until ref count be 0
Perhaps I don't fully understand what you're proposing, but I don't think it's reasonable for a kernel API (epoll_ctl in this case) to block while waiting for a userspace action (decrementing the ref count) that might never occur.
Andrew Morton also proposed using ref counting in response to my initial patch submission; my reply to his proposal might also be applicable to your proposal. A link to that discussion thread: http://thread.gmane.org/gmane.linux.kernel/1311457/focus=1315096
Sorry if I am misunderstanding your proposal, but I don't see how it solves the original problem.
Pat
So after DISABLE return, we can safely delete any thing related to that epi.
One thing is that the user should not change the events info returned by
epoll_wait().
It's just a propose, but if it works, there will be no limit on ONESHOT
any more ;-)
Regards,
Michael Wang
quoted
Signed-off-by: Paton J. Lewis <redacted>
---
fs/eventpoll.c | 40 ++-
include/linux/eventpoll.h | 1 +
tools/testing/selftests/Makefile | 2 +-
tools/testing/selftests/epoll/Makefile | 11 +
tools/testing/selftests/epoll/test_epoll.c | 364 ++++++++++++++++++++++++++++
5 files changed, 414 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/epoll/Makefile
create mode 100644 tools/testing/selftests/epoll/test_epoll.c
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 739b098..c718afd 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -339,7 +339,7 @@ static inline struct epitem *ep_item_from_epqueue(poll_table *p)
/* Tells if the epoll_ctl(2) operation needs an event copy from userspace */
static inline int ep_op_has_event(int op)
{
- return op != EPOLL_CTL_DEL;
+ return op == EPOLL_CTL_ADD || op == EPOLL_CTL_MOD;
}
/* Initialize the poll safe wake up structure */@@ -664,6 +664,36 @@ static int ep_remove(struct eventpoll *ep, struct epitem *epi)
return 0;
}
468 more quoted lines