[Top] [All Lists]

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

To: David Rientjes <>
Subject: Re: [PATCH] hugetlb: Provide a default HPAGE_SHIFT if !CONFIG_HUGETLB_PAGE
From: David Daney <>
Date: Thu, 17 Nov 2011 09:21:14 -0800
Cc: "" <>, "" <>, Andrew Morton <>, "" <>, David Daney <>
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed;; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=6GSA93BZjbkLmvSNoWjUvtr6Zl18D8+iDJ/YTWbY23o=; b=xYwmlFAq6qqbzKdO1OWPn4xSQ5QrJtOO4Kmhe7+e2VSXjExAhs+J8whHNayjzE6Zcy 53CdmnGepmJ9bdwHOSBLY5W0ahLhLAfy7RLFqQuGwqZKsJpdhZVPxltPzmxWlUKY2V9b +jIEAxqDgL7xocIZvyGRWmLwsd6u3fIdBbMqs=
In-reply-to: <>
References: <> <> <> <> <> <> <> <>
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10
On 11/16/2011 08:56 PM, David Rientjes wrote:
On Wed, 16 Nov 2011, David Daney wrote:

It is a well established kernel idiom to supply dummy values for symbols
are required to be defined in order to form a syntactically correct C
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,

Ok, then please tell me why:

1) the dummy version of is_vm_hugetlb_page() exists in hugetlb_inline.h?
2) the dummy version of PageHuge() exists in hugetlb.h

To avoid the #ifdef's,

Good, now we are on the same page.

The answer is not, as first suggested, to sprinkle #ifdefs all over the place, but ...

but you'll notice that they return either NULL, 0,
or are a no-op.  We don't substitute real values in dummy functions where
they could be used in generic code as though they were valid.  That's the
problem with defining HPAGE_SHIFT to be PAGE_SHIFT and, yes, defining
HPAGE_MASK and HPAGE_SIZE as well shouldn't be done and should be removed.

We expect HPAGE_* to represent hugepages, not the native page size of the
bare metal.  This is why we do things like

        #define HPAGE_PMD_SHIFT ({ BUG(); 0; })
        #define HPAGE_PMD_MASK ({ BUG(); 0; })
        #define HPAGE_PMD_SIZE ({ BUG(); 0; })

... rather to fix HPAGE_SHIFT, HPAGE_MASK, and HPAGE_SIZE to be more like these.

I will generate a version of the patch that does this, noting that it the addition of a dummy definition of HPAGE_SHIFT is added to facilitate cleaner architecture specific code for MIPS patches in linux-next.

David Daney

for CONFIG_TRANSPARENT_HUGEPAGE=n.  We don't want to pretend that they're
valid outside the context of hugepages.

We also never use HPAGE_SHIFT, HPAGE_MASK, or HPAGE_SIZE in generic code
that isn't dependent on CONFIG_HUGETLB_PAGE.  If you'd like to submit a
patch to fix this in the mips tree, then that would be good, and if you'd
like to submit a patch to remove these dummy definitions all together by
auditing other arch code, then that would be great.

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