Revert "epoll: atomically remove wait entry on wake up"

Revert "eventpoll: fix missing wakeup for ovflist in ep_poll_callback"

Revert commit: 72417f2244
Revert commit: f51fcbb32f

thoese two commit make nginx slowly like this:

before revert:
--------------
./wrk -t84 -c8400 -d10s -T5s -H "Connection: Close" --latency http://x.x.x.x/hello
Running 10s test @ http://x.x.x.x/hello
  84 threads and 8400 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    26.06ms   33.53ms   1.03s    92.66%
    Req/Sec   723.03    416.59     9.03k    72.25%
  Latency Distribution
     50%   18.59ms
     75%   31.94ms
     90%   52.37ms
     99%  141.92ms
  592056 requests in 10.10s, 83.00MB read
  Socket errors: connect 0, read 253478, write 0, timeout 0
Requests/sec:  58619.13
Transfer/sec:      8.22MB

after revert:
-------------
./wrk -t84 -c8400 -d10s -T5s -H "Connection: Close" --latency http://x.x.x.x/hello

Running 10s test @ http://x.x.x.x/hello
  84 threads and 8400 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    22.10ms   29.46ms 894.07ms   93.15%
    Req/Sec     0.90k   598.46    12.02k    74.86%
  Latency Distribution
     50%   15.29ms
     75%   27.79ms
     90%   44.95ms
     99%  111.73ms
  736417 requests in 10.10s, 103.24MB read
  Socket errors: connect 0, read 327949, write 0, timeout 0
Requests/sec:  72921.54
Transfer/sec:     10.22MB

see, after revert nginx bench's through high and time latency low

Signed-off-by: mengensun<mengensun@tencent.com>
Reviewed-by: mungerjiang<mungerjiang@tencent.com>
Reviewed-by: kaixuxia<kaixuxia@tencent.com>
This commit is contained in:
mengensun 2022-02-18 19:35:39 +08:00 committed by kaixuxiakx
parent 1fde852cfe
commit b97f4782e1
1 changed files with 28 additions and 33 deletions

View File

@ -1174,10 +1174,6 @@ static inline bool chain_epi_lockless(struct epitem *epi)
{
struct eventpoll *ep = epi->ep;
/* Fast preliminary check */
if (epi->next != EP_UNACTIVE_PTR)
return false;
/* Check that the same epi has not been just chained from another CPU */
if (cmpxchg(&epi->next, EP_UNACTIVE_PTR, NULL) != EP_UNACTIVE_PTR)
return false;
@ -1244,12 +1240,16 @@ static int ep_poll_callback(wait_queue_entry_t *wait, unsigned mode, int sync, v
* chained in ep->ovflist and requeued later on.
*/
if (READ_ONCE(ep->ovflist) != EP_UNACTIVE_PTR) {
if (chain_epi_lockless(epi))
ep_pm_stay_awake_rcu(epi);
} else if (!ep_is_linked(epi)) {
/* In the usual case, add event to ready list. */
if (list_add_tail_lockless(&epi->rdllink, &ep->rdllist))
if (epi->next == EP_UNACTIVE_PTR &&
chain_epi_lockless(epi))
ep_pm_stay_awake_rcu(epi);
goto out_unlock;
}
/* If this file is already in the ready list we exit soon */
if (!ep_is_linked(epi) &&
list_add_tail_lockless(&epi->rdllink, &ep->rdllist)) {
ep_pm_stay_awake_rcu(epi);
}
/*
@ -1825,6 +1825,7 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
{
int res = 0, eavail, timed_out = 0;
u64 slack = 0;
bool waiter = false;
wait_queue_entry_t wait;
ktime_t expires, *to = NULL;
@ -1869,23 +1870,21 @@ fetch_events:
*/
ep_reset_busy_poll_napi_id(ep);
do {
/*
* Internally init_wait() uses autoremove_wake_function(),
* thus wait entry is removed from the wait queue on each
* wakeup. Why it is important? In case of several waiters
* each new wakeup will hit the next waiter, giving it the
* chance to harvest new event. Otherwise wakeup can be
* lost. This is also good performance-wise, because on
* normal wakeup path no need to call __remove_wait_queue()
* explicitly, thus ep->lock is not taken, which halts the
* event delivery.
*/
init_wait(&wait);
/*
* We don't have any available event to return to the caller. We need
* to sleep here, and we will be woken by ep_poll_callback() when events
* become available.
*/
if (!waiter) {
waiter = true;
init_waitqueue_entry(&wait, current);
write_lock_irq(&ep->lock);
__add_wait_queue_exclusive(&ep->wq, &wait);
write_unlock_irq(&ep->lock);
}
for (;;) {
/*
* We don't want to sleep if the ep_poll_callback() sends us
* a wakeup in between. That's why we set the task state
@ -1915,20 +1914,10 @@ fetch_events:
timed_out = 1;
break;
}
/* We were woken up, thus go and try to harvest some events */
eavail = 1;
} while (0);
}
__set_current_state(TASK_RUNNING);
if (!list_empty_careful(&wait.entry)) {
write_lock_irq(&ep->lock);
__remove_wait_queue(&ep->wq, &wait);
write_unlock_irq(&ep->lock);
}
send_events:
/*
* Try to transfer events to user space. In case we get 0 events and
@ -1939,6 +1928,12 @@ send_events:
!(res = ep_send_events(ep, events, maxevents)) && !timed_out)
goto fetch_events;
if (waiter) {
write_lock_irq(&ep->lock);
__remove_wait_queue(&ep->wq, &wait);
write_unlock_irq(&ep->lock);
}
return res;
}