eventpoll: Fix epoll_wait() report false negative

ep_events_available() checks for available events by looking at ep->rdllist and ep->ovflist. However, this is done without a lock, therefore the returned value is not reliable. Because it is possible that both checks on ep->rdllist and ep->ovflist are false while ep_start_scan() or ep_done_scan() is being executed on other CPUs, despite events are available.

This bug can be observed by:

1. Create an eventpoll with at least one ready level-triggered event

2. Create multiple threads who do epoll_wait() with zero timeout. The threads do not consume the events, therefore all epoll_wait() should return at least one event.

If one thread is executing ep_events_available() while another thread is executing ep_start_scan() or ep_done_scan(), epoll_wait() may wrongly return no event for the former thread.

This reproducer is implemented as TEST(epoll65) in tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c

Fix it by skipping ep_events_available(), just call ep_try_send_events() directly.

epoll_sendevents() (io_uring) suffers the same problem, fix that as well.

There is still ep_busy_loop() who uses ep_events_available() without lock, but it is probably okay (?) for busy-polling.

Fixes c5a282e9635e ("fs/epoll: reduce the scope of wq lock in epoll_wait()")

Fixes e59d3c64cba6 ("epoll: eliminate unnecessary lock for zero timeout")

Fixes ae3a4f1fdc2c ("eventpoll: add epoll_sendevents() helper")

Signed-off-by Nam Cao <[email protected]>

Cc [email protected]

---
 fs/eventpoll.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

fs/eventpoll.c +2 -15

index 0fbf5dfedb24..541481eafc20 100644
@@ -2022,7 +2022,7 @@ static int ep_schedule_timeout(ktime_t *to)
 static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
                   int maxevents, struct timespec64 *timeout)
 {
-       int res, eavail, timed_out = 0;
+       int res, eavail = 1, timed_out = 0;
        u64 slack = 0;
        wait_queue_entry_t wait;
        ktime_t expires, *to = NULL;

@@ -2041,16 +2041,6 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
                timed_out = 1;
        }
 
