[Devel] [RFC][PATCH] Do not set /proc inode->pid for non-pid-related inodes
Dave Hansen
hansendc at us.ibm.com
Mon Mar 19 15:27:47 PDT 2007
I was tracking down why we need find_get_pid(1) in
proc_get_sb(), when I realized that we apparently
don't need a pid at all in the non-pid parts of /proc.
Anyone see any problems with this approach?
----
For what I would imagine are historical reasons, we set
all struct proc_inode->pid fields. We use the init
process for all non-/proc/<pid> inodes.
We get a handle to the init process in proc_get_sb()
then fetch it out in proc_pid_readdir():
struct task_struct *reaper = get_proc_task(filp->f_path.dentry->d_inode);
The filp in that case is always the root inode on which
someone is doing a readdir. This reaper variable gets
passed down into proc_base_instantiate() and eventually
set in the new inode's ->pid field.
The problem is that I don't see anywhere that we
actually go and use this, outside of the /proc/<pid>
directories. Just referencing the init process like
this is a pain for containers because our init process
(pid == 1) can actually go away.
So, this patch removes all non-pid-dir use of
proc_inode->pid. It puts a WARN_ON() in case anyone
tries to instantiate a proc inode with a pid in a place
we don't expect there to be one.
---
lxc-dave/fs//proc/inode.c | 6 ------
lxc-dave/fs/proc/base.c | 31 ++++++++++---------------------
2 files changed, 10 insertions(+), 27 deletions(-)
diff -puN fs//proc/inode.c~funny-proc-patch fs//proc/inode.c
--- lxc/fs//proc/inode.c~funny-proc-patch 2007-03-19 15:10:50.000000000 -0700
+++ lxc-dave/fs//proc/inode.c 2007-03-19 15:10:50.000000000 -0700
@@ -184,7 +184,6 @@ out_mod:
int proc_fill_super(struct super_block *s, void *data, int silent)
{
struct pid_namespace *pid_ns = data;
- struct proc_inode *ei;
struct inode * root_inode;
s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
@@ -204,11 +203,6 @@ int proc_fill_super(struct super_block *
s->s_root = d_alloc_root(root_inode);
if (!s->s_root)
goto out_no_root;
- /* Seed the root directory with a pid so it doesn't need
- * to be special in base.c.
- */
- ei = PROC_I(root_inode);
- ei->pid = find_get_pid(1);
return 0;
out_no_root:
diff -puN fs//proc/internal.h~funny-proc-patch fs//proc/internal.h
diff -puN fs/proc/base.c~funny-proc-patch fs/proc/base.c
--- lxc/fs/proc/base.c~funny-proc-patch 2007-03-19 15:10:50.000000000 -0700
+++ lxc-dave/fs/proc/base.c 2007-03-19 15:11:40.000000000 -0700
@@ -1171,11 +1171,15 @@ static int pid_revalidate(struct dentry
static int pid_delete_dentry(struct dentry * dentry)
{
+ struct pid *pid;
/* Is the task we represent dead?
* If so, then don't put the dentry on the lru list,
* kill it immediately.
*/
- return !proc_pid(dentry->d_inode)->tasks[PIDTYPE_PID].first;
+ pid = proc_pid(dentry->d_inode);
+ if (!pid)
+ return 0;
+ return !pid->tasks[PIDTYPE_PID].first;
}
static struct dentry_operations pid_dentry_operations =
@@ -1813,6 +1817,7 @@ static struct dentry *proc_base_instanti
struct proc_inode *ei;
struct dentry *error = ERR_PTR(-EINVAL);
+ WARN_ON(task);
/* Allocate the inode */
error = ERR_PTR(-ENOMEM);
inode = new_inode(dir->i_sb);
@@ -1823,13 +1828,6 @@ static struct dentry *proc_base_instanti
ei = PROC_I(inode);
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
- /*
- * grab the reference to the task.
- */
- ei->pid = get_task_pid(task, PIDTYPE_PID);
- if (!ei->pid)
- goto out_iput;
-
inode->i_uid = 0;
inode->i_gid = 0;
inode->i_mode = p->mode;
@@ -1847,9 +1845,6 @@ static struct dentry *proc_base_instanti
error = NULL;
out:
return error;
-out_iput:
- iput(inode);
- goto out;
}
static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry)
@@ -1874,7 +1869,7 @@ static struct dentry *proc_base_lookup(s
if (p > last)
goto out;
- error = proc_base_instantiate(dir, dentry, task, p);
+ error = proc_base_instantiate(dir, dentry, NULL, p);
out:
put_task_struct(task);
@@ -1883,10 +1878,10 @@ out_no_task:
}
static int proc_base_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
- struct task_struct *task, struct pid_entry *p)
+ struct pid_entry *p)
{
return proc_fill_cache(filp, dirent, filldir, p->name, p->len,
- proc_base_instantiate, task, p);
+ proc_base_instantiate, NULL, p);
}
#ifdef CONFIG_TASK_IO_ACCOUNTING
@@ -2197,16 +2192,12 @@ static int proc_pid_fill_cache(struct fi
int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
{
unsigned int nr = filp->f_pos - FIRST_PROCESS_ENTRY;
- struct task_struct *reaper = get_proc_task(filp->f_path.dentry->d_inode);
struct task_struct *task;
int tgid;
- if (!reaper)
- goto out_no_task;
-
for (; nr < ARRAY_SIZE(proc_base_stuff); filp->f_pos++, nr++) {
struct pid_entry *p = &proc_base_stuff[nr];
- if (proc_base_fill_cache(filp, dirent, filldir, reaper, p) < 0)
+ if (proc_base_fill_cache(filp, dirent, filldir, p) < 0)
goto out;
}
@@ -2223,8 +2214,6 @@ int proc_pid_readdir(struct file * filp,
}
filp->f_pos = PID_MAX_LIMIT + TGID_OFFSET;
out:
- put_task_struct(reaper);
-out_no_task:
return 0;
}
diff -puN fs/proc/root.c~funny-proc-patch fs/proc/root.c
_
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
More information about the Devel
mailing list