mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/5] proc: Make the generation of the self symlink table driven.
@ 2006-09-06 16:23 Eric W. Biederman
  2006-09-06 16:24 ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Eric W. Biederman
  2006-09-07 17:15 ` [PATCH 1/5] proc: Make the generation of the self symlink table driven Andrew Morton
  0 siblings, 2 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-06 16:23 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


This patch generalizes the concept of files in /proc that are
related to processes but live in the root directory of /proc

Ideally this would reuse infrastructure from the rest of the
process specific parts of proc but unfortunately
security_task_to_inode must not be called on files that
are not strictly per process.  security_task_to_inode
really needs to be reexamined as the security label can
change in important places that we are not currently
catching, but I'm not certain that simplifies this problem.

By at least matching the structure of the rest of proc
we get more idiom reuse and it becomes easier to spot problems
in the way things are put together.

Later things like /proc/mounts are likely to be moved into
proc_base as well.  If union mounts are ever supported
we may be able to make /proc a union mount, and properly
split it into 2 filesystems.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/proc/base.c |  133 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 111 insertions(+), 22 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 4096518..9055918 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1674,6 +1674,108 @@ static struct inode_operations proc_self
 };
 
 /*
+ * proc base
+ *
+ * These are the directory entries in the root directory of /proc
+ * that properly belong to the /proc filesystem, as they describe
+ * describe something that is process related.
+ */
+static struct pid_entry proc_base_stuff[] = {
+	NOD(PROC_TGID_INO, 	"self", S_IFLNK|S_IRWXUGO,
+		&proc_self_inode_operations, NULL, {}),
+	{}
+};
+
+/*
+ *	Exceptional case: normally we are not allowed to unhash a busy
+ * directory. In this case, however, we can do it - no aliasing problems
+ * due to the way we treat inodes.
+ */
+static int proc_base_revalidate(struct dentry *dentry, struct nameidata *nd)
+{
+	struct inode *inode = dentry->d_inode;
+	struct task_struct *task = get_proc_task(inode);
+	if (task) {
+		put_task_struct(task);
+		return 1;
+	}
+	d_drop(dentry);
+	return 0;
+}
+
+static struct dentry_operations proc_base_dentry_operations =
+{
+	.d_revalidate	= proc_base_revalidate,
+	.d_delete	= pid_delete_dentry,
+};
+
+static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry)
+{
+	struct inode *inode;
+	struct dentry *error;
+	struct task_struct *task = get_proc_task(dir);
+	struct pid_entry *p;
+	struct proc_inode *ei;
+
+	error = ERR_PTR(-ENOENT);
+	inode = NULL;
+
+	if (!task)
+		goto out_no_task;
+
+	/* Lookup the directory entry */
+	for (p = proc_base_stuff; p->name; p++) {
+		if (p->len != dentry->d_name.len)
+			continue;
+		if (!memcmp(dentry->d_name.name, p->name, p->len))
+			break;
+	}
+	if (!p->name)
+		goto out;
+
+	/* Allocate the inode */
+	error = ERR_PTR(-ENOMEM);
+	inode = new_inode(dir->i_sb);
+	if (!inode)
+		goto out;
+
+	/* Initialize the inode */
+	ei = PROC_I(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
+	inode->i_ino = fake_ino(0, p->type);
+
+	/*
+	 * grab the reference to the task.
+	 */
+	ei->pid = get_pid(task_pid(task));
+	if (!ei->pid)
+		goto out_iput;
+
+	inode->i_uid = 0;
+	inode->i_gid = 0;
+	inode->i_mode = p->mode;
+	if (S_ISDIR(inode->i_mode))
+		inode->i_nlink = 2;
+	if (S_ISLNK(inode->i_mode))
+		inode->i_size = 64;
+	if (p->iop)
+		inode->i_op = p->iop;
+	if (p->fop)
+		inode->i_fop = p->fop;
+	ei->op = p->op;
+	dentry->d_op = &proc_base_dentry_operations;
+	d_add(dentry, inode);
+	error = NULL;
+out:
+	put_task_struct(task);
+out_no_task:
+	return error;
+out_iput:
+	iput(inode);
+	goto out;
+}
+
+/*
  * Thread groups
  */
 static struct file_operations proc_task_operations;
@@ -1819,24 +1921,12 @@ struct dentry *proc_pid_lookup(struct in
 	struct dentry *result = ERR_PTR(-ENOENT);
 	struct task_struct *task;
 	struct inode *inode;
-	struct proc_inode *ei;
 	unsigned tgid;
 
-	if (dentry->d_name.len == 4 && !memcmp(dentry->d_name.name,"self",4)) {
-		inode = new_inode(dir->i_sb);
-		if (!inode)
-			return ERR_PTR(-ENOMEM);
-		ei = PROC_I(inode);
-		inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-		inode->i_ino = fake_ino(0, PROC_TGID_INO);
-		ei->pde = NULL;
-		inode->i_mode = S_IFLNK|S_IRWXUGO;
-		inode->i_uid = inode->i_gid = 0;
-		inode->i_size = 64;
-		inode->i_op = &proc_self_inode_operations;
-		d_add(dentry, inode);
-		return NULL;
-	}
+	result = proc_base_lookup(dir, dentry);
+	if (!IS_ERR(result) || PTR_ERR(result) != -ENOENT)
+		goto out;
+
 	tgid = name_to_int(dentry);
 	if (tgid == ~0U)
 		goto out;
@@ -1922,12 +2012,11 @@ int proc_pid_readdir(struct file * filp,
 	struct task_struct *task;
 	int tgid;
 
-	if (!nr) {
-		ino_t ino = fake_ino(0,PROC_TGID_INO);
-		if (filldir(dirent, "self", 4, filp->f_pos, ino, DT_LNK) < 0)
-			return 0;
-		filp->f_pos++;
-		nr++;
+	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
+		struct pid_entry *p = &proc_base_stuff[nr];
+		if (filldir(dirent, p->name, p->len, filp->f_pos,
+			    fake_ino(0, p->type), p->mode >> 12) < 0)
+			goto out;
 	}
 
 	tgid = filp->f_pos - TGID_OFFSET;
-- 
1.4.2.rc3.g7e18e-dirty


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/5] proc: Factor out an instantiate method from every lookup method.
  2006-09-06 16:23 [PATCH 1/5] proc: Make the generation of the self symlink table driven Eric W. Biederman
@ 2006-09-06 16:24 ` Eric W. Biederman
  2006-09-06 16:27   ` [PATCH 3/5] proc: Remove the hard coded inode numbers Eric W. Biederman
  2006-09-07 17:18   ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Andrew Morton
  2006-09-07 17:15 ` [PATCH 1/5] proc: Make the generation of the self symlink table driven Andrew Morton
  1 sibling, 2 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-06 16:24 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


To remove the hard coded proc inode numbers it is necessary to be able
to create the proc inodes during readdir.  The instantiate methods are
the subset of lookup that is needed to accomplish that.

This first step just splits the lookup methods into 2 functions.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/proc/base.c |  269 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 158 insertions(+), 111 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 9055918..8c62fe1 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1254,21 +1254,15 @@ static struct dentry_operations tid_fd_d
 	.d_delete	= pid_delete_dentry,
 };
 
-/* SMP-safe */
-static struct dentry *proc_lookupfd(struct inode * dir, struct dentry * dentry, struct nameidata *nd)
+static struct dentry *proc_fd_instantiate(struct inode *dir,
+	struct dentry *dentry, struct task_struct *task, void *ptr)
 {
-	struct task_struct *task = get_proc_task(dir);
-	unsigned fd = name_to_int(dentry);
-	struct dentry *result = ERR_PTR(-ENOENT);
-	struct file * file;
-	struct files_struct * files;
-	struct inode *inode;
-	struct proc_inode *ei;
-
-	if (!task)
-		goto out_no_task;
-	if (fd == ~0U)
-		goto out;
+	unsigned fd = *(unsigned *)ptr;
+	struct file *file;
+	struct files_struct *files;
+ 	struct inode *inode;
+ 	struct proc_inode *ei;
+	struct dentry *error = ERR_PTR(-ENOENT);
 
 	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TID_FD_DIR+fd);
 	if (!inode)
@@ -1277,7 +1271,7 @@ static struct dentry *proc_lookupfd(stru
 	ei->fd = fd;
 	files = get_files_struct(task);
 	if (!files)
-		goto out_unlock;
+		goto out_iput;
 	inode->i_mode = S_IFLNK;
 
 	/*
@@ -1287,13 +1281,14 @@ static struct dentry *proc_lookupfd(stru
 	spin_lock(&files->file_lock);
 	file = fcheck_files(files, fd);
 	if (!file)
-		goto out_unlock2;
+		goto out_unlock;
 	if (file->f_mode & 1)
 		inode->i_mode |= S_IRUSR | S_IXUSR;
 	if (file->f_mode & 2)
 		inode->i_mode |= S_IWUSR | S_IXUSR;
 	spin_unlock(&files->file_lock);
 	put_files_struct(files);
+
 	inode->i_op = &proc_pid_link_inode_operations;
 	inode->i_size = 64;
 	ei->op.proc_get_link = proc_fd_link;
@@ -1301,20 +1296,37 @@ static struct dentry *proc_lookupfd(stru
 	d_add(dentry, inode);
 	/* Close the race of the process dying before we return the dentry */
 	if (tid_fd_revalidate(dentry, NULL))
-		result = NULL;
-out:
-	put_task_struct(task);
-out_no_task:
-	return result;
+		error = NULL;
 
-out_unlock2:
+ out:
+	return error;
+out_unlock:
 	spin_unlock(&files->file_lock);
 	put_files_struct(files);
-out_unlock:
+out_iput:
 	iput(inode);
 	goto out;
 }
 
