In tests with multiple concurrent waiters on edge-triggered epoll
instances where an emitter writes to multiple sockets (epoll16,
epoll56, epoll58):
When the emitter performs its first write(), ep_poll_callback() fires
and wakes up both waiters because one waiter uses epoll_wait() and the
other one uses poll(). This translates to different wait queues,
ep->wq for epoll and ep->poll_wait for poll/select, which are both
awoken by the kernel because of that single write. Next, both waiter
threads invoke epoll_wait(), but since there is only one event, only
one epoll_wait() will return non-zero because of the edge-triggered
mode being used (in level-triggered mode, the kernel would re-queue
the event because of remaining unread data).
Since the second waiter sees an empty ready list, it does not
increment ctx.count and the test fails spuriously with ctx.count == 1
instead of 2.
Emitter (CPU 0) Thread 0 (CPU 1) Thread 1 (CPU 2)
=============== ================ ================
epoll_wait(e0, -1) poll(e0, -1)
[on e0->wq] [on e0->poll_wait]
write(sfd[1])
|
+--(Kernel wakes BOTH e0->wq and e0->poll_wait via callback)--+
| |
| wakes up wakes up |
| epoll_wait() reaps e1 poll() returns 1 |
| (e1 removed via ET) (wants event) |
| e0->rdllist is EMPTY | |
| count++ (count = 1) v |
| epoll_wait(e0, 0) |
| sees EMPTY list! |
| returns 0! |
| thread exits |
v |
write(sfd[3]) |
(event arrives too late!) v
EXPECT_EQ(count, 2) <-- SPURIOUS FAILURE!
Introduce waiter_entry1ap_loop() to retry poll() if the initial
epoll_wait(..., 0) yielded no events. This ensures the thread waits for
the subsequent write rather than failing immediately. Apply this helper in
epoll16, epoll56, and for both waiter threads in epoll58.
Fixes: f2728fe80cef ("selftests: add epoll selftests")
Signed-off-by: Florian Schmaus <[email protected]>
---
Changes in v2:
- do not modify level-triggered epoll15 as it is not affected by the race
- Link to v1:
https://patch.msgid.link/[email protected]
To: Shuah Khan <[email protected]>
To: Heiher <[email protected]>
To: Roman Penyaev <[email protected]>
To: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
.../filesystems/epoll/epoll_wakeup_test.c | 32 +++++++++++++++-------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c
b/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c
index 81a994943e12..b4dcbcd79773 100644
--- a/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c
+++ b/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c
@@ -74,6 +74,24 @@ static void *waiter_entry1ap(void *data)
return NULL;
}
+static void *waiter_entry1ap_loop(void *data)
+{
+ struct pollfd pfd;
+ struct epoll_event e;
+ struct epoll_mtcontext *ctx = data;
+
+ pfd.fd = ctx->efd[0];
+ pfd.events = POLLIN;
+ while (poll(&pfd, 1, 2000) > 0) {
+ if (epoll_wait(ctx->efd[0], &e, 1, 0) > 0) {
+ __sync_fetch_and_add(&ctx->count, 1);
+ break;
+ }
+ }
+
+ return NULL;
+}
+
static void *waiter_entry1o(void *data)
{
struct epoll_event e;
@@ -809,7 +827,7 @@ TEST(epoll16)
ASSERT_EQ(epoll_ctl(ctx.efd[0], EPOLL_CTL_ADD, ctx.sfd[2], events), 0);
ctx.main = pthread_self();
- ASSERT_EQ(pthread_create(&ctx.waiter, NULL, waiter_entry1ap, &ctx), 0);
+ ASSERT_EQ(pthread_create(&ctx.waiter, NULL, waiter_entry1ap_loop,
&ctx), 0);
ASSERT_EQ(pthread_create(&emitter, NULL, emitter_entry2, &ctx), 0);
if (epoll_wait(ctx.efd[0], events, 1, -1) > 0)
@@ -2925,7 +2943,7 @@ TEST(epoll56)
ASSERT_EQ(epoll_ctl(ctx.efd[0], EPOLL_CTL_ADD, ctx.efd[2], &e), 0);
ctx.main = pthread_self();
- ASSERT_EQ(pthread_create(&ctx.waiter, NULL, waiter_entry1ap, &ctx), 0);
+ ASSERT_EQ(pthread_create(&ctx.waiter, NULL, waiter_entry1ap_loop,
&ctx), 0);
ASSERT_EQ(pthread_create(&emitter, NULL, emitter_entry2, &ctx), 0);
if (epoll_wait(ctx.efd[0], &e, 1, -1) > 0)
@@ -3030,7 +3048,6 @@ TEST(epoll57)
TEST(epoll58)
{
pthread_t emitter;
- struct pollfd pfd;
struct epoll_event e;
struct epoll_mtcontext ctx = { 0 };
@@ -3061,15 +3078,10 @@ TEST(epoll58)
ASSERT_EQ(epoll_ctl(ctx.efd[0], EPOLL_CTL_ADD, ctx.efd[2], &e), 0);
ctx.main = pthread_self();
- ASSERT_EQ(pthread_create(&ctx.waiter, NULL, waiter_entry1ap, &ctx), 0);
+ ASSERT_EQ(pthread_create(&ctx.waiter, NULL, waiter_entry1ap_loop,
&ctx), 0);
ASSERT_EQ(pthread_create(&emitter, NULL, emitter_entry2, &ctx), 0);
- pfd.fd = ctx.efd[0];
- pfd.events = POLLIN;
- if (poll(&pfd, 1, -1) > 0) {
- if (epoll_wait(ctx.efd[0], &e, 1, 0) > 0)
- __sync_fetch_and_add(&ctx.count, 1);
- }
+ waiter_entry1ap_loop(&ctx);
ASSERT_EQ(pthread_join(ctx.waiter, NULL), 0);
EXPECT_EQ(ctx.count, 2);
---
base-commit: 1b78070aaef63512688aebfbc82365ef9d6660f1
change-id: 20260828-selftest-epoll-fix-race-6f5fbe3c22e7
Best regards,
--
Florian Schmaus <[email protected]>
