Skip to content

Commit

Permalink
proc: change proc_subdir_lock to a rwlock
Browse files Browse the repository at this point in the history
The proc_subdir_lock spinlock is used to allow only one task to make
change to the proc directory structure as well as looking up information
in it.  However, the information lookup part can actually be entered by
more than one task as the pde_get() and pde_put() reference count update
calls in the critical sections are atomic increment and decrement
respectively and so are safe with concurrent updates.

The x86 architecture has already used qrwlock which is fair and other
architectures like ARM are in the process of switching to qrwlock.  So
unfairness shouldn't be a concern in that conversion.

This patch changed the proc_subdir_lock to a rwlock in order to enable
concurrent lookup. The following functions were modified to take a
write lock:
 - proc_register()
 - remove_proc_entry()
 - remove_proc_subtree()

The following functions were modified to take a read lock:
 - xlate_proc_name()
 - proc_lookup_de()
 - proc_readdir_de()

A parallel /proc filesystem search with the "find" command (1000 threads)
was run on a 4-socket Haswell-EX box (144 threads).  Before the patch, the
parallel search took about 39s.  After the patch, the parallel find took
only 25s, a saving of about 14s.

The micro-benchmark that I used was artificial, but it was used to
reproduce an exit hanging problem that I saw in real application.  In
fact, only allow one task to do a lookup seems too limiting to me.

Signed-off-by: Waiman Long <Waiman.Long@hp.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Scott J Norton <scott.norton@hp.com>
Cc: Douglas Hatch <doug.hatch@hp.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
longman88 authored and torvalds committed Sep 10, 2015
1 parent bdb4d10 commit ecf1a3d
Showing 1 changed file with 22 additions and 22 deletions.
44 changes: 22 additions & 22 deletions fs/proc/generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

#include "internal.h"

static DEFINE_SPINLOCK(proc_subdir_lock);
static DEFINE_RWLOCK(proc_subdir_lock);

static int proc_match(unsigned int len, const char *name, struct proc_dir_entry *de)
{
Expand Down Expand Up @@ -172,9 +172,9 @@ static int xlate_proc_name(const char *name, struct proc_dir_entry **ret,
{
int rv;

spin_lock(&proc_subdir_lock);
read_lock(&proc_subdir_lock);
rv = __xlate_proc_name(name, ret, residual);
spin_unlock(&proc_subdir_lock);
read_unlock(&proc_subdir_lock);
return rv;
}

Expand Down Expand Up @@ -231,19 +231,19 @@ struct dentry *proc_lookup_de(struct proc_dir_entry *de, struct inode *dir,
{
struct inode *inode;

spin_lock(&proc_subdir_lock);
read_lock(&proc_subdir_lock);
de = pde_subdir_find(de, dentry->d_name.name, dentry->d_name.len);
if (de) {
pde_get(de);
spin_unlock(&proc_subdir_lock);
read_unlock(&proc_subdir_lock);
inode = proc_get_inode(dir->i_sb, de);
if (!inode)
return ERR_PTR(-ENOMEM);
d_set_d_op(dentry, &simple_dentry_operations);
d_add(dentry, inode);
return NULL;
}
spin_unlock(&proc_subdir_lock);
read_unlock(&proc_subdir_lock);
return ERR_PTR(-ENOENT);
}

Expand All @@ -270,12 +270,12 @@ int proc_readdir_de(struct proc_dir_entry *de, struct file *file,
if (!dir_emit_dots(file, ctx))
return 0;

spin_lock(&proc_subdir_lock);
read_lock(&proc_subdir_lock);
de = pde_subdir_first(de);
i = ctx->pos - 2;
for (;;) {
if (!de) {
spin_unlock(&proc_subdir_lock);
read_unlock(&proc_subdir_lock);
return 0;
}
if (!i)
Expand All @@ -287,19 +287,19 @@ int proc_readdir_de(struct proc_dir_entry *de, struct file *file,
do {
struct proc_dir_entry *next;
pde_get(de);
spin_unlock(&proc_subdir_lock);
read_unlock(&proc_subdir_lock);
if (!dir_emit(ctx, de->name, de->namelen,
de->low_ino, de->mode >> 12)) {
pde_put(de);
return 0;
}
spin_lock(&proc_subdir_lock);
read_lock(&proc_subdir_lock);
ctx->pos++;
next = pde_subdir_next(de);
pde_put(de);
de = next;
} while (de);
spin_unlock(&proc_subdir_lock);
read_unlock(&proc_subdir_lock);
return 1;
}

Expand Down Expand Up @@ -338,16 +338,16 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp
if (ret)
return ret;

spin_lock(&proc_subdir_lock);
write_lock(&proc_subdir_lock);
dp->parent = dir;
if (pde_subdir_insert(dir, dp) == false) {
WARN(1, "proc_dir_entry '%s/%s' already registered\n",
dir->name, dp->name);
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);
proc_free_inum(dp->low_ino);
return -EEXIST;
}
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);

return 0;
}
Expand Down Expand Up @@ -549,17 +549,17 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent)
const char *fn = name;
unsigned int len;

spin_lock(&proc_subdir_lock);
write_lock(&proc_subdir_lock);
if (__xlate_proc_name(name, &parent, &fn) != 0) {
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);
return;
}
len = strlen(fn);

de = pde_subdir_find(parent, fn, len);
if (de)
rb_erase(&de->subdir_node, &parent->subdir);
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);
if (!de) {
WARN(1, "name '%s'\n", name);
return;
Expand All @@ -583,16 +583,16 @@ int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
const char *fn = name;
unsigned int len;

spin_lock(&proc_subdir_lock);
write_lock(&proc_subdir_lock);
if (__xlate_proc_name(name, &parent, &fn) != 0) {
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);
return -ENOENT;
}
len = strlen(fn);

root = pde_subdir_find(parent, fn, len);
if (!root) {
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);
return -ENOENT;
}
rb_erase(&root->subdir_node, &parent->subdir);
Expand All @@ -605,7 +605,7 @@ int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
de = next;
continue;
}
spin_unlock(&proc_subdir_lock);
write_unlock(&proc_subdir_lock);

proc_entry_rundown(de);
next = de->parent;
Expand All @@ -616,7 +616,7 @@ int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
break;
pde_put(de);

spin_lock(&proc_subdir_lock);
write_lock(&proc_subdir_lock);
de = next;
}
pde_put(root);
Expand Down

0 comments on commit ecf1a3d

Please sign in to comment.