On 11/17/2011 03:44 PM, David Rientjes wrote:
On Thu, 17 Nov 2011, Andrew Morton wrote:
So, just remove the dummy and dangerous definitions since they are no
longer needed and reveals the correct dependencies. Tested on
architectures using the definitions with allyesconfig: x86 (even with
thp), hppa, mips, powerpc, s390, sh3, sh4, sparc, and sparc64, and
with defconfig on ia64.
How could arch/mips/mm/tlb-r4k.c:local_flush_tlb_range() compile OK
with this change?
This was tested on Linus' tree, not on Ralf's linux-next tree. All uses
of HPAGE_* are protected by CONFIG_HUGETLB_PAGE as it appropriately should
be in Linus' tree in that file.
What that function is doing looks reasonable to me. Why fill the poor
thing with an ifdef mess?
otoh, catching mistakes is good too. Doing it at runtime as David
proposes is OK.
Nobody else needs it other than Ralf's pending change, and you're
suggesting we need them in a generic header file when any sane arch that
uses hugepages (all of them, in the current tree) declares these
themselves in arch/*/include/asm/page.h where it's supposed to be done?
Why on earth do we have CONFIG_HUGETLB_PAGE for at all, then? To catch
code that's operating on hugepages when our kernel doesn't support it.
I'd much rather break the build than get a runtime BUG() because we want
to avoid an #ifdef or actually write well-written code like every other
arch has! Panicking the code to find errors like this is just insanity.
A counter argument would be:
There are hundreds of places in the kernel where dummy definitions are
selected by !CONFIG_* so that we can do:
if (test_something()) {
do_one_thing();
} else {
do_the_other_thing();
}
Rather than:
#ifdef CONFIG_SOMETHING
if (test_something()) {
do_one_thing();
} else
#else
{
do_the_other_thing();
}
We even do this all over the place with dummy definitions selected by
CONFIG_HUGETLB_PAGE, What exactly makes HPAGE_MASK special and not the
hundreds of other similar situations?
David Daney
