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

[David Howells]

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

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

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

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

> and define both VMALLOC_START and VMALLOC_END to 0 for !MMU

Most definitely not! What these are is a per-arch decision, and m68knommu at
least has things set up differently. FRV doesn't even define these for !MMU.

> you're missing an endif.

Good point. That ended up in the nommu patch, where this CONFIG_MMU should've
been.

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

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

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

David
-
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/