[PATCH -V7 01/18] mm/THP: HPAGE_SHIFT is not a #define on some arch

[David Gibson]

On Tue, Apr 30, 2013 at 09:12:09AM +0530, Aneesh Kumar K.V wrote:
> David Gibson <dwg at au1.ibm.com> writes:
> 
> > On Mon, Apr 29, 2013 at 01:07:22AM +0530, Aneesh Kumar K.V wrote:
> >> From: "Aneesh Kumar K.V" <aneesh.kumar at linux.vnet.ibm.com>
> >> 
> >> On archs like powerpc that support different hugepage sizes, HPAGE_SHIFT
> >> and other derived values like HPAGE_PMD_ORDER are not constants. So move
> >> that to hugepage_init
> >
> > These seems to miss the point.  Those variables may be defined in
> > terms of HPAGE_SHIFT right now, but that is of itself kind of broken.
> > The transparent hugepage mechanism only works if the hugepage size is
> > equal to the PMD size - and PMD_SHIFT remains a compile time constant.
> >
> > There's no reason having transparent hugepage should force the PMD
> > size of hugepage to be the default for other purposes - it should be
> > possible to do THP as long as PMD-sized is a possible hugepage size.
> >
> 
> THP code does
> 
> #define HPAGE_PMD_SHIFT HPAGE_SHIFT
> #define HPAGE_PMD_MASK HPAGE_MASK
> #define HPAGE_PMD_SIZE HPAGE_SIZE
> 
> I had two options, one to move all those in terms of PMD_SHIFT

This is a much better option that you've taken now, and really
shouldn't be that hard.  The THP code is much more strongly tied to
the fact that it is a PMD than the fact that it's the same size as
explicit huge pages.

> or switch
> ppc64 to not use HPAGE_SHIFT the way it use now. Both would involve large
> code changes. Hence I end up moving some of the checks to runtime
> checks. Actual HPAGE_SHIFT == PMD_SHIFT check happens in the has_transparent_hugepage() 
> 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-April/106002.html

And my other point is that this is also wrong.  All you should need to
check is that HPAGE_PMD_SHIFT (== PMD_SHIFT) is a supported hugepage
size, not that it is equal to HPAGE_SHIFT the default explicit
hugepage size.

> IMHO what the patch is checking is that, HPAGE_SHIFT
> value is not resulting in a page order higher than MAX_ORDER. 

Which you don't actually care about in THP - you only care that
HPAGE_PMD_SHIFT doesn't exceed MAX_ORDER.

> Related to Reviewed-by: that came from V5 patchset 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-April/105299.html
> 
> Your review suggestion to move that runtime check back to macro happened
> in V6. I missed dropping reviewed-by after that. 

Ok.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20130430/6b2d32a8/attachment.sig>