Commit ae10b2b4 authored by Jason Baron's avatar Jason Baron Committed by Linus Torvalds

epoll: optimize EPOLL_CTL_DEL using rcu

Nathan Zimmer found that once we get over 10+ cpus, the scalability of
SPECjbb falls over due to the contention on the global 'epmutex', which is
taken in on EPOLL_CTL_ADD and EPOLL_CTL_DEL operations.

Patch #1 removes the 'epmutex' lock completely from the EPOLL_CTL_DEL path
by using rcu to guard against any concurrent traversals.

Patch #2 remove the 'epmutex' lock from EPOLL_CTL_ADD operations for
simple topologies.  IE when adding a link from an epoll file descriptor to
a wakeup source, where the epoll file descriptor is not nested.

This patch (of 2):

Optimize EPOLL_CTL_DEL such that it does not require the 'epmutex' by
converting the file->f_ep_links list into an rcu one.  In this way, we can
traverse the epoll network on the add path in parallel with deletes.
Since deletes can't create loops or worse wakeup paths, this is safe.

This patch in combination with the patch "epoll: Do not take global 'epmutex'
for simple topologies", shows a dramatic performance improvement in
scalability for SPECjbb.
Signed-off-by: default avatarJason Baron <jbaron@akamai.com>
Tested-by: default avatarNathan Zimmer <nzimmer@sgi.com>
Cc: Eric Wong <normalperson@yhbt.net>
Cc: Nelson Elhage <nelhage@nelhage.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Davide Libenzi <davidel@xmailserver.org>
Cc: "Paul E. McKenney" <paulmck@us.ibm.com>
CC: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 823b794c
...@@ -41,6 +41,7 @@ ...@@ -41,6 +41,7 @@
#include <linux/proc_fs.h> #include <linux/proc_fs.h>
#include <linux/seq_file.h> #include <linux/seq_file.h>
#include <linux/compat.h> #include <linux/compat.h>
#include <linux/rculist.h>
/* /*
* LOCKING: * LOCKING:
...@@ -133,8 +134,12 @@ struct nested_calls { ...@@ -133,8 +134,12 @@ struct nested_calls {
* of these on a server and we do not want this to take another cache line. * of these on a server and we do not want this to take another cache line.
*/ */
struct epitem { struct epitem {
/* RB tree node used to link this structure to the eventpoll RB tree */ union {
struct rb_node rbn; /* RB tree node links this structure to the eventpoll RB tree */
struct rb_node rbn;
/* Used to free the struct epitem */
struct rcu_head rcu;
};
/* List header used to link this structure to the eventpoll ready list */ /* List header used to link this structure to the eventpoll ready list */
struct list_head rdllink; struct list_head rdllink;
...@@ -671,6 +676,12 @@ static int ep_scan_ready_list(struct eventpoll *ep, ...@@ -671,6 +676,12 @@ static int ep_scan_ready_list(struct eventpoll *ep,
return error; return error;
} }
static void epi_rcu_free(struct rcu_head *head)
{
struct epitem *epi = container_of(head, struct epitem, rcu);
kmem_cache_free(epi_cache, epi);
}
/* /*
* Removes a "struct epitem" from the eventpoll RB tree and deallocates * Removes a "struct epitem" from the eventpoll RB tree and deallocates
* all the associated resources. Must be called with "mtx" held. * all the associated resources. Must be called with "mtx" held.
...@@ -692,8 +703,7 @@ static int ep_remove(struct eventpoll *ep, struct epitem *epi) ...@@ -692,8 +703,7 @@ static int ep_remove(struct eventpoll *ep, struct epitem *epi)
/* Remove the current item from the list of epoll hooks */ /* Remove the current item from the list of epoll hooks */
spin_lock(&file->f_lock); spin_lock(&file->f_lock);
if (ep_is_linked(&epi->fllink)) list_del_rcu(&epi->fllink);
list_del_init(&epi->fllink);
spin_unlock(&file->f_lock); spin_unlock(&file->f_lock);
rb_erase(&epi->rbn, &ep->rbr); rb_erase(&epi->rbn, &ep->rbr);
...@@ -704,9 +714,14 @@ static int ep_remove(struct eventpoll *ep, struct epitem *epi) ...@@ -704,9 +714,14 @@ static int ep_remove(struct eventpoll *ep, struct epitem *epi)
spin_unlock_irqrestore(&ep->lock, flags); spin_unlock_irqrestore(&ep->lock, flags);
wakeup_source_unregister(ep_wakeup_source(epi)); wakeup_source_unregister(ep_wakeup_source(epi));
/*
/* At this point it is safe to free the eventpoll item */ * At this point it is safe to free the eventpoll item. Use the union
kmem_cache_free(epi_cache, epi); * field epi->rcu, since we are trying to minimize the size of
* 'struct epitem'. The 'rbn' field is no longer in use. Protected by
* ep->mtx. The rcu read side, reverse_path_check_proc(), does not make
* use of the rbn field.
*/
call_rcu(&epi->rcu, epi_rcu_free);
atomic_long_dec(&ep->user->epoll_watches); atomic_long_dec(&ep->user->epoll_watches);
...@@ -872,7 +887,6 @@ static const struct file_operations eventpoll_fops = { ...@@ -872,7 +887,6 @@ static const struct file_operations eventpoll_fops = {
*/ */
void eventpoll_release_file(struct file *file) void eventpoll_release_file(struct file *file)
{ {
struct list_head *lsthead = &file->f_ep_links;
struct eventpoll *ep; struct eventpoll *ep;
struct epitem *epi; struct epitem *epi;
...@@ -890,17 +904,12 @@ void eventpoll_release_file(struct file *file) ...@@ -890,17 +904,12 @@ void eventpoll_release_file(struct file *file)
* Besides, ep_remove() acquires the lock, so we can't hold it here. * Besides, ep_remove() acquires the lock, so we can't hold it here.
*/ */
mutex_lock(&epmutex); mutex_lock(&epmutex);
list_for_each_entry_rcu(epi, &file->f_ep_links, fllink) {
while (!list_empty(lsthead)) {
epi = list_first_entry(lsthead, struct epitem, fllink);
ep = epi->ep; ep = epi->ep;
list_del_init(&epi->fllink);
mutex_lock_nested(&ep->mtx, 0); mutex_lock_nested(&ep->mtx, 0);
ep_remove(ep, epi); ep_remove(ep, epi);
mutex_unlock(&ep->mtx); mutex_unlock(&ep->mtx);
} }
mutex_unlock(&epmutex); mutex_unlock(&epmutex);
} }
...@@ -1138,7 +1147,9 @@ static int reverse_path_check_proc(void *priv, void *cookie, int call_nests) ...@@ -1138,7 +1147,9 @@ static int reverse_path_check_proc(void *priv, void *cookie, int call_nests)
struct file *child_file; struct file *child_file;
struct epitem *epi; struct epitem *epi;
list_for_each_entry(epi, &file->f_ep_links, fllink) { /* CTL_DEL can remove links here, but that can't increase our count */
rcu_read_lock();
list_for_each_entry_rcu(epi, &file->f_ep_links, fllink) {
child_file = epi->ep->file; child_file = epi->ep->file;
if (is_file_epoll(child_file)) { if (is_file_epoll(child_file)) {
if (list_empty(&child_file->f_ep_links)) { if (list_empty(&child_file->f_ep_links)) {
...@@ -1160,6 +1171,7 @@ static int reverse_path_check_proc(void *priv, void *cookie, int call_nests) ...@@ -1160,6 +1171,7 @@ static int reverse_path_check_proc(void *priv, void *cookie, int call_nests)
"file is not an ep!\n"); "file is not an ep!\n");
} }
} }
rcu_read_unlock();
return error; return error;
} }
...@@ -1286,7 +1298,7 @@ static int ep_insert(struct eventpoll *ep, struct epoll_event *event, ...@@ -1286,7 +1298,7 @@ static int ep_insert(struct eventpoll *ep, struct epoll_event *event,
/* Add the current item to the list of active epoll hook for this file */ /* Add the current item to the list of active epoll hook for this file */
spin_lock(&tfile->f_lock); spin_lock(&tfile->f_lock);
list_add_tail(&epi->fllink, &tfile->f_ep_links); list_add_tail_rcu(&epi->fllink, &tfile->f_ep_links);
spin_unlock(&tfile->f_lock); spin_unlock(&tfile->f_lock);
/* /*
...@@ -1327,8 +1339,7 @@ static int ep_insert(struct eventpoll *ep, struct epoll_event *event, ...@@ -1327,8 +1339,7 @@ static int ep_insert(struct eventpoll *ep, struct epoll_event *event,
error_remove_epi: error_remove_epi:
spin_lock(&tfile->f_lock); spin_lock(&tfile->f_lock);
if (ep_is_linked(&epi->fllink)) list_del_rcu(&epi->fllink);
list_del_init(&epi->fllink);
spin_unlock(&tfile->f_lock); spin_unlock(&tfile->f_lock);
rb_erase(&epi->rbn, &ep->rbr); rb_erase(&epi->rbn, &ep->rbr);
...@@ -1844,15 +1855,12 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd, ...@@ -1844,15 +1855,12 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
* and hang them on the tfile_check_list, so we can check that we * and hang them on the tfile_check_list, so we can check that we
* haven't created too many possible wakeup paths. * haven't created too many possible wakeup paths.
* *
* We need to hold the epmutex across both ep_insert and ep_remove * We need to hold the epmutex across ep_insert to prevent
* b/c we want to make sure we are looking at a coherent view of * multple adds from creating loops in parallel.
* epoll network.
*/ */
if (op == EPOLL_CTL_ADD || op == EPOLL_CTL_DEL) { if (op == EPOLL_CTL_ADD) {
mutex_lock(&epmutex); mutex_lock(&epmutex);
did_lock_epmutex = 1; did_lock_epmutex = 1;
}
if (op == EPOLL_CTL_ADD) {
if (is_file_epoll(tf.file)) { if (is_file_epoll(tf.file)) {
error = -ELOOP; error = -ELOOP;
if (ep_loop_check(ep, tf.file) != 0) { if (ep_loop_check(ep, tf.file) != 0) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment