Re: [PATCH 9/14] FRV: CONFIG_MMU fixes

[Christoph Hellwig <hch@infradead.org>]

On Wed, Nov 03, 2004 at 03:06:44PM +0000, David Howells wrote:
> 
> > > +++ linux-2.6.10-rc1-bk10-frv/fs/proc/kcore.c	2004-11-01 11:47:04.872656796 +0000
> > > @@ -344,6 +344,7 @@
> > >  		if (m == NULL) {
> > >  			if (clear_user(buffer, tsz))
> > >  				return -EFAULT;
> > > +#ifdef CONFIG_MMU
> >  		} else if ((start >= VMALLOC_START) && (start < VMALLOC_END)) {
> > ...
> > > +#endif
> > 
> > move this into a helper function that can be compiled away for the !MMU case
> 
> That won't work. The else-if clause has to be within the context of the parent
> function in which it now resides.

but you can still have the body of the if clause a noop for !MMU

> It might actually be better to make /proc/kcore conditional on CONFIG_MMU.

Yupp.  In fact I'm pretty sure it can't be selected for m68knommu currently.

> > > +#ifdef CONFIG_MMU
> > >  static struct vmalloc_info get_vmalloc_info(void)
> > ...
> > > +#endif
> > 
> > move the whole function to a CONFIG_MMU-only file
> 
> No. The compiler can, if it wishes, inline this function as it is now. Putting
> it in a separate file removes that option.

Who cares?  This is absolutley not a fastpath.

> > add a small helper for this.  In fact it's the only caller of
> > get_vmalloc_info, so that could be merged into the helper, ala:
> 
> No. The compiler, if it inlines get_vmalloc_info() can avoid allocating a
> whole struct vmalloc_info if it wishes in the current scheme of things, and
> can generate better code by inserting the constants when it needs them.

Dito.

> > I's day just move this out of line into a MMU-only file.
> > ...
> > or at least keep a single MMU ifdef block per file
> 
> I think, perhaps, some of linux/mm.h should perhaps be split out into separate
> MMU and !MMU header files, which then get included as appropriate by
> linux/mm.h.

Probably makes sense.

> > provide stubs please.  pgtable_cache_init is a per-arch things anyway.
> 
> *sigh*. I'm trying to keep the time things take to boot and the amount of
> space the kernel image occupies down, and you're trying to push both back
> up. All these empty stubs consume time and space.

Since when does a noop macro or inline take up space in the kernel image.

> > just move the whole systctl registration into a MMU-only file
> 
> How's that going to help? A few of the VM options may still apply to !MMU.

I meant the registration of the sysctls not relevant for MMU-less systems.