On Wed, 16 Nov 2011, David Daney wrote:
> > > > > This is required now to get MIPS kernels to compile with
> > > > > !CONFIG_HUGETLB_PAGE.
> > > > >
> > > >
> > > > Why?
> > >
> > > I should have been more specific. The failure is in Ralf's
> > > mips-for-linux-next branch.
> > >
> > I can't find that branch (it's not in Ralf's tree at git.kernel.org), so
> > I'm looking at next-20111116. It doesn't compile for mips for other
> > reasons related to arch/mips/sgi-ip22/ip22-gio.c.
> Ralf's various trees are accessible via linux-mips.org
Wow, we've certainly gone a long way from a patch that appears to be
fixing a problem in Linus' tree based on your commit message. You're
working from ralf/upstream-sfr.git#mips-for-linux-next from
Might want to have mentioned that for a patch labeled "hugetlb".
> > Which is wrong. MIPS code should not be using HPAGE_SHIFT without
> > CONFIG_HUGETLB_PAGE and in fact defines it itself for such a configuration
> > in arch/mips/include/asm/page.h. The only generic uses are in
> > page_alloc.c where we need CONFIG_HUGETLB_PAGE_SIZE_VARIABLE, which isn't
> > available on mips, and in mm/hugetlb.c which requires CONFIG_HUGETLB_PAGE
> > by way of CONFIG_HUGETLBFS.
> > So feel free to show the actual compile error this time and I'll suggest a
> > mips fix for it.
> arch/mips/mm/tlb-r4k.c: In function ‘local_flush_tlb_range’:
> arch/mips/mm/tlb-r4k.c:129:28: error: ‘HPAGE_SHIFT’ undeclared (first use in
> this function)
Do you see where that file has #ifdef's for CONFIG_HUGETLB_PAGE? You need
them here too. The problem is that is_vm_hugetlb_page() is not #define to
0 for CONFIG_HUGETLB_PAGE=n so the clauses using HPAGE_* aren't compiled
out like the author is expecting.
> However, I call B.S. on your reasoning.
> It is a well established kernel idiom to supply dummy values for symbols that
> are required to be defined in order to form a syntactically correct C program,
> but that are known by the programmer to be used only on dead code paths.
> This is exactly what we are doing here.
> To do otherwise requires that code be cluttered with #ifdefery.
You're wrong, nobody should be using HPAGE_SHIFT unless they are working
with hugepages and, in fact, that code that placed those "dummy values"
for HPAGE_MASK and HPAGE_SIZE there has since been removed since 2005.
Defining HPAGE_SHIFT to be PAGE_SHIFT is just stupid and doing so may
allow the program to compile but will hide real bugs later on. In
fact if you merged your patch, it would be a bug since the vma would have
VM_HUGETLB but you'd now be operating on PAGE_SIZE pages!
Like I said, we should be removing those existing definitions of
HPAGE_MASK and HPAGE_SIZE rather than leaving them so someone like you can
come along and pretend there's any legitimacy to them whatsoever and
extend the insanity.
You may not realize it, but changes to include/linux/hugetlb.h go through
Andrew; Ralf won't be merging anything into this generic header file
because of a mips problem.