linux-kernel - Re: [PATCH] hugetlb: Provide a default HPAGE_SHIFT if !CONFIG_HUGETLB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 16 Nov 2011 15:44:16 -0800
From:	David Daney <ddaney.cavm@...il.com>
To:	David Rientjes <rientjes@...gle.com>
CC:	"ralf@...ux-mips.org" <ralf@...ux-mips.org>,
	"linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	David Daney <david.daney@...ium.com>
Subject: Re: [PATCH] hugetlb: Provide a default HPAGE_SHIFT if !CONFIG_HUGETLB_PAGE

On 11/16/2011 03:18 PM, David Rientjes wrote:
> 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

>
>>> This is definitely the wrong fix, anyway, and it would require a change to
>>> arch/mips/include/asm/page.h instead since it's localized to mips,
>>
>> No, all we are doing is supplying a dummy definition for HPAGE_SHIFT as we
>> currently have for HPAGE_SIZE and HPAGE_MASK.
>>
>
> 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)


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.

David Daney



>>> so nack.
>>>
>>>> Signed-off-by: David Daney<david.daney@...ium.com>
>>>> ---
>>>>    include/linux/hugetlb.h |    1 +
>>>>    1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>>>> index 19644e0..746d543 100644
>>>> --- a/include/linux/hugetlb.h
>>>> +++ b/include/linux/hugetlb.h
>>>> @@ -113,6 +113,7 @@ static inline void copy_huge_page(struct page *dst,
>>>> struct page *src)
>>>>    #ifndef HPAGE_MASK
>>>>    #define HPAGE_MASK	PAGE_MASK		/* Keep the compiler
>>>> happy */
>>>>    #define HPAGE_SIZE	PAGE_SIZE
>>
>> Why didn't you NACK the addition of these two lines too?
>>
>> Following your logic, we should remove these and patch up all the architecture
>> specific files instead.
>>
>
> I think it's a worthwhile goal to remove these as well, yes.
>

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