-       /*
-        * This call is racy: We may or may not see events that are being added
-        * to the ready list under the lock (e.g., in IRQ callbacks). For cases
-        * with a non-zero timeout, this thread will check the ready list under
-        * lock and will add to the wait queue.  For cases with a zero
-        * timeout, the user by definition should not care and will have to
-        * recheck again.
-        */
-       eavail = ep_events_available(ep);
-
        while (1) {
                if (eavail) {
                        res = ep_try_send_events(ep, events, maxevents);

@@ -2496,9 +2486,7 @@ int epoll_sendevents(struct file *file, struct epoll_event __user *events,
         * Racy call, but that's ok - it should get retried based on
         * poll readiness anyway.
         */
-       if (ep_events_available(ep))
-               return ep_try_send_events(ep, events, maxevents);
-       return 0;
+       return ep_try_send_events(ep, events, maxevents);
 }
 
 /*
--

2.39.5

Quoted 12 lines · Nam Cao

ep_events_available() checks for available events by looking at ep->rdllist and ep->ovflist. However, this is done without a lock, therefore the returned value is not reliable. Because it is possible that both checks on ep->rdllist and ep->ovflist are false while ep_start_scan() or ep_done_scan() is being executed on other CPUs, despite events are available.

This bug can be observed by:

1. Create an eventpoll with at least one ready level-triggered event

Quoted 7 lines

2. Create multiple threads who do epoll_wait() with zero timeout. The threads do not consume the events, therefore all epoll_wait() should return at least one event.

If one thread is executing ep_events_available() while another thread is executing ep_start_scan() or ep_done_scan(), epoll_wait() may wrongly return no event for the former thread.

That is the whole point of epoll_wait with a zero timeout. We would want to opportunistically poll without much overhead, which will have more false positives.
A caller that calls with a zero timeout should retry later, and will at some point observe the event.

I'm not sure if we would want to add much more overheads, for higher precision.

Quoted 21 lines · Soheil Hassas Yeganeh

Quoted 19 lines · Nam Cao

ep_events_available() checks for available events by looking at ep->rdllist and ep->ovflist. However, this is done without a lock, therefore the returned value is not reliable. Because it is possible that both checks on ep->rdllist and ep->ovflist are false while ep_start_scan() or ep_done_scan() is being executed on other CPUs, despite events are available.

This bug can be observed by:

1. Create an eventpoll with at least one ready level-triggered event

2. Create multiple threads who do epoll_wait() with zero timeout. The threads do not consume the events, therefore all epoll_wait() should return at least one event.

If one thread is executing ep_events_available() while another thread is executing ep_start_scan() or ep_done_scan(), epoll_wait() may wrongly return no event for the former thread.

Quoted 5 lines

That is the whole point of epoll_wait with a zero timeout. We would want to opportunistically poll without much overhead, which will have more false positives.
A caller that calls with a zero timeout should retry later, and will at some point observe the event.

Is this a documented behavior that users expect? I do not see this in the man page.

The selftests rely on this behavior that timeout=0 sees events from a concurrently running producer. They would fail at a very higher rate after this change - believe me I had a similar patch that changed something in this area.

I would explore the seqcount that Mateusz suggested tbh.

Quoted 4 lines · Christian Brauner

The selftests rely on this behavior that timeout=0 sees events from a concurrently running producer. They would fail at a very higher rate after this change - believe me I had a similar patch that changed something in this area.

Huh, that's interesting. Do you still remember which selftest cases rely on this behavior? I would like to study them further.

Quoted 1 line

I would explore the seqcount that Mateusz suggested tbh.

I will investigate that.

Strictly speaking more error prone than the seq approach, but should be faster on weaker-ordered archs thanks to avoided fences.

I'm definitely not going to protest the seqc route.

Quoted 4 lines · Mateusz Guzik

Strictly speaking more error prone than the seq approach, but should be faster on weaker-ordered archs thanks to avoided fences.

I'm definitely not going to protest the seqc route.

Linus probably wouldn't be thrilled if I break epoll again, so let's stay with the simpler seqcount route.

Nam

fs/eventpoll.c +25 -2

index a3090b446af1..22c3f0186476 100644
@@ -38,6 +38,7 @@
 #include <linux/compat.h>
 #include <linux/rculist.h>
 #include <linux/capability.h>
+#include <linux/seqlock.h>
 #include <net/busy_poll.h>
 
 /*

@@ -190,6 +191,9 @@ struct eventpoll {
        /* Lock which protects rdllist and ovflist */
        spinlock_t lock;
 
+       /* Protect switching between rdllist and ovflist */
+       seqcount_spinlock_t seq;
+
        /* RB tree root used to store monitored fd structs */
        struct rb_root_cached rbr;
 

@@ -382,8 +386,17 @@ static inline struct epitem *ep_item_from_wait(wait_queue_entry_t *p)
  */
 static inline int ep_events_available(struct eventpoll *ep)
 {
-       return !list_empty_careful(&ep->rdllist) ||
-               READ_ONCE(ep->ovflist) != EP_UNACTIVE_PTR;
+       bool events_available;
+       unsigned int seq;
+
+       do {
+               seq = read_seqcount_begin(&ep->seq);
+
+               events_available = !list_empty_careful(&ep->rdllist) ||
+                                  READ_ONCE(ep->ovflist) != EP_UNACTIVE_PTR;
+       } while (read_seqcount_retry(&ep->seq, seq));
+
+       return events_available;
 }
 
 #ifdef CONFIG_NET_RX_BUSY_POLL

@@ -735,8 +748,12 @@ static void ep_start_scan(struct eventpoll *ep, struct list_head *txlist)
         */
        lockdep_assert_irqs_enabled();
        spin_lock_irq(&ep->lock);
+       write_seqcount_begin(&ep->seq);
+
        list_splice_init(&ep->rdllist, txlist);
        WRITE_ONCE(ep->ovflist, NULL);
+
+       write_seqcount_end(&ep->seq);
        spin_unlock_irq(&ep->lock);
 }
 

@@ -768,6 +785,9 @@ static void ep_done_scan(struct eventpoll *ep,
                        ep_pm_stay_awake(epi);
                }
        }
+
+       write_seqcount_begin(&ep->seq);
+
        /*
         * We need to set back ep->ovflist to EP_UNACTIVE_PTR, so that after
         * releasing the lock, events will be queued in the normal way inside

@@ -779,6 +799,9 @@ static void ep_done_scan(struct eventpoll *ep,
         * Quickly re-inject items left on "txlist".
         */
        list_splice(txlist, &ep->rdllist);