+/* SMP-safe */
+static struct dentry *proc_lookupfd(struct inode * dir, struct dentry * dentry, struct nameidata *nd)
+{
+	struct task_struct *task = get_proc_task(dir);
+	unsigned fd = name_to_int(dentry);
+	struct dentry *result = ERR_PTR(-ENOENT);
+
+	if (!task)
+		goto out_no_task;
+	if (fd == ~0U)
+		goto out;
+
+	result = proc_fd_instantiate(dir, dentry, task, &fd);
+out:
+	put_task_struct(task);
+out_no_task:
+	return result;
+}
+
 static int proc_readfd(struct file * filp, void * dirent, filldir_t filldir)
 {
 	struct dentry *dentry = filp->f_dentry;
@@ -1395,6 +1407,36 @@ static struct inode_operations proc_fd_i
 	.setattr	= proc_setattr,
 };
 
+static struct dentry *proc_pident_instantiate(struct inode *dir,
+	struct dentry *dentry, struct task_struct *task, void *ptr)
+{
+	struct pid_entry *p = ptr;
+	struct inode *inode;
+	struct proc_inode *ei;
+	struct dentry *error = ERR_PTR(-EINVAL);
+
+	inode = proc_pid_make_inode(dir->i_sb, task, p->type);
+	if (!inode)
+		goto out;
+
+	ei = PROC_I(inode);
+	inode->i_mode = p->mode;
+	if (S_ISDIR(inode->i_mode))
+		inode->i_nlink = 2;	/* Use getattr to fix if necessary */
+	if (p->iop)
+		inode->i_op = p->iop;
+	if (p->fop)
+		inode->i_fop = p->fop;
+	ei->op = p->op;
+	dentry->d_op = &pid_dentry_operations;
+	d_add(dentry, inode);
+	/* Close the race of the process dying before we return the dentry */
+	if (pid_revalidate(dentry, NULL))
+		error = NULL;
+out:
+	return error;
+}
+
 /* SMP-safe */
 static struct dentry *proc_pident_lookup(struct inode *dir, 
 					 struct dentry *dentry,
@@ -1404,7 +1446,6 @@ static struct dentry *proc_pident_lookup
 	struct dentry *error;
 	struct task_struct *task = get_proc_task(dir);
 	struct pid_entry *p;
-	struct proc_inode *ei;
 
 	error = ERR_PTR(-ENOENT);
 	inode = NULL;
@@ -1425,25 +1466,7 @@ static struct dentry *proc_pident_lookup
 	if (!p->name)
 		goto out;
 
-	error = ERR_PTR(-EINVAL);
-	inode = proc_pid_make_inode(dir->i_sb, task, p->type);
-	if (!inode)
-		goto out;
-
-	ei = PROC_I(inode);
-	inode->i_mode = p->mode;
-	if (S_ISDIR(inode->i_mode))
-		inode->i_nlink = 2;	/* Use getattr to fix if necessary */
-	if (p->iop)
-		inode->i_op = p->iop;
-	if (p->fop)
-		inode->i_fop = p->fop;
-	ei->op = p->op;
-	dentry->d_op = &pid_dentry_operations;
-	d_add(dentry, inode);
-	/* Close the race of the process dying before we return the dentry */
-	if (pid_revalidate(dentry, NULL))
-		error = NULL;
+	error = proc_pident_instantiate(dir, dentry, task, p);
 out:
 	put_task_struct(task);
 out_no_task:
@@ -1709,29 +1732,13 @@ static struct dentry_operations proc_bas
 	.d_delete	= pid_delete_dentry,
 };
 
-static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry)
+static struct dentry *proc_base_instantiate(struct inode *dir,
+	struct dentry *dentry, struct task_struct *task, void *ptr)
 {
+	struct pid_entry *p = ptr;
 	struct inode *inode;
-	struct dentry *error;
-	struct task_struct *task = get_proc_task(dir);
-	struct pid_entry *p;
 	struct proc_inode *ei;
-
-	error = ERR_PTR(-ENOENT);
-	inode = NULL;
-
-	if (!task)
-		goto out_no_task;
-
-	/* Lookup the directory entry */
-	for (p = proc_base_stuff; p->name; p++) {
-		if (p->len != dentry->d_name.len)
-			continue;
-		if (!memcmp(dentry->d_name.name, p->name, p->len))
-			break;
-	}
-	if (!p->name)
-		goto out;
+	struct dentry *error = ERR_PTR(-EINVAL);
 
 	/* Allocate the inode */
 	error = ERR_PTR(-ENOMEM);
@@ -1767,14 +1774,41 @@ static struct dentry *proc_base_lookup(s
 	d_add(dentry, inode);
 	error = NULL;
 out:
-	put_task_struct(task);
-out_no_task:
 	return error;
 out_iput:
 	iput(inode);
 	goto out;
 }
 
+static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry)
+{
+	struct dentry *error;
+	struct task_struct *task = get_proc_task(dir);
+	struct pid_entry *p;
+
+	error = ERR_PTR(-ENOENT);
+
+	if (!task)
+		goto out_no_task;
+
+	/* Lookup the directory entry */
+	for (p = proc_base_stuff; p->name; p++) {
+		if (p->len != dentry->d_name.len)
+			continue;
+		if (!memcmp(dentry->d_name.name, p->name, p->len))
+			break;
+	}
+	if (!p->name)
+		goto out;
+
+	error = proc_base_instantiate(dir, dentry, task, p);
+
+out:
+	put_task_struct(task);
+out_no_task:
+	return error;
+}
+
 /*
  * Thread groups
  */
@@ -1915,12 +1949,40 @@ out:
 	return;
 }
 
+struct dentry *proc_pid_instantiate(struct inode *dir,
+	struct dentry * dentry, struct task_struct *task, void *ptr)
+{
+	struct dentry *error = ERR_PTR(-ENOENT);
+	struct inode *inode;
+
+	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TGID_INO);
+	if (!inode)
+		goto out;
+
+	inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
+	inode->i_op = &proc_tgid_base_inode_operations;
+	inode->i_fop = &proc_tgid_base_operations;
+	inode->i_flags|=S_IMMUTABLE;
+	inode->i_nlink = 4;
+#ifdef CONFIG_SECURITY
+	inode->i_nlink += 1;
+#endif
+
+	dentry->d_op = &pid_dentry_operations;
+
+	d_add(dentry, inode);
+	/* Close the race of the process dying before we return the dentry */
+	if (pid_revalidate(dentry, NULL))
+		error = NULL;
+out:
+	return error;
+}
+
 /* SMP-safe */
 struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *nd)
 {
 	struct dentry *result = ERR_PTR(-ENOENT);
 	struct task_struct *task;
-	struct inode *inode;
 	unsigned tgid;
 
 	result = proc_base_lookup(dir, dentry);
@@ -1939,28 +2001,7 @@ struct dentry *proc_pid_lookup(struct in
 	if (!task)
 		goto out;
 
-	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TGID_INO);
-	if (!inode)
-		goto out_put_task;
-
-	inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
-	inode->i_op = &proc_tgid_base_inode_operations;
-	inode->i_fop = &proc_tgid_base_operations;
-	inode->i_flags|=S_IMMUTABLE;
-#ifdef CONFIG_SECURITY
-	inode->i_nlink = 5;
-#else
-	inode->i_nlink = 4;
-#endif
-
-	dentry->d_op = &pid_dentry_operations;
-
-	d_add(dentry, inode);
-	/* Close the race of the process dying before we return the dentry */
-	if (pid_revalidate(dentry, NULL))
-		result = NULL;
-
-out_put_task:
+	result = proc_pid_instantiate(dir, dentry, task, NULL);
 	put_task_struct(task);
 out:
 	return result;
@@ -2107,13 +2148,40 @@ static struct inode_operations proc_tid_
 	.setattr	= proc_setattr,
 };
 
+static struct dentry *proc_task_instantiate(struct inode *dir,
+	struct dentry *dentry, struct task_struct *task, void *ptr)
+{
+	struct dentry *error = ERR_PTR(-ENOENT);
+	struct inode *inode;
+	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TID_INO);
+
+	if (!inode)
+		goto out;
+	inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
+	inode->i_op = &proc_tid_base_inode_operations;
+	inode->i_fop = &proc_tid_base_operations;
+	inode->i_flags|=S_IMMUTABLE;
+	inode->i_nlink = 3;
+#ifdef CONFIG_SECURITY
+	inode->i_nlink += 1;
+#endif
+
+	dentry->d_op = &pid_dentry_operations;
+
+	d_add(dentry, inode);
+	/* Close the race of the process dying before we return the dentry */
+	if (pid_revalidate(dentry, NULL))
+		error = NULL;
+out:
+	return error;
+}
+
 /* SMP-safe */
 static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *nd)
 {
 	struct dentry *result = ERR_PTR(-ENOENT);
 	struct task_struct *task;
 	struct task_struct *leader = get_proc_task(dir);
-	struct inode *inode;
 	unsigned tid;
 
 	if (!leader)
@@ -2133,28 +2201,7 @@ static struct dentry *proc_task_lookup(s
 	if (leader->tgid != task->tgid)
 		goto out_drop_task;
 
-	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TID_INO);
-
-
-	if (!inode)
-		goto out_drop_task;
-	inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
-	inode->i_op = &proc_tid_base_inode_operations;
-	inode->i_fop = &proc_tid_base_operations;
-	inode->i_flags|=S_IMMUTABLE;
-#ifdef CONFIG_SECURITY
-	inode->i_nlink = 4;
-#else
-	inode->i_nlink = 3;
-#endif
-
-	dentry->d_op = &pid_dentry_operations;
-
-	d_add(dentry, inode);
-	/* Close the race of the process dying before we return the dentry */
-	if (pid_revalidate(dentry, NULL))
-		result = NULL;
-
+	result = proc_task_instantiate(dir, dentry, task, NULL);
 out_drop_task:
 	put_task_struct(task);
 out:
-- 
1.4.2.rc3.g7e18e-dirty


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/5] proc: Remove the hard coded inode numbers.
  2006-09-06 16:24 ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Eric W. Biederman
