linux-mips
[Top] [All Lists]

Re: [PATCH] hugetlb: Provide a default HPAGE_SHIFT if !CONFIG_HUGETLB_PA

To: David Rientjes <rientjes@google.com>
Subject: Re: [PATCH] hugetlb: Provide a default HPAGE_SHIFT if !CONFIG_HUGETLB_PAGE
From: David Daney <ddaney.cavm@gmail.com>
Date: Wed, 16 Nov 2011 15:44:16 -0800
Cc: "ralf@linux-mips.org" <ralf@linux-mips.org>, "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>, Andrew Morton <akpm@linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, David Daney <david.daney@cavium.com>
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=pTY0v1lQzHO22A6TgcBbdCsf/ZZPcW9t/T8sSpy75MU=; b=a2ZTh23oASVll+fV0+FgFHE+wSEGu/uuvAd1JdT6QWh7duGOQKRkkFOtf7v1VbLoLn B+8j9aS4ZFucrwcPPzUx2xtjvsulqpxnaLB15xXyT0RAotBbcLyHCwtbO/Pnd1ncfE2K J75gXmaiUVSC1Fmlst+6BfDmm6lQI9R104UDk=
In-reply-to: <alpine.DEB.2.00.1111161512371.16596@chino.kir.corp.google.com>
References: <1321472611-13283-1-git-send-email-ddaney.cavm@gmail.com> <alpine.DEB.2.00.1111161327180.16596@chino.kir.corp.google.com> <4EC43488.3070400@gmail.com> <alpine.DEB.2.00.1111161512371.16596@chino.kir.corp.google.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10
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@cavium.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.



<Prev in Thread] Current Thread [Next in Thread>