[PATCH] proc: calculate the correct /proc/<pid> link count

Previous thread: RE: Submitting fixes for bugs: Re: tun device patch for IPv6 by Zabele, Stephen (US SSA) on Tuesday, June 3, 2008 - 8:26 am. (2 messages)

Next thread: [WARNING] local_bh_enable with irqs disabled: by Guennadi Liakhovetski on Tuesday, June 3, 2008 - 9:32 am. (8 messages)
To: Andrew Morton <akpm@...>, Eric W. Biederman <ebiederm@...>
Cc: Pavel Emelyanov <xemul@...>, <linux-kernel@...>
Date: Tuesday, June 3, 2008 - 9:16 am

From: Vegard Nossum <vegard.nossum@gmail.com>
Date: Mon, 2 Jun 2008 08:57:45 +0200
Subject: [PATCH] proc: calculate the correct /proc/<pid> link count

commit e9720acd728a46cb40daa52c99a979f7c4ff195c introduced a
/proc/self/net directory without bumping the corresponding link count
for /proc/self.

This patch replaces the static link count initializations with a call
that counts the number of directory entries in the given pid_entry
table whenever it is instantiated, and thus relieves the burden of
manually keeping the two in sync.

Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
---
fs/proc/base.c | 31 +++++++++++++++++++++++--------
1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index c447e07..334ce46 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -127,6 +127,23 @@ struct pid_entry {
NULL, &proc_single_file_operations, \
{ .proc_show = &proc_##OTYPE } )

+/* Count the number of hardlinks for the pid_entry table, excluding the .
+ * and .. links. */
+static unsigned int pid_entry_count_dirs(const struct pid_entry *entries,
+ unsigned int n)
+{
+ unsigned int i;
+ unsigned int count;
+
+ count = 0;
+ for (i = 0; i < n; ++i) {
+ if (S_ISDIR(entries[i].mode))
+ ++count;
+ }
+
+ return count;
+}
+
int maps_protect;
EXPORT_SYMBOL(maps_protect);

@@ -2585,10 +2602,9 @@ static struct dentry *proc_pid_instantiate(struct inode *dir,
inode->i_op = &proc_tgid_base_inode_operations;
inode->i_fop = &proc_tgid_base_operations;
inode->i_flags|=S_IMMUTABLE;
- inode->i_nlink = 5;
-#ifdef CONFIG_SECURITY
- inode->i_nlink += 1;
-#endif
+
+ inode->i_nlink = 2 + pid_entry_count_dirs(tgid_base_stuff,
+ ARRAY_SIZE(tgid_base_stuff));

dentry->d_op = &pid_dentry_operations;

@@ -2816,10 +2832,9 @@ static struct dentry *proc_task_ins...

To: Vegard Nossum <vegard.nossum@...>
Cc: Eric W. Biederman <ebiederm@...>, Pavel Emelyanov <xemul@...>, <linux-kernel@...>
Date: Wednesday, June 4, 2008 - 4:40 am

Linus gets upset when we refer to commits only by their hash.

I did this:

This patch:

commit e9720acd728a46cb40daa52c99a979f7c4ff195c
Author: Pavel Emelyanov <xemul@openvz.org>
Date: Fri Mar 7 11:08:40 2008 -0800

[NET]: Make /proc/net a symlink on /proc/self/net (v3)

introduced a /proc/self/net directory without bumping the corresponding

Like this:

/*
* Count the number of hardlinks for the pid_entry table, excluding the .
* and .. links.
*/

I'm unable to correlate the code with the comment. There is nothing in

oh, can we do that? Is it possible for some code somewhere to come
along and add a new entry to /proc/pid which doesn't appear in
these static tables?

I guess that doesn't happen. In which case can we not calculate the
unmber of directories in these two tables just a single time, at
bootup?

I think I'm missing things here...
--

To: Andrew Morton <akpm@...>
Cc: Vegard Nossum <vegard.nossum@...>, Pavel Emelyanov <xemul@...>, <linux-kernel@...>
Date: Wednesday, June 4, 2008 - 6:02 am

I sent a message acking the patch but it seems to have gotten lost.

Because they don't appear in the table. It seems the comment was to make

Currently we do not dynamically modify the pid_entry tables.
On some days I think it would be a nice addition, to make it easier to
handle modular subsystems. Given the number of #ifdefs we have
in those tables something more dynamic may ultimately be the way
to go.

However we still have in lookup:
/*
* Yes, it does not scale. And it should not. Don't add
* new entries into /proc/<tgid>/ without very good reasons.
*/
Which doesn't seem to have much impact as these directories are

Not much.

Historically /proc used to be very bad with the link counts on
directories. There was a switch statement that hard coded nlinks for
every directory, and only used the values 2 or 3. Last time I was in
there I fixed it up so we actually returned the proper hard link
counts for the directories. In this last conversation I realized we
could be more maintainable without a hard coded number.

Eric

--

Previous thread: RE: Submitting fixes for bugs: Re: tun device patch for IPv6 by Zabele, Stephen (US SSA) on Tuesday, June 3, 2008 - 8:26 am. (2 messages)

Next thread: [WARNING] local_bh_enable with irqs disabled: by Guennadi Liakhovetski on Tuesday, June 3, 2008 - 9:32 am. (8 messages)