@ 2006-09-06 16:27   ` Eric W. Biederman
  2006-09-06 16:28     ` [PATCH 4/5] proc: Merge proc_tid_attr and proc_tgid_attr Eric W. Biederman
  2006-09-07 17:22     ` [PATCH 3/5] proc: Remove the hard coded inode numbers Andrew Morton
  2006-09-07 17:18   ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Andrew Morton
  1 sibling, 2 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-06 16:27 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


The hard coded inode numbers in proc currently limit it's
maintainability, it's flexibility, and what can be done with
the rest of system.  /proc limits pid-max to 32768 on 32 bit
systems it limits fd-max to 32768 on all systems, and placing
the pid in the inode number really gets in the way of implementing
subdirectories of per process information.

Ever since people started adding to the middle of the file type
enumeration we haven't been maintaing the historical inode numbers,
all we have really succeeded in doing is keeping the pid in the proc
inode number.  The pid is already available in the directory name
so no information is lost removing it from the inode number.

So if something in user space cares if we remove the inode number
from the /proc inode it is almost certainly broken.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/proc/base.c |  384 +++++++++++++++++++++++++-------------------------------
 1 files changed, 174 insertions(+), 210 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 8c62fe1..be4d49d 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -84,114 +84,11 @@ #include "internal.h"
  *	in /proc for a task before it execs a suid executable.
  */
 
-/*
- * For hysterical raisins we keep the same inumbers as in the old procfs.
- * Feel free to change the macro below - just keep the range distinct from
- * inumbers of the rest of procfs (currently those are in 0x0000--0xffff).
- * As soon as we'll get a separate superblock we will be able to forget
- * about magical ranges too.
- */
-
-#define fake_ino(pid,ino) (((pid)<<16)|(ino))
-
-enum pid_directory_inos {
-	PROC_TGID_INO = 2,
-	PROC_TGID_TASK,
-	PROC_TGID_STATUS,
-	PROC_TGID_MEM,
-#ifdef CONFIG_SECCOMP
-	PROC_TGID_SECCOMP,
-#endif
-	PROC_TGID_CWD,
-	PROC_TGID_ROOT,
-	PROC_TGID_EXE,
-	PROC_TGID_FD,
-	PROC_TGID_ENVIRON,
-	PROC_TGID_AUXV,
-	PROC_TGID_CMDLINE,
-	PROC_TGID_STAT,
-	PROC_TGID_STATM,
-	PROC_TGID_MAPS,
-	PROC_TGID_NUMA_MAPS,
-	PROC_TGID_MOUNTS,
-	PROC_TGID_MOUNTSTATS,
-	PROC_TGID_WCHAN,
-#ifdef CONFIG_MMU
-	PROC_TGID_SMAPS,
-#endif
-#ifdef CONFIG_SCHEDSTATS
-	PROC_TGID_SCHEDSTAT,
-#endif
-#ifdef CONFIG_CPUSETS
-	PROC_TGID_CPUSET,
-#endif
-#ifdef CONFIG_SECURITY
-	PROC_TGID_ATTR,
-	PROC_TGID_ATTR_CURRENT,
-	PROC_TGID_ATTR_PREV,
-	PROC_TGID_ATTR_EXEC,
-	PROC_TGID_ATTR_FSCREATE,
-	PROC_TGID_ATTR_KEYCREATE,
-	PROC_TGID_ATTR_SOCKCREATE,
-#endif
-#ifdef CONFIG_AUDITSYSCALL
-	PROC_TGID_LOGINUID,
-#endif
-	PROC_TGID_OOM_SCORE,
-	PROC_TGID_OOM_ADJUST,
-	PROC_TID_INO,
-	PROC_TID_STATUS,
-	PROC_TID_MEM,
-#ifdef CONFIG_SECCOMP
-	PROC_TID_SECCOMP,
-#endif
-	PROC_TID_CWD,
-	PROC_TID_ROOT,
-	PROC_TID_EXE,
-	PROC_TID_FD,
-	PROC_TID_ENVIRON,
-	PROC_TID_AUXV,
-	PROC_TID_CMDLINE,
-	PROC_TID_STAT,
-	PROC_TID_STATM,
-	PROC_TID_MAPS,
-	PROC_TID_NUMA_MAPS,
-	PROC_TID_MOUNTS,
-	PROC_TID_MOUNTSTATS,
-	PROC_TID_WCHAN,
-#ifdef CONFIG_MMU
-	PROC_TID_SMAPS,
-#endif
-#ifdef CONFIG_SCHEDSTATS
-	PROC_TID_SCHEDSTAT,
-#endif
-#ifdef CONFIG_CPUSETS
-	PROC_TID_CPUSET,
-#endif
-#ifdef CONFIG_SECURITY
-	PROC_TID_ATTR,
-	PROC_TID_ATTR_CURRENT,
-	PROC_TID_ATTR_PREV,
-	PROC_TID_ATTR_EXEC,
-	PROC_TID_ATTR_FSCREATE,
-	PROC_TID_ATTR_KEYCREATE,
-	PROC_TID_ATTR_SOCKCREATE,
-#endif
-#ifdef CONFIG_AUDITSYSCALL
-	PROC_TID_LOGINUID,
-#endif
-	PROC_TID_OOM_SCORE,
-	PROC_TID_OOM_ADJUST,
-
-	/* Add new entries before this */
-	PROC_TID_FD_DIR = 0x8000,	/* 0x8000-0xffff */
-};
 
 /* Worst case buffer size needed for holding an integer. */
 #define PROC_NUMBUF 10
 
 struct pid_entry {
-	int type;
 	int len;
 	char *name;
 	mode_t mode;
@@ -200,8 +97,7 @@ struct pid_entry {
 	union proc_op op;
 };
 
-#define NOD(TYPE, NAME, MODE, IOP, FOP, OP) {		\
-	.type = (TYPE),					\
+#define NOD(NAME, MODE, IOP, FOP, OP) {			\
 	.len  = sizeof(NAME) - 1,			\
 	.name = (NAME),					\
 	.mode = MODE,					\
@@ -210,19 +106,19 @@ #define NOD(TYPE, NAME, MODE, IOP, FOP, 
 	.op   = OP,					\
 }
 
-#define DIR(TYPE, NAME, MODE, OTYPE)						\
-	NOD(TYPE, NAME, (S_IFDIR|(MODE)),					\
+#define DIR(NAME, MODE, OTYPE)							\
+	NOD(NAME, (S_IFDIR|(MODE)),						\
 		&proc_##OTYPE##_inode_operations, &proc_##OTYPE##_operations,	\
 		{} )
-#define LNK(TYPE, NAME, OTYPE)					\
-	NOD(TYPE, NAME, (S_IFLNK|S_IRWXUGO),			\
+#define LNK(NAME, OTYPE)					\
+	NOD(NAME, (S_IFLNK|S_IRWXUGO),				\
 		&proc_pid_link_inode_operations, NULL,		\
 		{ .proc_get_link = &proc_##OTYPE##_link } )
-#define REG(TYPE, NAME, MODE, OTYPE)			\
-	NOD(TYPE, NAME, (S_IFREG|(MODE)), NULL,		\
+#define REG(NAME, MODE, OTYPE)				\
+	NOD(NAME, (S_IFREG|(MODE)), NULL,		\
 		&proc_##OTYPE##_operations, {})
-#define INF(TYPE, NAME, MODE, OTYPE)			\
-	NOD(TYPE, NAME, (S_IFREG|(MODE)), 		\
+#define INF(NAME, MODE, OTYPE)				\
+	NOD(NAME, (S_IFREG|(MODE)), 			\
 		NULL, &proc_info_file_operations,	\
 		{ .proc_read = &proc_##OTYPE } )
 
@@ -1043,7 +939,7 @@ static int task_dumpable(struct task_str
 }
 
 
-static struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *task, int ino)
+static struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *task)
 {
 	struct inode * inode;
 	struct proc_inode *ei;
@@ -1057,7 +953,6 @@ static struct inode *proc_pid_make_inode
 	/* Common stuff */
 	ei = PROC_I(inode);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_ino = fake_ino(task->pid, ino);
 	inode->i_op = &proc_def_inode_operations;
 
 	/*
@@ -1160,6 +1055,50 @@ static struct dentry_operations pid_dent
 
 /* Lookups */
 
+typedef struct dentry *instantiate_t(struct inode *, struct dentry *, struct task_struct *, void *);
+
+static int proc_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
+	char *name, int len,
+	instantiate_t instantiate, struct task_struct *task, void *ptr)
+{
+	struct dentry *child, *dir = filp->f_dentry;
+	struct inode *inode;
+	struct qstr qname;
+	ino_t ino = 0;
+	unsigned type = DT_UNKNOWN;
+
+	qname.name = name;
+	qname.len  = len;
+	qname.hash = full_name_hash(name, len);
+
+	child = d_lookup(dir, &qname);
+	if (!child) {
+		struct dentry *new;
+		new = d_alloc(dir, &qname);
+		if (new) {
+			child = instantiate(dir->d_inode, new, task, ptr);
+			if (child)
+				dput(new);
+			else
+				child = new;
+		}
+	}
+	if (!child || IS_ERR(child) || !child->d_inode)
+		goto end_instantiate;
+	inode = child->d_inode;
+	if (inode) {
+		ino = inode->i_ino;
+		type = inode->i_mode >> 12;
+	}
+	dput(child);
+end_instantiate:
+	if (!ino)
+		ino = find_inode_number(dir, &qname);
+	if (!ino)
+		ino = 1;
+	return filldir(dirent, name, len, filp->f_pos, ino, type);
+}
+
 static unsigned name_to_int(struct dentry *dentry)
 {
 	const char *name = dentry->d_name.name;
@@ -1264,7 +1203,7 @@ static struct dentry *proc_fd_instantiat
  	struct proc_inode *ei;
 	struct dentry *error = ERR_PTR(-ENOENT);
 
-	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TID_FD_DIR+fd);
+	inode = proc_pid_make_inode(dir->i_sb, task);
 	if (!inode)
 		goto out;
 	ei = PROC_I(inode);
@@ -1327,6 +1266,15 @@ out_no_task:
 	return result;
 }
 
+static int proc_fd_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
+	struct task_struct *task, int fd)
+{
+	char name[PROC_NUMBUF];
+	int len = snprintf(name, sizeof(name), "%d", fd);
+	return proc_fill_cache(filp, dirent, filldir, name, len,
+				proc_fd_instantiate, task, &fd);
+}
+
 static int proc_readfd(struct file * filp, void * dirent, filldir_t filldir)
 {
 	struct dentry *dentry = filp->f_dentry;
@@ -1334,7 +1282,6 @@ static int proc_readfd(struct file * fil
 	struct task_struct *p = get_proc_task(inode);
 	unsigned int fd, tid, ino;
 	int retval;
-	char buf[PROC_NUMBUF];
 	struct files_struct * files;
 	struct fdtable *fdt;
 
@@ -1364,22 +1311,12 @@ static int proc_readfd(struct file * fil
 			for (fd = filp->f_pos-2;
 			     fd < fdt->max_fds;
 			     fd++, filp->f_pos++) {
-				unsigned int i,j;
 
 				if (!fcheck_files(files, fd))
 					continue;
 				rcu_read_unlock();
 
-				j = PROC_NUMBUF;
-				i = fd;
-				do {
-					j--;
-					buf[j] = '0' + (i % 10);
-					i /= 10;
-				} while (i);
-
-				ino = fake_ino(tid, PROC_TID_FD_DIR + fd);
-				if (filldir(dirent, buf+j, PROC_NUMBUF-j, fd+2, ino, DT_LNK) < 0) {
+				if (proc_fd_fill_cache(filp, dirent, filldir, p, fd) < 0) {
 					rcu_read_lock();
 					break;
 				}
@@ -1415,7 +1352,7 @@ static struct dentry *proc_pident_instan
 	struct proc_inode *ei;
 	struct dentry *error = ERR_PTR(-EINVAL);
 
-	inode = proc_pid_make_inode(dir->i_sb, task, p->type);
+	inode = proc_pid_make_inode(dir->i_sb, task);
 	if (!inode)
 		goto out;
 
@@ -1473,6 +1410,13 @@ out_no_task:
 	return error;
 }
 
+static int proc_pident_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
+	struct task_struct *task, struct pid_entry *p)
+{
+	return proc_fill_cache(filp, dirent, filldir, p->name, p->len,
+				proc_pident_instantiate, task, p);
+}
+
 static int proc_pident_readdir(struct file *filp,
 		void *dirent, filldir_t filldir,
 		struct pid_entry *ents, unsigned int nents)
@@ -1488,11 +1432,10 @@ static int proc_pident_readdir(struct fi
 
 	ret = -ENOENT;
 	if (!task)
-		goto out;
+		goto out_no_task;
 
 	ret = 0;
 	pid = task->pid;
-	put_task_struct(task);
 	i = filp->f_pos;
 	switch (i) {
 	case 0:
@@ -1517,8 +1460,7 @@ static int proc_pident_readdir(struct fi
 		}
 		p = ents + i;
 		while (p->name) {
-			if (filldir(dirent, p->name, p->len, filp->f_pos,
-				    fake_ino(pid, p->type), p->mode >> 12) < 0)
+			if (proc_pident_fill_cache(filp, dirent, filldir, task, p) < 0)
 				goto out;
 			filp->f_pos++;
 			p++;
@@ -1527,6 +1469,8 @@ static int proc_pident_readdir(struct fi
 
 	ret = 1;
 out:
+	put_task_struct(task);
+out_no_task:
 	return ret;
 }
 
@@ -1606,21 +1550,21 @@ static struct file_operations proc_pid_a
 };
 
 static struct pid_entry tgid_attr_stuff[] = {
-	REG(PROC_TGID_ATTR_CURRENT,    "current",    S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TGID_ATTR_PREV,       "prev",       S_IRUGO,	      pid_attr),
-	REG(PROC_TGID_ATTR_EXEC,       "exec",       S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TGID_ATTR_FSCREATE,   "fscreate",   S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TGID_ATTR_KEYCREATE,  "keycreate",  S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TGID_ATTR_SOCKCREATE, "sockcreate", S_IRUGO|S_IWUGO, pid_attr),
+	REG("current",    S_IRUGO|S_IWUGO, pid_attr),
+	REG("prev",       S_IRUGO,	   pid_attr),
+	REG("exec",       S_IRUGO|S_IWUGO, pid_attr),
+	REG("fscreate",   S_IRUGO|S_IWUGO, pid_attr),
+	REG("keycreate",  S_IRUGO|S_IWUGO, pid_attr),
+	REG("sockcreate", S_IRUGO|S_IWUGO, pid_attr),
 	{}
 };
 static struct pid_entry tid_attr_stuff[] = {
-	REG(PROC_TID_ATTR_CURRENT,    "current",    S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TID_ATTR_PREV,       "prev",       S_IRUGO,	     pid_attr),
-	REG(PROC_TID_ATTR_EXEC,       "exec",       S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TID_ATTR_FSCREATE,   "fscreate",   S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TID_ATTR_KEYCREATE,  "keycreate",  S_IRUGO|S_IWUGO, pid_attr),
-	REG(PROC_TID_ATTR_SOCKCREATE, "sockcreate", S_IRUGO|S_IWUGO, pid_attr),
+	REG("current",    S_IRUGO|S_IWUGO, pid_attr),
+	REG("prev",       S_IRUGO,	   pid_attr),
+	REG("exec",       S_IRUGO|S_IWUGO, pid_attr),
+	REG("fscreate",   S_IRUGO|S_IWUGO, pid_attr),
+	REG("keycreate",  S_IRUGO|S_IWUGO, pid_attr),
+	REG("sockcreate", S_IRUGO|S_IWUGO, pid_attr),
 	{}
 };
 
@@ -1704,7 +1648,7 @@ static struct inode_operations proc_self
  * describe something that is process related.
  */
 static struct pid_entry proc_base_stuff[] = {
-	NOD(PROC_TGID_INO, 	"self", S_IFLNK|S_IRWXUGO,
+	NOD("self", S_IFLNK|S_IRWXUGO,
 		&proc_self_inode_operations, NULL, {}),
 	{}
 };
@@ -1749,7 +1693,6 @@ static struct dentry *proc_base_instanti
 	/* Initialize the inode */
 	ei = PROC_I(inode);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_ino = fake_ino(0, p->type);
 
 	/*
 	 * grab the reference to the task.
@@ -1809,6 +1752,13 @@ out_no_task:
 	return error;
 }
 
+static int proc_base_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
+	struct task_struct *task, struct pid_entry *p)
+{
+	return proc_fill_cache(filp, dirent, filldir, p->name, p->len,
+				proc_base_instantiate, task, p);
+}
+
 /*
  * Thread groups
  */
@@ -1816,46 +1766,46 @@ static struct file_operations proc_task_
 static struct inode_operations proc_task_inode_operations;
 
 static struct pid_entry tgid_base_stuff[] = {
-	DIR(PROC_TGID_TASK,       "task",       S_IRUGO|S_IXUGO, task),
-	DIR(PROC_TGID_FD,         "fd",         S_IRUSR|S_IXUSR, fd),
-	INF(PROC_TGID_ENVIRON,    "environ",    S_IRUSR, pid_environ),
-	INF(PROC_TGID_AUXV,       "auxv",       S_IRUSR, pid_auxv),
-	INF(PROC_TGID_STATUS,     "status",     S_IRUGO, pid_status),
-	INF(PROC_TGID_CMDLINE,    "cmdline",    S_IRUGO, pid_cmdline),
-	INF(PROC_TGID_STAT,       "stat",       S_IRUGO, tgid_stat),
-	INF(PROC_TGID_STATM,      "statm",      S_IRUGO, pid_statm),
-	REG(PROC_TGID_MAPS,       "maps",       S_IRUGO, maps),
+	DIR("task",       S_IRUGO|S_IXUGO, task),
+	DIR("fd",         S_IRUSR|S_IXUSR, fd),
+	INF("environ",    S_IRUSR, pid_environ),
+	INF("auxv",       S_IRUSR, pid_auxv),
+	INF("status",     S_IRUGO, pid_status),
+	INF("cmdline",    S_IRUGO, pid_cmdline),
+	INF("stat",       S_IRUGO, tgid_stat),
+	INF("statm",      S_IRUGO, pid_statm),
+	REG("maps",       S_IRUGO, maps),
 #ifdef CONFIG_NUMA
-	REG(PROC_TGID_NUMA_MAPS,  "numa_maps",  S_IRUGO, numa_maps),
+	REG("numa_maps",  S_IRUGO, numa_maps),
 #endif
-	REG(PROC_TGID_MEM,        "mem",        S_IRUSR|S_IWUSR, mem),
+	REG("mem",        S_IRUSR|S_IWUSR, mem),
 #ifdef CONFIG_SECCOMP
-	REG(PROC_TGID_SECCOMP,    "seccomp",    S_IRUSR|S_IWUSR, seccomp),
+	REG("seccomp",    S_IRUSR|S_IWUSR, seccomp),
 #endif
-	LNK(PROC_TGID_CWD,        "cwd",        cwd),
-	LNK(PROC_TGID_ROOT,       "root",       root),
-	LNK(PROC_TGID_EXE,        "exe",        exe),
-	REG(PROC_TGID_MOUNTS,     "mounts",     S_IRUGO, mounts),
-	REG(PROC_TGID_MOUNTSTATS, "mountstats", S_IRUSR, mountstats),
+	LNK("cwd",        cwd),
+	LNK("root",       root),
+	LNK("exe",        exe),
+	REG("mounts",     S_IRUGO, mounts),
+	REG("mountstats", S_IRUSR, mountstats),
 #ifdef CONFIG_MMU
-	REG(PROC_TGID_SMAPS,      "smaps",      S_IRUGO, smaps),
+	REG("smaps",      S_IRUGO, smaps),
 #endif
 #ifdef CONFIG_SECURITY
-	DIR(PROC_TGID_ATTR,       "attr",       S_IRUGO|S_IXUGO, tgid_attr),
+	DIR("attr",       S_IRUGO|S_IXUGO, tgid_attr),
 #endif
 #ifdef CONFIG_KALLSYMS
-	INF(PROC_TGID_WCHAN,      "wchan",      S_IRUGO, pid_wchan),
+	INF("wchan",      S_IRUGO, pid_wchan),
 #endif
 #ifdef CONFIG_SCHEDSTATS
-	INF(PROC_TGID_SCHEDSTAT,  "schedstat",  S_IRUGO, pid_schedstat),
+	INF("schedstat",  S_IRUGO, pid_schedstat),
 #endif
 #ifdef CONFIG_CPUSETS
-	REG(PROC_TGID_CPUSET,     "cpuset",     S_IRUGO, cpuset),
+	REG("cpuset",     S_IRUGO, cpuset),
 #endif
-	INF(PROC_TGID_OOM_SCORE,  "oom_score",  S_IRUGO, oom_score),
-	REG(PROC_TGID_OOM_ADJUST, "oom_adj",    S_IRUGO|S_IWUSR, oom_adjust),
+	INF("oom_score",  S_IRUGO, oom_score),
+	REG("oom_adj",    S_IRUGO|S_IWUSR, oom_adjust),
 #ifdef CONFIG_AUDITSYSCALL
-	REG(PROC_TGID_LOGINUID,   "loginuid",   S_IWUSR|S_IRUGO, loginuid),
+	REG("loginuid",   S_IWUSR|S_IRUGO, loginuid),
 #endif
 	{}
 };
@@ -1955,7 +1905,7 @@ struct dentry *proc_pid_instantiate(stru
 	struct dentry *error = ERR_PTR(-ENOENT);
 	struct inode *inode;
 
-	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TGID_INO);
+	inode = proc_pid_make_inode(dir->i_sb, task);
 	if (!inode)
 		goto out;
 
@@ -2045,18 +1995,29 @@ retry:
 
 #define TGID_OFFSET (FIRST_PROCESS_ENTRY + (1 /* /proc/self */))
 
+static int proc_pid_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
+	struct task_struct *task, int tgid)
+{
+	char name[PROC_NUMBUF];
+	int len = snprintf(name, sizeof(name), "%d", tgid);
+	return proc_fill_cache(filp, dirent, filldir, name, len,
+				proc_pid_instantiate, task, NULL);
+}
+
 /* for the /proc/ directory itself, after non-process stuff has been done */
 int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
 {
-	char buf[PROC_NUMBUF];
 	unsigned int nr = filp->f_pos - FIRST_PROCESS_ENTRY;
+	struct task_struct *reaper = get_proc_task(filp->f_dentry->d_inode);
 	struct task_struct *task;
 	int tgid;
 
+	if (!reaper)
+		goto out_no_task;
+
 	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
 		struct pid_entry *p = &proc_base_stuff[nr];
-		if (filldir(dirent, p->name, p->len, filp->f_pos,
-			    fake_ino(0, p->type), p->mode >> 12) < 0)
+		if (proc_base_fill_cache(filp, dirent, filldir, reaper, p) < 0)
 			goto out;
 	}
 
@@ -2064,19 +2025,17 @@ int proc_pid_readdir(struct file * filp,
 	for (task = next_tgid(tgid);
 	     task;
 	     put_task_struct(task), task = next_tgid(tgid + 1)) {
-		int len;
-		ino_t ino;
 		tgid = task->pid;
 		filp->f_pos = tgid + TGID_OFFSET;
-		len = snprintf(buf, sizeof(buf), "%d", tgid);
-		ino = fake_ino(tgid, PROC_TGID_INO);
-		if (filldir(dirent, buf, len, filp->f_pos, ino, DT_DIR) < 0) {
+		if (proc_pid_fill_cache(filp, dirent, filldir, task, tgid) < 0) {
 			put_task_struct(task);
 			goto out;
 		}
 	}
 	filp->f_pos = PID_MAX_LIMIT + TGID_OFFSET;
 out:
+	put_task_struct(reaper);
+out_no_task:
 	return 0;
 }
 
@@ -2084,44 +2043,44 @@ out:
  * Tasks
  */
 static struct pid_entry tid_base_stuff[] = {
-	DIR(PROC_TID_FD,         "fd",        S_IRUSR|S_IXUSR, fd),
-	INF(PROC_TID_ENVIRON,    "environ",   S_IRUSR, pid_environ),
-	INF(PROC_TID_AUXV,       "auxv",      S_IRUSR, pid_auxv),
-	INF(PROC_TID_STATUS,     "status",    S_IRUGO, pid_status),
-	INF(PROC_TID_CMDLINE,    "cmdline",   S_IRUGO, pid_cmdline),
-	INF(PROC_TID_STAT,       "stat",      S_IRUGO, tid_stat),
-	INF(PROC_TID_STATM,      "statm",     S_IRUGO, pid_statm),
-	REG(PROC_TID_MAPS,       "maps",      S_IRUGO, maps),
+	DIR("fd",        S_IRUSR|S_IXUSR, fd),
+	INF("environ",   S_IRUSR, pid_environ),
+	INF("auxv",      S_IRUSR, pid_auxv),
+	INF("status",    S_IRUGO, pid_status),
+	INF("cmdline",   S_IRUGO, pid_cmdline),
+	INF("stat",      S_IRUGO, tid_stat),
+	INF("statm",     S_IRUGO, pid_statm),
+	REG("maps",      S_IRUGO, maps),
 #ifdef CONFIG_NUMA
-	REG(PROC_TID_NUMA_MAPS,  "numa_maps", S_IRUGO, numa_maps),
+	REG("numa_maps", S_IRUGO, numa_maps),
 #endif
-	REG(PROC_TID_MEM,        "mem",       S_IRUSR|S_IWUSR, mem),
+	REG("mem",       S_IRUSR|S_IWUSR, mem),
 #ifdef CONFIG_SECCOMP
-	REG(PROC_TID_SECCOMP,    "seccomp",   S_IRUSR|S_IWUSR, seccomp),
+	REG("seccomp",   S_IRUSR|S_IWUSR, seccomp),
 #endif
-	LNK(PROC_TID_CWD,        "cwd",       cwd),
-	LNK(PROC_TID_ROOT,       "root",      root),
-	LNK(PROC_TID_EXE,        "exe",       exe),
-	REG(PROC_TID_MOUNTS,     "mounts",    S_IRUGO, mounts),
+	LNK("cwd",       cwd),
+	LNK("root",      root),
+	LNK("exe",       exe),
+	REG("mounts",    S_IRUGO, mounts),
 #ifdef CONFIG_MMU
-	REG(PROC_TID_SMAPS,      "smaps",     S_IRUGO, smaps),
+	REG("smaps",     S_IRUGO, smaps),
 #endif
 #ifdef CONFIG_SECURITY
-	DIR(PROC_TID_ATTR,       "attr",      S_IRUGO|S_IXUGO, tid_attr),
+	DIR("attr",      S_IRUGO|S_IXUGO, tid_attr),
 #endif
 #ifdef CONFIG_KALLSYMS
-	INF(PROC_TID_WCHAN,      "wchan",     S_IRUGO, pid_wchan),
+	INF("wchan",     S_IRUGO, pid_wchan),
 #endif
 #ifdef CONFIG_SCHEDSTATS
-	INF(PROC_TID_SCHEDSTAT,  "schedstat", S_IRUGO, pid_schedstat),
+	INF("schedstat", S_IRUGO, pid_schedstat),
 #endif
 #ifdef CONFIG_CPUSETS
-	REG(PROC_TID_CPUSET,     "cpuset",    S_IRUGO, cpuset),
+	REG("cpuset",    S_IRUGO, cpuset),
 #endif
-	INF(PROC_TID_OOM_SCORE,  "oom_score", S_IRUGO, oom_score),
-	REG(PROC_TID_OOM_ADJUST, "oom_adj",   S_IRUGO|S_IWUSR, oom_adjust),
+	INF("oom_score", S_IRUGO, oom_score),
+	REG("oom_adj",   S_IRUGO|S_IWUSR, oom_adjust),
 #ifdef CONFIG_AUDITSYSCALL
-	REG(PROC_TID_LOGINUID,   "loginuid",  S_IWUSR|S_IRUGO, loginuid),
+	REG("loginuid",  S_IWUSR|S_IRUGO, loginuid),
 #endif
 	{}
 };
@@ -2153,7 +2112,7 @@ static struct dentry *proc_task_instanti
 {
 	struct dentry *error = ERR_PTR(-ENOENT);
 	struct inode *inode;
-	inode = proc_pid_make_inode(dir->i_sb, task, PROC_TID_INO);
+	inode = proc_pid_make_inode(dir->i_sb, task);
 
 	if (!inode)
 		goto out;
@@ -2279,10 +2238,18 @@ static struct task_struct *next_tid(stru
 	return pos;
 }
 
+static int proc_task_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
+	struct task_struct *task, int tid)
+{
+	char name[PROC_NUMBUF];
+	int len = snprintf(name, sizeof(name), "%d", tid);
+	return proc_fill_cache(filp, dirent, filldir, name, len,
+				proc_task_instantiate, task, NULL);
+}
+
 /* for the /proc/TGID/task/ directories */
 static int proc_task_readdir(struct file * filp, void * dirent, filldir_t filldir)
 {
-	char buf[PROC_NUMBUF];
 	struct dentry *dentry = filp->f_dentry;
 	struct inode *inode = dentry->d_inode;
 	struct task_struct *leader = get_proc_task(inode);
@@ -2319,11 +2286,8 @@ static int proc_task_readdir(struct file
 	for (task = first_tid(leader, tid, pos - 2);
 	     task;
 	     task = next_tid(task), pos++) {
-		int len;
 		tid = task->pid;
-		len = snprintf(buf, sizeof(buf), "%d", tid);
-		ino = fake_ino(tid, PROC_TID_INO);
-		if (filldir(dirent, buf, len, pos, ino, DT_DIR < 0)) {
+		if (proc_task_fill_cache(filp, dirent, filldir, task, tid) < 0) {
 			/* returning this tgid failed, save it as the first
 			 * pid for the next readir call */
 			filp->f_version = tid;
-- 
1.4.2.rc3.g7e18e-dirty


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 4/5] proc: Merge proc_tid_attr and proc_tgid_attr
  2006-09-06 16:27   ` [PATCH 3/5] proc: Remove the hard coded inode numbers Eric W. Biederman
