Re: [PATCH RESEND v5] fs/epoll: Remove unnecessary wakeups of nested epoll

Hi,

On Wed, Oct 9, 2019 at 5:21 PM Roman Penyaev rpenyaev@suse.de wrote:

On 2019-10-09 08:05, hev wrote:

...

From: Heiher r@hev.cc

Take the case where we have:

    t0
     | (ew)
    e0
     | (et)
    e1
     | (lt)
    s0

t0: thread 0 e0: epoll fd 0 e1: epoll fd 1 s0: socket fd 0 ew: epoll_wait et: edge-trigger lt: level-trigger

We remove unnecessary wakeups to prevent the nested epoll that working in edge- triggered mode to waking up continuously.

Test code: #include <unistd.h> #include <sys/epoll.h> #include <sys/socket.h>

int main(int argc, char *argv[]) { int sfd[2]; int efd[2]; struct epoll_event e;

  if (socketpair(AF_UNIX, SOCK_STREAM, 0, sfd) < 0)
          goto out;

  efd[0] = epoll_create(1);
  if (efd[0] < 0)
          goto out;

  efd[1] = epoll_create(1);
  if (efd[1] < 0)
          goto out;

  e.events = EPOLLIN;
  if (epoll_ctl(efd[1], EPOLL_CTL_ADD, sfd[0], &e) < 0)
          goto out;

  e.events = EPOLLIN | EPOLLET;
  if (epoll_ctl(efd[0], EPOLL_CTL_ADD, efd[1], &e) < 0)
          goto out;

  if (write(sfd[1], "w", 1) != 1)
          goto out;

  if (epoll_wait(efd[0], &e, 1, 0) != 1)
          goto out;

  if (epoll_wait(efd[0], &e, 1, 0) != 0)
          goto out;

  close(efd[0]);
  close(efd[1]);
  close(sfd[0]);
  close(sfd[1]);

  return 0;

out: return -1; }

More tests: https://github.com/heiher/epoll-wakeup

Cc: Al Viro viro@ZenIV.linux.org.uk Cc: Andrew Morton akpm@linux-foundation.org Cc: Davide Libenzi davidel@xmailserver.org Cc: Davidlohr Bueso dave@stgolabs.net Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Eric Wong e@80x24.org Cc: Jason Baron jbaron@akamai.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Roman Penyaev rpenyaev@suse.de Cc: Sridhar Samudrala sridhar.samudrala@intel.com Cc: linux-kernel@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org Signed-off-by: hev r@hev.cc

---

fs/eventpoll.c | 16 ---------------- 1 file changed, 16 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c index c4159bcc05d9..75fccae100b5 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -671,7 +671,6 @@ static __poll_t ep_scan_ready_list(struct eventpoll *ep, void *priv, int depth, bool ep_locked) { __poll_t res;

• int pwake = 0;
  struct epitem *epi, *nepi;
  LIST_HEAD(txlist);
  
  

@@ -738,26 +737,11 @@ static __poll_t ep_scan_ready_list(struct eventpoll *ep, */ list_splice(&txlist, &ep->rdllist); __pm_relax(ep->ws);

• if (!list_empty(&ep->rdllist)) {
  

•         /*
  

•          * Wake up (if active) both the eventpoll wait list and
  

•          * the ->poll() wait list (delayed after we release the lock).
  

•          */
  

•         if (waitqueue_active(&ep->wq))
  

•                 wake_up(&ep->wq);
  

•         if (waitqueue_active(&ep->poll_wait))
  

•                 pwake++;
  

• }
  write_unlock_irq(&ep->lock);
  
  if (!ep_locked)
          mutex_unlock(&ep->mtx);
  
  

• /* We have to call this outside the lock */
  

• if (pwake)
  

•         ep_poll_safewake(&ep->poll_wait);
  

• return res;
  

}

This looks good to me. Heiher, mind to make kselftest out of your test suite?

Reviewed-by: Roman Penyaev rpenyaev@suse.de

-- Roman

Need to back port this patch to stable branches?