All of lore.kernel.org
 help / color / mirror / Atom feed
From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
To: torvalds@linux-foundation.org, viro@zeniv.linux.org.uk,
	linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	dave@stgolabs.net, edumazet@google.com, willemb@google.com,
	khazhy@google.com, guantaol@google.com,
	Soheil Hassas Yeganeh <soheil@google.com>
Subject: [PATCH 8/8] epoll: eliminate unnecessary lock for zero timeout
Date: Fri,  6 Nov 2020 18:16:35 -0500	[thread overview]
Message-ID: <20201106231635.3528496-9-soheil.kdev@gmail.com> (raw)
In-Reply-To: <20201106231635.3528496-1-soheil.kdev@gmail.com>

From: Soheil Hassas Yeganeh <soheil@google.com>

We call ep_events_available() under lock when timeout is 0,
and then call it without locks in the loop for the other cases.

Instead, call ep_events_available() without lock for all cases.
For non-zero timeouts, we will recheck after adding the thread to
the wait queue. For zero timeout cases, by definition, user is
opportunistically polling and will have to call epoll_wait again
in the future.

Note that this lock was kept in c5a282e9635e9 because the whole
loop was historically under lock.

This patch results in a 1% CPU/RPC reduction in RPC benchmarks.

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Khazhismel Kumykov <khazhy@google.com>
---
 fs/eventpoll.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index f4e1be7ada26..1aa23b0be72b 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1830,7 +1830,7 @@ static inline struct timespec64 ep_set_mstimeout(long ms)
 static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
 		   int maxevents, long timeout)
 {
-	int res, eavail = 0, timed_out = 0;
+	int res, eavail, timed_out = 0;
 	u64 slack = 0;
 	wait_queue_entry_t wait;
 	ktime_t expires, *to = NULL;
@@ -1846,18 +1846,21 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
 	} else if (timeout == 0) {
 		/*
 		 * Avoid the unnecessary trip to the wait queue loop, if the
-		 * caller specified a non blocking operation. We still need
-		 * lock because we could race and not see an epi being added
-		 * to the ready list while in irq callback. Thus incorrectly
-		 * returning 0 back to userspace.
+		 * caller specified a non blocking operation.
 		 */
 		timed_out = 1;
-
-		write_lock_irq(&ep->lock);
-		eavail = ep_events_available(ep);
-		write_unlock_irq(&ep->lock);
 	}
 
+	/*
+	 * 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 added 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) {
 			/*
@@ -1873,10 +1876,6 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
 		if (timed_out)
 			return 0;
 
-		eavail = ep_events_available(ep);
-		if (eavail)
-			continue;
-
 		eavail = ep_busy_loop(ep, timed_out);
 		if (eavail)
 			continue;
-- 
2.29.1.341.ge80a0c044ae-goog


  parent reply	other threads:[~2020-11-06 23:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-06 23:16 [PATCH 0/8] simplify ep_poll Soheil Hassas Yeganeh
2020-11-06 23:16 ` [PATCH 1/8] epoll: check for events when removing a timed out thread from the wait queue Soheil Hassas Yeganeh
2020-11-10  6:05   ` Davidlohr Bueso
2020-11-06 23:16 ` [PATCH 2/8] epoll: simplify signal handling Soheil Hassas Yeganeh
2020-11-06 23:16 ` [PATCH 3/8] epoll: pull fatal signal checks into ep_send_events() Soheil Hassas Yeganeh
2020-11-06 23:16 ` [PATCH 4/8] epoll: move eavail next to the list_empty_careful check Soheil Hassas Yeganeh
2020-11-06 23:16 ` [PATCH 5/8] epoll: simplify and optimize busy loop logic Soheil Hassas Yeganeh
2020-11-06 23:16 ` [PATCH 6/8] epoll: pull all code between fetch_events and send_event into the loop Soheil Hassas Yeganeh
2020-11-06 23:16 ` [PATCH 7/8] epoll: replace gotos with a proper loop Soheil Hassas Yeganeh
2020-11-06 23:16 ` Soheil Hassas Yeganeh [this message]
2020-11-06 23:35 ` [PATCH 0/8] simplify ep_poll Linus Torvalds
2020-11-08  1:43 ` Andrew Morton
2020-11-08  4:45   ` Soheil Hassas Yeganeh
2020-11-09 18:59     ` Davidlohr Bueso
2020-11-09 19:28       ` Soheil Hassas Yeganeh
2020-11-10 22:05     ` Andrew Morton
2020-11-11  3:37       ` Soheil Hassas Yeganeh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201106231635.3528496-9-soheil.kdev@gmail.com \
    --to=soheil.kdev@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=edumazet@google.com \
    --cc=guantaol@google.com \
    --cc=khazhy@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=soheil@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.