@ 2006-09-06 16:28     ` Eric W. Biederman
  2006-09-06 16:31       ` [PATCH 5/5] proc: Use pid_task instead of open coding it Eric W. Biederman
  2006-09-07 17:22     ` [PATCH 3/5] proc: Remove the hard coded inode numbers Andrew Morton
  1 sibling, 1 reply; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-06 16:28 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


The implementation is exactly the same and there is currently nothing to
distinguish proc_tid_attr, and proc_tgid_attr.   So it is pointless
to have two separate implementations.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/proc/base.c |   54 +++++++++++-------------------------------------------
 1 files changed, 11 insertions(+), 43 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index be4d49d..5500ff6 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1549,16 +1549,7 @@ static struct file_operations proc_pid_a
 	.write		= proc_pid_attr_write,
 };
 
-static struct pid_entry tgid_attr_stuff[] = {
-	REG("current",    S_IRUGO|S_IWUGO, pid_attr),
-	REG("prev",       S_IRUGO,	   pid_attr),
-	REG("exec",       S_IRUGO|S_IWUGO, pid_attr),
-	REG("fscreate",   S_IRUGO|S_IWUGO, pid_attr),
-	REG("keycreate",  S_IRUGO|S_IWUGO, pid_attr),
-	REG("sockcreate", S_IRUGO|S_IWUGO, pid_attr),
-	{}
-};
-static struct pid_entry tid_attr_stuff[] = {
+static struct pid_entry attr_dir_stuff[] = {
 	REG("current",    S_IRUGO|S_IWUGO, pid_attr),
 	REG("prev",       S_IRUGO,	   pid_attr),
 	REG("exec",       S_IRUGO|S_IWUGO, pid_attr),
@@ -1568,53 +1559,30 @@ static struct pid_entry tid_attr_stuff[]
 	{}
 };
 
-static int proc_tgid_attr_readdir(struct file * filp,
+static int proc_attr_dir_readdir(struct file * filp,
 			     void * dirent, filldir_t filldir)
 {
 	return proc_pident_readdir(filp,dirent,filldir,
-				   tgid_attr_stuff,ARRAY_SIZE(tgid_attr_stuff));
+				   attr_dir_stuff,ARRAY_SIZE(attr_dir_stuff));
 }
 
-static int proc_tid_attr_readdir(struct file * filp,
-			     void * dirent, filldir_t filldir)
-{
-	return proc_pident_readdir(filp,dirent,filldir,
-				   tid_attr_stuff,ARRAY_SIZE(tid_attr_stuff));
-}
-
-static struct file_operations proc_tgid_attr_operations = {
-	.read		= generic_read_dir,
-	.readdir	= proc_tgid_attr_readdir,
-};
-
-static struct file_operations proc_tid_attr_operations = {
+static struct file_operations proc_attr_dir_operations = {
 	.read		= generic_read_dir,
-	.readdir	= proc_tid_attr_readdir,
+	.readdir	= proc_attr_dir_readdir,
 };
 
-static struct dentry *proc_tgid_attr_lookup(struct inode *dir,
+static struct dentry *proc_attr_dir_lookup(struct inode *dir,
 				struct dentry *dentry, struct nameidata *nd)
 {
-	return proc_pident_lookup(dir, dentry, tgid_attr_stuff);
+	return proc_pident_lookup(dir, dentry, attr_dir_stuff);
 }
 
-static struct dentry *proc_tid_attr_lookup(struct inode *dir,
-				struct dentry *dentry, struct nameidata *nd)
-{
-	return proc_pident_lookup(dir, dentry, tid_attr_stuff);
-}
-
-static struct inode_operations proc_tgid_attr_inode_operations = {
-	.lookup		= proc_tgid_attr_lookup,
+static struct inode_operations proc_attr_dir_inode_operations = {
+	.lookup		= proc_attr_dir_lookup,
 	.getattr	= pid_getattr,
 	.setattr	= proc_setattr,
 };
 
-static struct inode_operations proc_tid_attr_inode_operations = {
-	.lookup		= proc_tid_attr_lookup,
-	.getattr	= pid_getattr,
-	.setattr	= proc_setattr,
-};
 #endif
 
 /*
@@ -1791,7 +1759,7 @@ #ifdef CONFIG_MMU
 	REG("smaps",      S_IRUGO, smaps),
 #endif
 #ifdef CONFIG_SECURITY
-	DIR("attr",       S_IRUGO|S_IXUGO, tgid_attr),
+	DIR("attr",       S_IRUGO|S_IXUGO, attr_dir),
 #endif
 #ifdef CONFIG_KALLSYMS
 	INF("wchan",      S_IRUGO, pid_wchan),
@@ -2066,7 +2034,7 @@ #ifdef CONFIG_MMU
 	REG("smaps",     S_IRUGO, smaps),
 #endif
 #ifdef CONFIG_SECURITY
-	DIR("attr",      S_IRUGO|S_IXUGO, tid_attr),
+	DIR("attr",      S_IRUGO|S_IXUGO, attr_dir),
 #endif
 #ifdef CONFIG_KALLSYMS
 	INF("wchan",     S_IRUGO, pid_wchan),
-- 
1.4.2.rc3.g7e18e-dirty


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 5/5] proc: Use pid_task instead of open coding it
  2006-09-06 16:28     ` [PATCH 4/5] proc: Merge proc_tid_attr and proc_tgid_attr Eric W. Biederman
