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

[ebiederm@xmission.com (Eric W. Biederman)]

Andrew Morton <akpm@linux-foundation.org> writes:

> On Tue, 3 Jun 2008 15:16:59 +0200 Vegard Nossum <vegard.nossum@gmail.com> wrote:
>
>> 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

I sent a message acking the patch but it seems to have gotten lost.
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>> +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;
>> +}
>
> I'm unable to correlate the code with the comment.  There is nothing in
> here which handles .  and ..?

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

>> @@ -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));
>>  
>
> 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?

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
slowly growing.


> 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...

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