To: Andrew Morton <akpm@linux-foundation.org>
Cc: Amerigo Wang <xiyou.wangcong@gmail.com>, tao.ma@oracle.com,
       linux-kernel@vger.kernel.org, adobriyan@gmail.com,
       mtk.manpages@gmail.com
References: <m11vpqvozl.fsf@fess.ebiederm.org> <20090613040958.GA2959@cr0>
	<m17hzdhkr8.fsf@fess.ebiederm.org>
	<2375c9f90906160829g3d605836yb4c5b9beeac50c5f@mail.gmail.com>
	<m1my88568n.fsf@fess.ebiederm.org>
	<20090618030051.GA6133@cr0.nay.redhat.com>
	<m1tz2ei54r.fsf@fess.ebiederm.org>
	<20090618044055.GB6133@cr0.nay.redhat.com>
	<m1prd2rtdf.fsf@fess.ebiederm.org>
	<20090622085405.GA6499@cr0.nay.redhat.com>
	<20090630100850.GD5873@cr0.nay.redhat.com>
	<20090701144742.6ce3535b.akpm@linux-foundation.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 01 Jul 2009 16:25:05 -0700
In-Reply-To: <20090701144742.6ce3535b.akpm@linux-foundation.org> (Andrew Morton's message of "Wed\, 1 Jul 2009 14\:47\:42 -0700")
Message-ID: <m14otwq94e.fsf@fess.ebiederm.org>
User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [RESEND Patch] kcore: remove its pointless size
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2628
Lines: 72

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

> On Tue, 30 Jun 2009 18:08:50 +0800
> Amerigo Wang <xiyou.wangcong@gmail.com> wrote:
>
>> 
>> Linus fixes wrong size of /proc/kcore problem in commit 9063c61fd5cbd.
>> 
>> But its size still looks insane, since it never equals to the size
>> of physical memory.
>
> Better changelogs, please!
>
> I think that what you're saying is that the stat.st_size field of the
> /proc/kcore inode does not equal the amount of physical memory, and
> that you think it should do so?
>
> If that is correct then it would be appropriate to explain what value
> the stat.st_size field has before the patch and afterwards.  Just
> calling it "insane" isn't optimal.
>
>> Signed-off-by: WANG Cong <amwang@redhat.com>
>> Cc: mtk.manpages@gmail.com
>> 
>> (Andrew, could you please just cut off the kernel part from below? :)
>> 
>> ---
>> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
>> index 59b43a0..eca5201 100644
>> --- a/fs/proc/kcore.c
>> +++ b/fs/proc/kcore.c
>> @@ -405,9 +405,6 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
>>  static int __init proc_kcore_init(void)
>>  {
>>  	proc_root_kcore = proc_create("kcore", S_IRUSR, NULL, &proc_kcore_operations);
>> -	if (proc_root_kcore)
>> -		proc_root_kcore->size =
>> -				(size_t)high_memory - PAGE_OFFSET + PAGE_SIZE;
>>  	return 0;
>>  }
>>  module_init(proc_kcore_init);
>
> AFAICT this means that proc_root_kcore->size will remain uninitialised
> until a process opens and reads from /proc/kcore.  So on initial boot
> the `ls' output will presumably show a size of zero, and this will
> change once /proc/kcore has been read?

Which is better than showing a random number of dubious relationship
to the size we normally show.  That code is just a maintenance problem.

> If so, should we run get_kcore_size() in proc_kcore_init(), perhaps?
>
> In fact, do we need to run get_kcore_size() more than once per boot? 
>
> AFAICT we only run kclist_add() during bootup, so if proc_kcore_init()
> is called at the appropriate time, we can permanently cache its result?
>
> In which case get_kcore_size() and kclist_add() can be marked __init.
>
> Maybe that's all wrong - I didn't look terribly closely.

Memory hot add I expect is the excuse.  There is more that could be
done.  But this patch is an obvious bit of chipping away nonsense
code.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/