@ 2006-09-06 16:31       ` Eric W. Biederman
  0 siblings, 0 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-06 16:31 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/proc/base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 5500ff6..5da5f5f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -958,7 +958,7 @@ static struct inode *proc_pid_make_inode
 	/*
 	 * grab the reference to task.
 	 */
-	ei->pid = get_pid(task->pids[PIDTYPE_PID].pid);
+	ei->pid = get_pid(task_pid(task));
 	if (!ei->pid)
 		goto out_unlock;
 
-- 
1.4.2.rc3.g7e18e-dirty


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-06 16:23 [PATCH 1/5] proc: Make the generation of the self symlink table driven Eric W. Biederman
  2006-09-06 16:24 ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Eric W. Biederman
@ 2006-09-07 17:15 ` Andrew Morton
  2006-09-07 18:04   ` Eric W. Biederman
  2006-09-08  7:07   ` Jan Engelhardt
  1 sibling, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2006-09-07 17:15 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel

On Wed, 06 Sep 2006 10:23:00 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> 
> This patch generalizes the concept of files in /proc that are
> related to processes but live in the root directory of /proc
> 
> Ideally this would reuse infrastructure from the rest of the
> process specific parts of proc but unfortunately
> security_task_to_inode must not be called on files that
> are not strictly per process.  security_task_to_inode
> really needs to be reexamined as the security label can
> change in important places that we are not currently
> catching, but I'm not certain that simplifies this problem.
> 
> By at least matching the structure of the rest of proc
> we get more idiom reuse and it becomes easier to spot problems
> in the way things are put together.
> 
> Later things like /proc/mounts are likely to be moved into
> proc_base as well.  If union mounts are ever supported
> we may be able to make /proc a union mount, and properly
> split it into 2 filesystems.
> 
> ..
>
>  /*
> + * proc base
> + *
> + * These are the directory entries in the root directory of /proc
> + * that properly belong to the /proc filesystem, as they describe
> + * describe something that is process related.
> + */
> +static struct pid_entry proc_base_stuff[] = {
> +	NOD(PROC_TGID_INO, 	"self", S_IFLNK|S_IRWXUGO,
> +		&proc_self_inode_operations, NULL, {}),
> +	{}
> +};

