Re: [RESEND Patch] kcore: remove its pointless size

[Andrew Morton]

On Wed, 01 Jul 2009 16:25:05 -0700 ebiederm@xxxxxxxxxxxx (Eric W. Biederman) wrote:

> Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> writes:
> 
> >> 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.

Well it's not just that st_size is wrong before the first read.  It's
also wrong after memory hot-add, up until the next read.

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

We have the infrastructure to get this right, I think:

- run

	proc_root_kcore->size = get_kcore_size(...)

  within proc_kcore_init()

- register a memory-hotplug notifier and each time memory goes online
  or offline, rerun

	proc_root_kcore->size = get_kcore_size(...)

- stop running get_kcore_size() within read_kcore().

I suspect that read_kcore() will not behave well if a memory hotplug
operation happens concurrently.  But that's a separate problem.

(hopefully cc's some memory-hotplug people)


Or we just leave /proc/kcore's st_size at zero.  It's a pretty hopeless
exercise trying to get this "right", as nobody can safely _use_ that
size - it can be wrong as soon as the caller has read from it.

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