Skip to contentSkip to conversation

proc: faster /proc/$PID lookup

PATCH In review Alexey Dobriyan 2 participants 2 messages Jun 13, 2014 Proc filesystem

lore
Share & tools
lore

Alexey Dobriyan

author
#0 lore raw

PATCH 1/1 proc: faster /proc/$PID lookup

Use first character as a hint for /proc/$PID lookup.

Currently it takes one spinlock and search through dozens of misc proc entries which can not match /proc/$PID.

Signed-off-by Alexey Dobriyan <[email protected]>

---

 fs/proc/root.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
fs/proc/root.c +11 -4
@@ -199,10 +199,17 @@ static int proc_root_getattr(struct vfsmount *mnt, struct dentry *dentry, struct
 
 static struct dentry *proc_root_lookup(struct inode * dir, struct dentry * dentry, unsigned int flags)
 {
-	if (!proc_lookup(dir, dentry, flags))
-		return NULL;
-	
-	return proc_pid_lookup(dir, dentry, flags);
+	const char c = dentry->d_name.name[0];
+	struct dentry *rv;
+
+	if ('0' <= c && c <= '9') {
+		rv = proc_pid_lookup(dir, dentry, flags);
+		if (rv)
+			rv = proc_lookup(dir, dentry, flags);
+	} else {
+		rv = proc_lookup(dir, dentry, flags);
+	}
+	return rv;
 }
 
 static int proc_root_readdir(struct file *file, struct dir_context *ctx)
#1 lore raw
Quoted 8 lines · Alexey Dobriyan

static struct dentry *proc_root_lookup(struct inode * dir, struct dentry * dentry, unsigned int flags) {

(quoted diff) +2 -4
-	if (!proc_lookup(dir, dentry, flags))
-		return NULL;
-	
-	return proc_pid_lookup(dir, dentry, flags);
+	const char c = dentry->d_name.name[0];
+	struct dentry *rv;

Review

Quoted 10 lines
(quoted diff) +9 -0
+
+	if ('0' <= c && c <= '9') {
+		rv = proc_pid_lookup(dir, dentry, flags);
+		if (rv)
+			rv = proc_lookup(dir, dentry, flags);
+	} else {
+		rv = proc_lookup(dir, dentry, flags);
+	}
+	return rv;
 }

<digs in old stashes>

OK... First of all, proc_pid_lookup() will return NULL on anything that isn't a valid number. So you really want to change that (e.g. make it return ERR_PTR(-ENOENT) on failure). But then you don't need that opencoded isdigit(c) - proc_pid_lookup() will bugger off very quickly on any names that are not numbers, so your first branch will serve just fine in both cases.

Another thing missing is that you really want to prevent __proc_create() in root with name looking like a number. I ended up making name_to_int() qstr->unsigned (instead of dentry->unsigned); it probably could've been switched to something less opencoded at the same time.

Anyway, diff below is old, probably won't apply and might be completely broken - I don't remember why it didn't go anywhere. Hell knows, might turn out to be useful...

commit 893028f538560b4d687c1685d7b1a2b4a396277c
Merge: e8cd816 28d55bd
Author: Al Viro <[email protected]>
Date: Tue Mar 26 20:33:14 2013 -0400

WIP on for-linus: e8cd816 vt: synchronize_rcu() under spinlock is not nice...

fs/proc/base.c +0 -0
diff --cc fs/proc/base.c
index 69078c7,69078c7..02b07c7
@@@ -2727,12 -2727,12 +2727,12 @@@ out
  
  struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsigned int flags)
  {
--	struct dentry *result = NULL;
++	struct dentry *result = ERR_PTR(-ENOENT);
  	struct task_struct *task;
  	unsigned tgid;
  	struct pid_namespace *ns;
  
--	tgid = name_to_int(dentry);
++	tgid = name_to_int(&dentry->d_name);
  	if (tgid == ~0U)
  		goto out;
  
@@@ -2990,7 -2990,7 +2990,7 @@@ static struct dentry *proc_task_lookup(
  	if (!leader)
  		goto out_no_task;
  
--	tid = name_to_int(dentry);
++	tid = name_to_int(&dentry->d_name);
  	if (tid == ~0U)
  		goto out;
fs/proc/fd.c +0 -0
diff --cc fs/proc/fd.c
index d7a4a28,d7a4a28..5f4286c
@@@ -205,7 -205,7 +205,7 @@@ static struct dentry *proc_lookupfd_com
  {
  	struct task_struct *task = get_proc_task(dir);
  	struct dentry *result = ERR_PTR(-ENOENT);
--	unsigned fd = name_to_int(dentry);
++	unsigned fd = name_to_int(&dentry->d_name);
  
  	if (!task)
  		goto out_no_task;
fs/proc/generic.c +0 -0
diff --cc fs/proc/generic.c
index 4b3b3ff,4b3b3ff..e13d2da
@@@ -580,7 -580,7 +580,7 @@@ static struct proc_dir_entry *__proc_cr
  {
  	struct proc_dir_entry *ent = NULL;
  	const char *fn = name;
--	unsigned int len;
++	struct qstr q;
  
  	/* make sure name is valid */
  	if (!name || !strlen(name))
@@@ -593,14 -593,14 +593,17 @@@
  	if (strchr(fn, '/'))
  		goto out;
  
--	len = strlen(fn);
++	q.name = fn;
++	q.len = strlen(fn);
++	if (*parent == &proc_root && name_to_int(&q) != ~0U)
++		return NULL;
  
--	ent = kzalloc(sizeof(struct proc_dir_entry) + len + 1, GFP_KERNEL);
++	ent = kzalloc(sizeof(struct proc_dir_entry) + q.len + 1, GFP_KERNEL);
  	if (!ent)
  		goto out;
  
--	memcpy(ent->name, fn, len + 1);
--	ent->namelen = len;
++	memcpy(ent->name, q.name, q.len + 1);
++	ent->namelen = q.len;
  	ent->mode = mode;
  	ent->nlink = nlink;
  	atomic_set(&ent->count, 1);
fs/proc/internal.h +0 -0
diff --cc fs/proc/internal.h
index 85ff3a4,85ff3a4..fba6da2
@@@ -123,10 -123,10 +123,10 @@@ static inline int pid_delete_dentry(con
  	return !proc_pid(dentry->d_inode)->tasks[PIDTYPE_PID].first;
  }
  
--static inline unsigned name_to_int(struct dentry *dentry)
++static inline unsigned name_to_int(struct qstr *qstr)
  {
--	const char *name = dentry->d_name.name;
--	int len = dentry->d_name.len;
++	const char *name = qstr->name;
++	int len = qstr->len;
  	unsigned n = 0;
  
  	if (len > 1 && *name == '0')
fs/proc/root.c +0 -0
diff --cc fs/proc/root.c
index c6e9fac,c6e9fac..67c9dc4
@@@ -190,10 -190,10 +190,10 @@@ static int proc_root_getattr(struct vfs
  
  static struct dentry *proc_root_lookup(struct inode * dir, struct dentry * dentry, unsigned int flags)
  {
--	if (!proc_lookup(dir, dentry, flags))
++	if (!proc_pid_lookup(dir, dentry, flags))
  		return NULL;
  	
--	return proc_pid_lookup(dir, dentry, flags);
++	return proc_lookup(dir, dentry, flags);
  }
  
  static int proc_root_readdir(struct file * filp,