We could save a bunch of bytes here.

> +	/* Lookup the directory entry */
> +	for (p = proc_base_stuff; p->name; p++) {

By using ARRAY_SIZE here.

> +	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {

like that does.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/5] proc: Factor out an instantiate method from every lookup method.
  2006-09-06 16:24 ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Eric W. Biederman
  2006-09-06 16:27   ` [PATCH 3/5] proc: Remove the hard coded inode numbers Eric W. Biederman
@ 2006-09-07 17:18   ` Andrew Morton
  2006-09-07 18:08     ` Eric W. Biederman
  1 sibling, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2006-09-07 17:18 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel

On Wed, 06 Sep 2006 10:24:50 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> -/* SMP-safe */
> ...
> +/* SMP-safe */

Not the most useful comment in the kernel, and probably untrue, given that
it's /proc ;)

Please feel free to nuke such silliness sometime.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] proc: Remove the hard coded inode numbers.
  2006-09-06 16:27   ` [PATCH 3/5] proc: Remove the hard coded inode numbers Eric W. Biederman
  2006-09-06 16:28     ` [PATCH 4/5] proc: Merge proc_tid_attr and proc_tgid_attr Eric W. Biederman
@ 2006-09-07 17:22     ` Andrew Morton
  2006-09-07 17:55       ` Eric W. Biederman
  1 sibling, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2006-09-07 17:22 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel

On Wed, 06 Sep 2006 10:27:13 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> +static int proc_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
> +	char *name, int len,
> +	instantiate_t instantiate, struct task_struct *task, void *ptr)
> +{
> +	struct dentry *child, *dir = filp->f_dentry;
> +	struct inode *inode;
> +	struct qstr qname;
> +	ino_t ino = 0;
> +	unsigned type = DT_UNKNOWN;
> +
> +	qname.name = name;
> +	qname.len  = len;
> +	qname.hash = full_name_hash(name, len);
> +
> +	child = d_lookup(dir, &qname);
> +	if (!child) {
> +		struct dentry *new;
> +		new = d_alloc(dir, &qname);
> +		if (new) {
> +			child = instantiate(dir->d_inode, new, task, ptr);
> +			if (child)
> +				dput(new);
> +			else
> +				child = new;
> +		}
> +	}
> +	if (!child || IS_ERR(child) || !child->d_inode)
> +		goto end_instantiate;
> +	inode = child->d_inode;
> +	if (inode) {
> +		ino = inode->i_ino;
> +		type = inode->i_mode >> 12;
> +	}
> +	dput(child);
> +end_instantiate:
> +	if (!ino)
> +		ino = find_inode_number(dir, &qname);
> +	if (!ino)
> +		ino = 1;
> +	return filldir(dirent, name, len, filp->f_pos, ino, type);
> +}

The error handling in here looks rather absent.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] proc: Remove the hard coded inode numbers.
  2006-09-07 17:22     ` [PATCH 3/5] proc: Remove the hard coded inode numbers Andrew Morton
@ 2006-09-07 17:55       ` Eric W. Biederman
  2006-09-07 18:06         ` Andrew Morton
  0 siblings, 1 reply; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-07 17:55 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Andrew Morton <akpm@osdl.org> writes:

> On Wed, 06 Sep 2006 10:27:13 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> +static int proc_fill_cache(struct file *filp, void *dirent, filldir_t
> filldir,
>> +	char *name, int len,
>> +	instantiate_t instantiate, struct task_struct *task, void *ptr)
>> +{
>> +	struct dentry *child, *dir = filp->f_dentry;
>> +	struct inode *inode;
>> +	struct qstr qname;
>> +	ino_t ino = 0;
>> +	unsigned type = DT_UNKNOWN;
>> +
>> +	qname.name = name;
>> +	qname.len  = len;
>> +	qname.hash = full_name_hash(name, len);
>> +
>> +	child = d_lookup(dir, &qname);
>> +	if (!child) {
>> +		struct dentry *new;
>> +		new = d_alloc(dir, &qname);
>> +		if (new) {
>> +			child = instantiate(dir->d_inode, new, task, ptr);
>> +			if (child)
>> +				dput(new);
>> +			else
>> +				child = new;
>> +		}
>> +	}
>> +	if (!child || IS_ERR(child) || !child->d_inode)
>> +		goto end_instantiate;
>> +	inode = child->d_inode;
>> +	if (inode) {
>> +		ino = inode->i_ino;
>> +		type = inode->i_mode >> 12;
>> +	}
>> +	dput(child);
>> +end_instantiate:
>> +	if (!ino)
>> +		ino = find_inode_number(dir, &qname);
>> +	if (!ino)
>> +		ino = 1;
>> +	return filldir(dirent, name, len, filp->f_pos, ino, type);
>> +}
>
> The error handling in here looks rather absent.

Hey, thanks for the review.

I don't think so but a comment or two might be in order.

Calling filldir with the filename is the important part,
and the only real error is if filldir fails.

The rest of the logic is about populating and querying the
dcache so we can find our real inode number, if every reasonable
attempt to perform a dcache lookup fails I simply set the inode
number to 1 and use that in filldir.  It's wrong but at least
I report the file is there.

If I can find the dentry I lookup the inode and the inode
number and file type, and the dput the dentry.

If I can't lookup the dentry I attempt to create it.
instantiate will return a dentry or NULL if the dentry I preallocate
for it is good enough.

Is there something specific you are not seeing?

Eric











^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-07 17:15 ` [PATCH 1/5] proc: Make the generation of the self symlink table driven Andrew Morton
@ 2006-09-07 18:04   ` Eric W. Biederman
  2006-09-08  7:07   ` Jan Engelhardt
  1 sibling, 0 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-07 18:04 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Andrew Morton <akpm@osdl.org> writes:

> On Wed, 06 Sep 2006 10:23:00 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> 
>> This patch generalizes the concept of files in /proc that are
>> related to processes but live in the root directory of /proc
>> 
>> Ideally this would reuse infrastructure from the rest of the
>> process specific parts of proc but unfortunately
>> security_task_to_inode must not be called on files that
>> are not strictly per process.  security_task_to_inode
>> really needs to be reexamined as the security label can
>> change in important places that we are not currently
>> catching, but I'm not certain that simplifies this problem.
>> 
>> By at least matching the structure of the rest of proc
>> we get more idiom reuse and it becomes easier to spot problems
>> in the way things are put together.
>> 
>> Later things like /proc/mounts are likely to be moved into
>> proc_base as well.  If union mounts are ever supported
>> we may be able to make /proc a union mount, and properly
>> split it into 2 filesystems.
>> 
>> ..
>>
>>  /*
>> + * proc base
>> + *
>> + * These are the directory entries in the root directory of /proc
>> + * that properly belong to the /proc filesystem, as they describe
>> + * describe something that is process related.
>> + */
>> +static struct pid_entry proc_base_stuff[] = {
>> +	NOD(PROC_TGID_INO, 	"self", S_IFLNK|S_IRWXUGO,
>> +		&proc_self_inode_operations, NULL, {}),
>> +	{}
>> +};
>
> We could save a bunch of bytes here.
>
>> +	/* Lookup the directory entry */
>> +	for (p = proc_base_stuff; p->name; p++) {
>
> By using ARRAY_SIZE here.
>
>> +	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
>
> like that does.

Agreed.   The problem is that I loose consistency with the other proc
lookup methods.  If it wasn't for the call to security_task_inode I could use
proc_pident_lookup.  Getting some common helpers is still a direction
I want to pursue,  though not as much as I want to kill the hard
coded inode numbers.

Now maybe that means I need to remove the trailing empty entry on
all of the struct pid_entry candidates.

Another part of it is that I would like to convert from an array
of pid_entries into a linked list, eventually so I can have subsystems
that register their proc entries instead of having code that is riddled
with ifdefs.

Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] proc: Remove the hard coded inode numbers.
  2006-09-07 17:55       ` Eric W. Biederman
@ 2006-09-07 18:06         ` Andrew Morton
  2006-09-07 18:37           ` Eric W. Biederman
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2006-09-07 18:06 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel

On Thu, 07 Sep 2006 11:55:59 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> Andrew Morton <akpm@osdl.org> writes:
> 
> > On Wed, 06 Sep 2006 10:27:13 -0600
> > ebiederm@xmission.com (Eric W. Biederman) wrote:
> >
> >> +static int proc_fill_cache(struct file *filp, void *dirent, filldir_t
> > filldir,
> >> +	char *name, int len,
> >> +	instantiate_t instantiate, struct task_struct *task, void *ptr)
> >> +{
> >> +	struct dentry *child, *dir = filp->f_dentry;
> >> +	struct inode *inode;
> >> +	struct qstr qname;
> >> +	ino_t ino = 0;
> >> +	unsigned type = DT_UNKNOWN;
> >> +
> >> +	qname.name = name;
> >> +	qname.len  = len;
> >> +	qname.hash = full_name_hash(name, len);
> >> +
> >> +	child = d_lookup(dir, &qname);
> >> +	if (!child) {
> >> +		struct dentry *new;
> >> +		new = d_alloc(dir, &qname);
> >> +		if (new) {
> >> +			child = instantiate(dir->d_inode, new, task, ptr);
> >> +			if (child)
> >> +				dput(new);
> >> +			else
> >> +				child = new;
> >> +		}
> >> +	}
> >> +	if (!child || IS_ERR(child) || !child->d_inode)
> >> +		goto end_instantiate;
> >> +	inode = child->d_inode;
> >> +	if (inode) {
> >> +		ino = inode->i_ino;
> >> +		type = inode->i_mode >> 12;
> >> +	}
> >> +	dput(child);
> >> +end_instantiate:
> >> +	if (!ino)
> >> +		ino = find_inode_number(dir, &qname);
> >> +	if (!ino)
> >> +		ino = 1;
> >> +	return filldir(dirent, name, len, filp->f_pos, ino, type);
> >> +}
> >
> > The error handling in here looks rather absent.
> 
> Hey, thanks for the review.
> 
> I don't think so but a comment or two might be in order.
> 
> Calling filldir with the filename is the important part,
> and the only real error is if filldir fails.
> 
> The rest of the logic is about populating and querying the
> dcache so we can find our real inode number, if every reasonable
> attempt to perform a dcache lookup fails I simply set the inode
> number to 1 and use that in filldir.  It's wrong but at least
> I report the file is there.
> 
> If I can find the dentry I lookup the inode and the inode
> number and file type, and the dput the dentry.
> 
> If I can't lookup the dentry I attempt to create it.
> instantiate will return a dentry or NULL if the dentry I preallocate
> for it is good enough.

I suspected it was something like that.

> Is there something specific you are not seeing?

Code comments explaining this stuff ;)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/5] proc: Factor out an instantiate method from every lookup method.
  2006-09-07 17:18   ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Andrew Morton