+
+       write_seqcount_end(&ep->seq);
+
        __pm_relax(ep->ws);
 
        if (!list_empty(&ep->rdllist)) {

Apologies for late reply.

The diff reads ok to me, but I would consider not looping in case of seq mismatch.

So does it solve the problem?

I think a somewhat of a blocker here would be to bench the thing -- I would expect some slowdown compared to stock kernel but should be still be faster than the previously proposed patch.

Quoted 28 lines · Nam Cao

Quoted 26 lines · Soheil Hassas Yeganeh

Quoted 12 lines · Nam Cao

ep_events_available() checks for available events by looking at ep->rdllist and ep->ovflist. However, this is done without a lock, therefore the returned value is not reliable. Because it is possible that both checks on ep->rdllist and ep->ovflist are false while ep_start_scan() or ep_done_scan() is being executed on other CPUs, despite events are available.

This bug can be observed by:

1. Create an eventpoll with at least one ready level-triggered event

Quoted 7 lines

2. Create multiple threads who do epoll_wait() with zero timeout. The threads do not consume the events, therefore all epoll_wait() should return at least one event.

If one thread is executing ep_events_available() while another thread is executing ep_start_scan() or ep_done_scan(), epoll_wait() may wrongly return no event for the former thread.

That is the whole point of epoll_wait with a zero timeout. We would want to opportunistically poll without much overhead, which will have more false positives.
A caller that calls with a zero timeout should retry later, and will at some point observe the event.

Quoted 2 lines

Is this a documented behavior that users expect? I do not see this in the man page.

The selftests rely on this behavior that timeout=0 sees events from a concurrently running producer. They would fail at a very higher rate after this change - believe me I had a similar patch that changed something in this area. I would explore the seqcount that Mateusz suggested tbh.

ep_events_available() checks for available events by looking at ep->rdllist and ep->ovflist. However, this is done without a lock, therefore the returned value is not reliable. Because it is possible that both checks on ep->rdllist and ep->ovflist are false while ep_start_scan() or ep_done_scan() is being executed on other CPUs, despite events are available.

This bug can be observed by:

1. Create an eventpoll with at least one ready level-triggered event

2. Create multiple threads who do epoll_wait() with zero timeout. The threads do not consume the events, therefore all epoll_wait() should return at least one event.

If one thread is executing ep_events_available() while another thread is executing ep_start_scan() or ep_done_scan(), epoll_wait() may wrongly return no event for the former thread.

This reproducer is implemented as TEST(epoll65) in tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c

Fix it by skipping ep_events_available(), just call ep_try_send_events() directly.

epoll_sendevents() (io_uring) suffers the same problem, fix that as well.

There is still ep_busy_loop() who uses ep_events_available() without lock, but it is probably okay (?) for busy-polling.

Fixes c5a282e9635e ("fs/epoll: reduce the scope of wq lock in

epoll_wait()") Fixes: e59d3c64cba6 ("epoll: eliminate unnecessary lock for zero timeout") Fixes: ae3a4f1fdc2c ("eventpoll: add epoll_sendevents() helper") Signed-off-by: Nam Cao <[email protected]>
Cc: [email protected]

---
 fs/eventpoll.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

fs/eventpoll.c +2 -14

index 0fbf5dfedb24..541481eafc20 100644
@@ -2022,7 +2022,7 @@ static int ep_schedule_timeout(ktime_t *to)
 static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
 		   int maxevents, struct timespec64 *timeout)
 {
-	int res, eavail, timed_out = 0;
+	int res, eavail = 1, timed_out = 0;
 	u64 slack = 0;
 	wait_queue_entry_t wait;
 	ktime_t expires, *to = NULL;

@@ -2041,16 +2041,6 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
 		timed_out = 1;
 	}
 
-	/*
-	 * This call is racy: We may or may not see events that are being added
-	 * to the ready list under the lock (e.g., in IRQ callbacks). For cases
-	 * with a non-zero timeout, this thread will check the ready list under
-	 * lock and will add to the wait queue.  For cases with a zero
-	 * timeout, the user by definition should not care and will have to
-	 * recheck again.
-	 */
-	eavail = ep_events_available(ep);
-
 	while (1) {
 		if (eavail) {
 			res = ep_try_send_events(ep, events, maxevents);

@@ -2496,9 +2486,7 @@ int epoll_sendevents(struct file *file, struct epoll_event __user *events,
 	 * Racy call, but that's ok - it should get retried based on
 	 * poll readiness anyway.
 	 */
-	if (ep_events_available(ep))
-		return ep_try_send_events(ep, events, maxevents);
-	return 0;
+	return ep_try_send_events(ep, events, maxevents);
 }
 
 /*

Signature · 1 line

2.39.5

I'll say upfront I'm not an epoll person, just looked here out of curiosity.

I can agree there is a bug. The event is generated before any of the threads even exist and they only poll for it, none of them consume it.

However, the commit message fails to explain why the change fixes anything and I think your patch de facto reverts e59d3c64cba6 ("epoll:
eliminate unnecessary lock for zero timeout"). Looking at that diff the point was to avoid the expensive lock trip if timeout == 0 and there are no events.

As for the bug is, from my reading the ep_start_scan()/ep_done_scan() pair transiently disturbs the state checked by ep_events_available(), resulting in false-negatives. Then the locked check works because by the time you acquire it, the damage is undone.

Given the commits referenced in Fixes:, I suspect the real fix would be to stop destroying that state of course.

But if that's not feasible, I would check if a sequence counter around this would do the trick -- then the racy ep_events_available(ep) upfront would become safe with smaller overhead than with your proposal for the no-event case, but with higher overhead when there is something.

My proposal is trivial to implement, I have no idea if it will get a buy-in though.

My question is whether the performance of epoll_wait() with zero timeout is really that important that we have to complicate things. If epoll_wait() with zero timeout is called repeatedly in a loop but there is no event, I'm sure there will be measurabled performance drop. But sane user would just use timeout in that case.

epoll's data is protected by a lock. Therefore I think the most straightforward solution is just taking the lock before reading the data.

Lockless is hard to get right and may cause hard-to-debug problems. So unless this performance drop somehow bothers someone, I would prefer "keep it simple, stupid".

Quoted 10 lines · Nam Cao

My question is whether the performance of epoll_wait() with zero timeout is really that important that we have to complicate things. If epoll_wait() with zero timeout is called repeatedly in a loop but there is no event, I'm sure there will be measurabled performance drop. But sane user would just use timeout in that case.

epoll's data is protected by a lock. Therefore I think the most straightforward solution is just taking the lock before reading the data.

I have no idea what the original use case is. I see the author of the patch is cc'ed, so hopefully they will answer.

Quoted 4 lines

Lockless is hard to get right and may cause hard-to-debug problems. So unless this performance drop somehow bothers someone, I would prefer "keep it simple, stupid".

Well epoll is known to suffer from lock contention, so I would like to think the lockless games were motivated by a real-world need, but I'm not going peruse the history to find out.

I can agree the current state concerning ep_events_available() is avoidably error prone and something(tm) should be done. fwiw the refcount thing is almost free on amd64, I have no idea how this pans out on arm64.

One ordering thing that sticks out to me is

Quoted 2 lines · Khazhy Kumykov

One ordering thing that sticks out to me is

(ordering can't help here since we'll rapidly be flapping between ovflist in use and inactive... right)

a sequence counter around shenanigans will sort it out, but I don't know if it is worth it and don't really want to investigate myself.

Quoted 10 lines · Nam Cao

My question is whether the performance of epoll_wait() with zero timeout is really that important that we have to complicate things. If epoll_wait() with zero timeout is called repeatedly in a loop but there is no event, I'm sure there will be measurabled performance drop. But sane user would just use timeout in that case.

epoll's data is protected by a lock. Therefore I think the most straightforward solution is just taking the lock before reading the data.

I have no idea what the original use case is. I see the author of the patch is cc'ed, so hopefully they will answer.

Quoted 4 lines

Lockless is hard to get right and may cause hard-to-debug problems. So unless this performance drop somehow bothers someone, I would prefer "keep it simple, stupid".

Well epoll is known to suffer from lock contention, so I would like to think the lockless games were motivated by a real-world need, but I'm not going peruse the history to find out.

I can agree the current state concerning ep_events_available() is avoidably error prone and something(tm) should be done. fwiw the refcount thing is almost free on amd64, I have no idea how this pans out on arm64.

Quoted 4 lines · On Wed, 17 Sep 2025 18:05:45 +0200 Mateusz Guzik <[email protected]> wrote:

I can agree the current state concerning ep_events_available() is avoidably error prone and something(tm) should be done. fwiw the refcount thing is almost free on amd64, I have no idea how this pans out on arm64.

Atomic operations are anything but free....
They are likely to be a similar cost to an uncontested spinlock entry.

While staring at epoll, I noticed ep_events_available() looks wrong. I wrote a small program to confirm, and yes it is definitely wrong.

This series adds a reproducer to kselftest, and fix the bug.