We are optimizing the Request-Per-Second of nginx http server,
and we found that acquiring epmutex in eventpoll_release_file()
will become a bottleneck under the one-request-per-connection
scenario.
Optimize the epmutex with a smaller granularity. Introduce
an ref-counter to eventpoll and free eventpoll by rcu, using rcu
and list_first_or_null_rcu() to iterate file->f_ep_links instead
of epmutex.
The following are some details of the scenario:
HTTP server (nginx):
* under ARM64 with 64 cores
* 64 worker processes, each worker is binded to a specific CPU
* keepalive_requests = 1 in nginx.conf: nginx will close the
connection fd after a reply is send
HTTP client[benchmark] (wrk):
* under x86-64 with 48 cores
* 16 threads, 64 connections per-thread
Before the patch, the RPS measured by wrk is ~220K, after applying
the patch the RPS is ~240K. We also measure the overhead of
eventpoll_release_file() and its children by perf: 29% before and
2% after.
Link : https://lkml.org/lkml/2017/10/28/81
Signed-off-by: Cheng Jian <[email protected]>
Signed-off-by: Hou Tao <[email protected]>
---
fs/eventpoll.c | 106 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 92 insertions(+), 14 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index d7f1f5011fac..dc81f1c4fbaa 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -38,6 +38,7 @@
#include <linux/compat.h>
#include <linux/rculist.h>
#include <net/busy_poll.h>
+#include <linux/refcount.h>
/*
* LOCKING:
@@ -225,6 +226,11 @@ struct eventpoll {
/* used to track busy poll napi_id */
unsigned int napi_id;
#endif
+
+ /* used to ensure the validity of eventpoll when release file */
+ refcount_t ref;
+ /* used to free itself */
+ struct rcu_head rcu;
};
/* Wait structure used by the poll hooks */
@@ -809,6 +815,32 @@ static int ep_remove(struct eventpoll *ep, struct epitem
*epi)
return 0;
}
+static void ep_rcu_free(struct rcu_head *head)
+{
+ struct eventpoll *ep = container_of(head, struct eventpoll, rcu);
+
+ kfree(ep);
+}
+
+static void eventpoll_put_ep(struct eventpoll *ep)
+{
+ if (refcount_dec_and_test(&ep->ref)) {
+ mutex_destroy(&ep->mtx);
+ free_uid(ep->user);
+ wakeup_source_unregister(ep->ws);
+
+ call_rcu(&ep->rcu, ep_rcu_free);
+ }
+}
+
+static struct eventpoll *eventpoll_get_ep(struct eventpoll *ep)
+{
+ if (refcount_inc_not_zero(&ep->ref))
+ return ep;
+ else
+ return NULL;
+}
+
static void ep_free(struct eventpoll *ep)
{
struct rb_node *rbp;
@@ -826,11 +858,11 @@ static void ep_free(struct eventpoll *ep)
* anymore. The only hit might come from eventpoll_release_file() but
* holding "epmutex" is sufficient here.
*/
- mutex_lock(&epmutex);
/*
* Walks through the whole tree by unregistering poll callbacks.
*/
+ mutex_lock(&ep->mtx);
for (rbp = rb_first_cached(&ep->rbr); rbp; rbp = rb_next(rbp)) {
epi = rb_entry(rbp, struct epitem, rbn);
@@ -846,7 +878,6 @@ static void ep_free(struct eventpoll *ep)
* We do not need to lock ep->mtx, either, we only do it to prevent
* a lockdep warning.
*/
- mutex_lock(&ep->mtx);
while ((rbp = rb_first_cached(&ep->rbr)) != NULL) {
epi = rb_entry(rbp, struct epitem, rbn);
ep_remove(ep, epi);
@@ -854,11 +885,19 @@ static void ep_free(struct eventpoll *ep)
}
mutex_unlock(&ep->mtx);
- mutex_unlock(&epmutex);
- mutex_destroy(&ep->mtx);
- free_uid(ep->user);
- wakeup_source_unregister(ep->ws);
- kfree(ep);
+ /*
+ * ep will not been added to visited_list, because ep_ctrl()
+ * can not get its reference and can not reference it by the
+ * corresponding epitem. The only possible operation is list_del_init,
+ * so it's OK to use list_empty_careful() here.
+ */
+ if (!list_empty_careful(&ep->visited_list_link)) {
+ mutex_lock(&epmutex);
+ list_del_init(&ep->visited_list_link);
+ mutex_unlock(&epmutex);
+ }
+
+ eventpoll_put_ep(ep);
}
static int ep_eventpoll_release(struct inode *inode, struct file *file)
@@ -985,7 +1024,7 @@ static const struct file_operations eventpoll_fops = {
void eventpoll_release_file(struct file *file)
{
struct eventpoll *ep;
- struct epitem *epi, *next;
+ struct epitem *epi;
/*
* We don't want to get "file->f_lock" because it is not
@@ -1000,14 +1039,51 @@ void eventpoll_release_file(struct file *file)
*
* Besides, ep_remove() acquires the lock, so we can't hold it here.
*/
- mutex_lock(&epmutex);
- list_for_each_entry_safe(epi, next, &file->f_ep_links, fllink) {
- ep = epi->ep;
+ rcu_read_lock();
+ while (true) {
+ epi = list_first_or_null_rcu(&file->f_ep_links,
+ struct epitem, fllink);
+ if (!epi)
+ break;
+
+ ep = eventpoll_get_ep(epi->ep);
+ /* Current epi had been removed by ep_free() */
+ if (!ep)
+ continue;
+ rcu_read_unlock();
+
mutex_lock_nested(&ep->mtx, 0);
- ep_remove(ep, epi);
+ /*
+ * If rb_first_cached() returns NULL, it means that
+ * the current epi had been removed by ep_free().
+ * To prevent epi from double-freeing, check the
+ * condition before invoking ep_remove().
+ * If eventpoll_release_file() frees epi firstly,
+ * the epi will not be freed again because the epi
+ * must have been removed from ep->rbr when ep_free()
+ * is invoked.
+ */
+ if (rb_first_cached(&ep->rbr))
+ ep_remove(ep, epi);
mutex_unlock(&ep->mtx);
+
+ eventpoll_put_ep(ep);
+
+ rcu_read_lock();
+ }
+ rcu_read_unlock();
+
+ /*
+ * The file can not been added to tfile_check_list again, because
+ * (1) refcnt has been zero, ep_ctrl() can no longer get its reference
+ * (2) related ep items have been removed, ep_loop_check_proc() can not
+ * get the file by ep->rbr.
+ */
+ if (!list_empty_careful(&file->f_tfile_llink)) {
+ mutex_lock(&epmutex);
+ list_del_init(&file->f_tfile_llink);
+ mutex_unlock(&epmutex);
}
- mutex_unlock(&epmutex);
}
static int ep_alloc(struct eventpoll **pep)
@@ -1030,6 +1106,8 @@ static int ep_alloc(struct eventpoll **pep)
ep->rbr = RB_ROOT_CACHED;
ep->ovflist = EP_UNACTIVE_PTR;
ep->user = user;
+ INIT_LIST_HEAD(&ep->visited_list_link);
+ refcount_set(&ep->ref, 1);
*pep = ep;
@@ -2018,7 +2096,7 @@ static int ep_loop_check(struct eventpoll *ep, struct
file *file)
list_for_each_entry_safe(ep_cur, ep_next, &visited_list,
visited_list_link) {
ep_cur->visited = 0;
- list_del(&ep_cur->visited_list_link);
+ list_del_init(&ep_cur->visited_list_link);
}
return ret;
}
--
2.20.1