@ 2006-09-07 18:08     ` Eric W. Biederman
  0 siblings, 0 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-07 18:08 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Andrew Morton <akpm@osdl.org> writes:

> On Wed, 06 Sep 2006 10:24:50 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> -/* SMP-safe */
>> ...
>> +/* SMP-safe */
>
> Not the most useful comment in the kernel, and probably untrue, given that
> it's /proc ;)
>
> Please feel free to nuke such silliness sometime.

Sure.

I remember thinking about it at some point and deciding it must have been
a left over from when someone was make /proc SMP safe.

One piece at a time.  I think I am almost done with the heavy lifting,
and the rest of the pieces can start being little cleanups.

Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] proc: Remove the hard coded inode numbers.
  2006-09-07 18:06         ` Andrew Morton
@ 2006-09-07 18:37           ` Eric W. Biederman
  0 siblings, 0 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-07 18:37 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Andrew Morton <akpm@osdl.org> writes:

> On Thu, 07 Sep 2006 11:55:59 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> Hey, thanks for the review.
>> 
>> I don't think so but a comment or two might be in order.
>> 
>> Calling filldir with the filename is the important part,
>> and the only real error is if filldir fails.
>> 
>> The rest of the logic is about populating and querying the
>> dcache so we can find our real inode number, if every reasonable
>> attempt to perform a dcache lookup fails I simply set the inode
>> number to 1 and use that in filldir.  It's wrong but at least
>> I report the file is there.
>> 
>> If I can find the dentry I lookup the inode and the inode
>> number and file type, and the dput the dentry.
>> 
>> If I can't lookup the dentry I attempt to create it.
>> instantiate will return a dentry or NULL if the dentry I preallocate
>> for it is good enough.
>
> I suspected it was something like that.
>
>> Is there something specific you are not seeing?
>
> Code comments explaining this stuff ;)

Sure.  Not every looks at how this is done in filesystems like
smbfs, and fat.

When I get a moment I will see if I can cook up a big fat comment.

Eric






^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-07 17:15 ` [PATCH 1/5] proc: Make the generation of the self symlink table driven Andrew Morton
  2006-09-07 18:04   ` Eric W. Biederman
@ 2006-09-08  7:07   ` Jan Engelhardt
  2006-09-08 11:04     ` Eric W. Biederman
  1 sibling, 1 reply; 19+ messages in thread
From: Jan Engelhardt @ 2006-09-08  7:07 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Eric W. Biederman, linux-kernel


>> +static struct pid_entry proc_base_stuff[] = {
>> +	NOD(PROC_TGID_INO, 	"self", S_IFLNK|S_IRWXUGO,
>> +		&proc_self_inode_operations, NULL, {}),
>> +	{}
>> +};
>
>We could save a bunch of bytes here.
>
>> +	/* Lookup the directory entry */
>> +	for (p = proc_base_stuff; p->name; p++) {
>
>By using ARRAY_SIZE here.
>
>> +	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
>
>like that does.

Also works without the () around ARRAY_SIZE(..)-1



Jan Engelhardt
-- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-08  7:07   ` Jan Engelhardt
@ 2006-09-08 11:04     ` Eric W. Biederman
  2006-09-08 13:08       ` Jan Engelhardt
  0 siblings, 1 reply; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-08 11:04 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Andrew Morton, linux-kernel

Jan Engelhardt <jengelh@linux01.gwdg.de> writes:

>>> +static struct pid_entry proc_base_stuff[] = {
>>> +	NOD(PROC_TGID_INO, 	"self", S_IFLNK|S_IRWXUGO,
>>> +		&proc_self_inode_operations, NULL, {}),
>>> +	{}
>>> +};
>>
>>We could save a bunch of bytes here.
>>
>>> +	/* Lookup the directory entry */
>>> +	for (p = proc_base_stuff; p->name; p++) {
>>
>>By using ARRAY_SIZE here.
>>
>>> +	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
>>
>>like that does.
>
> Also works without the () around ARRAY_SIZE(..)-1

Sure.  But I don't really trust C precedence (because it is wrong)
and having to remember where it is wrong sucks.  Plus in this
case I really am making it clear that ARRAY_SIZE(..)-1 is the concept
I want.   If there would any more to the expression that would
be important.

Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-08 11:04     ` Eric W. Biederman
@ 2006-09-08 13:08       ` Jan Engelhardt
  2006-09-08 13:27         ` Eric W. Biederman
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Engelhardt @ 2006-09-08 13:08 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Andrew Morton, linux-kernel


>>>> +	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
>>>
>> Also works without the () around ARRAY_SIZE(..)-1
>
>Sure.  But I don't really trust C precedence (because it is wrong)

Wrong? In mathematics, "a < (b - 1)" also is equivalent to "a < b - 1".

>and having to remember where it is wrong sucks.  Plus in this
>case I really am making it clear that ARRAY_SIZE(..)-1 is the concept
>I want.   If there would any more to the expression that would
>be important.


Jan Engelhardt
-- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-08 13:08       ` Jan Engelhardt
@ 2006-09-08 13:27         ` Eric W. Biederman
  2006-09-08 16:55           ` Jan Engelhardt
  0 siblings, 1 reply; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-08 13:27 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Andrew Morton, linux-kernel

Jan Engelhardt <jengelh@linux01.gwdg.de> writes:

>>>>> +	for (; nr < (ARRAY_SIZE(proc_base_stuff) - 1); filp->f_pos++, nr++) {
>>>>
>>> Also works without the () around ARRAY_SIZE(..)-1
>>
>>Sure.  But I don't really trust C precedence (because it is wrong)
>
> Wrong? In mathematics, "a < (b - 1)" also is equivalent to "a < b - 1".

In mathematics < is not an operation that yields a result in
the domain of integers.  So  "(a < b) - 1" is impossible.

Regardless this isn't a case where the C precedence is wrong.
"a < b | 1" is an example of C getting the precedence wrong.

Having to remember where C is wrong and in what circumstances is
harder than just putting in parenthesis.

Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-08 13:27         ` Eric W. Biederman
@ 2006-09-08 16:55           ` Jan Engelhardt
  2006-09-09  2:34             ` Eric W. Biederman
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Engelhardt @ 2006-09-08 16:55 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Andrew Morton, linux-kernel


>Regardless this isn't a case where the C precedence is wrong.
>"a < b | 1" is an example of C getting the precedence wrong.

Blame the creator of C.
But maybe this was intended, since | is a logical operation, as is <, 
while + is an arithmetic one. Programmatically probably not making much 
sense, bitfield |= a < b is one use case.

>Having to remember where C is wrong and in what circumstances is
>harder than just putting in parenthesis.

The GNU C compiler will warn you where such may happen, but
currently does so - too bad - only with && and ||.
 c.c:2: warning: suggest parentheses around && within ||


Jan Engelhardt
-- 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] proc: Make the generation of the self symlink table driven.
  2006-09-08 16:55           ` Jan Engelhardt
@ 2006-09-09  2:34             ` Eric W. Biederman
  0 siblings, 0 replies; 19+ messages in thread
From: Eric W. Biederman @ 2006-09-09  2:34 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Andrew Morton, linux-kernel

Jan Engelhardt <jengelh@linux01.gwdg.de> writes:

>>Regardless this isn't a case where the C precedence is wrong.
>>"a < b | 1" is an example of C getting the precedence wrong.
>
> Blame the creator of C.
> But maybe this was intended, since | is a logical operation, as is <, 
> while + is an arithmetic one. Programmatically probably not making much 
> sense, bitfield |= a < b is one use case.

I do.  As I recall the history | and & predate the introduction
of || and && and originally served both functions, so the got
the lower precedence.

>>Having to remember where C is wrong and in what circumstances is
>>harder than just putting in parenthesis.
>
> The GNU C compiler will warn you where such may happen, but
> currently does so - too bad - only with && and ||.
>  c.c:2: warning: suggest parentheses around && within ||

You see my point :)  I have better things to worry about
when writing and reviewing code than remember what the
precedence rules are.

Anyway I have figured out how to remove the need for the - 1,
and the trailing empty entries in proc, patch to follow shortly.

Eric





^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2006-09-09  2:35 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-09-06 16:23 [PATCH 1/5] proc: Make the generation of the self symlink table driven Eric W. Biederman
2006-09-06 16:24 ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Eric W. Biederman
2006-09-06 16:27   ` [PATCH 3/5] proc: Remove the hard coded inode numbers Eric W. Biederman
2006-09-06 16:28     ` [PATCH 4/5] proc: Merge proc_tid_attr and proc_tgid_attr Eric W. Biederman
2006-09-06 16:31       ` [PATCH 5/5] proc: Use pid_task instead of open coding it Eric W. Biederman
2006-09-07 17:22     ` [PATCH 3/5] proc: Remove the hard coded inode numbers Andrew Morton
2006-09-07 17:55       ` Eric W. Biederman
2006-09-07 18:06         ` Andrew Morton
2006-09-07 18:37           ` Eric W. Biederman
2006-09-07 17:18   ` [PATCH 2/5] proc: Factor out an instantiate method from every lookup method Andrew Morton
2006-09-07 18:08     ` Eric W. Biederman
2006-09-07 17:15 ` [PATCH 1/5] proc: Make the generation of the self symlink table driven Andrew Morton
2006-09-07 18:04   ` Eric W. Biederman
2006-09-08  7:07   ` Jan Engelhardt
2006-09-08 11:04     ` Eric W. Biederman
2006-09-08 13:08       ` Jan Engelhardt
2006-09-08 13:27         ` Eric W. Biederman
2006-09-08 16:55           ` Jan Engelhardt
2006-09-09  2:34             ` Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome