Re: [v3,2/3] powerpc: get hugetlbpage handling more generic

On Fri, 2016-11-25 at 09:14 +0100, Christophe LEROY wrote:

Le 24/11/2016 à 06:23, Scott Wood a écrit :

quoted

On Wed, Sep 21, 2016 at 10:11:54AM +0200, Christophe Leroy wrote:

quoted

Today there are two implementations of hugetlbpages which are managed
by exclusive #ifdefs:
* FSL_BOOKE: several directory entries points to the same single
hugepage
* BOOK3S: one upper level directory entry points to a table of hugepages

In preparation of implementation of hugepage support on the 8xx, we
need a mix of the two above solutions, because the 8xx needs both cases
depending on the size of pages:
* In 4k page size mode, each PGD entry covers a 4M bytes area. It means
that 2 PGD entries will be necessary to cover an 8M hugepage while a
single PGD entry will cover 8x 512k hugepages.
* In 16 page size mode, each PGD entry covers a 64M bytes area. It means
that 8x 8M hugepages will be covered by one PGD entry and 64x 512k
hugepages will be covers by one PGD entry.

This patch:
* removes #ifdefs in favor of if/else based on the range sizes
* merges the two huge_pte_alloc() functions as they are pretty similar
* merges the two hugetlbpage_init() functions as they are pretty similar

Signed-off-by: Christophe Leroy <redacted>
Reviewed-by: Aneesh Kumar K.V <redacted>

With this patch on e6500, running the hugetlb testsuite results in the
system hanging in a storm of OOM killer invocations (I'll try to debug
more deeply later).  This patch also changes the default hugepage size on
FSL book3e from 4M to 16M.

Regarding the default hugepage size, it is a result of the merge of the 
two hugetlbpage_init().
Should I add an ifdef to get 4M on FSL book3e by default ?
What's the reason for selecting different hugepage sizes depending on 
the CPU ? 

I'm not sure what the original reason was, but a change in defaults could
disturb existing users.

I thought default size was selected based on what was existing.

...by code that doesn't expect 4M to ever exist.  Both 4M and 16M (and a bunch
of other sizes) are available on FSL book3e.

What is the reason for preferring 16M over 1M, but preferring 1M over 2M?
 Seems arbitrary.

If we want to preserve the exsiting behavior without an ifdef, we could put a
check for 4M before the other sizes, with a comment explaining why we're
making the selection look even more arbitrary.  Or we could try to figure out
what size actually makes a better default.

What testsuite do you run exactly ?

The one that comes with libhugetlbfs (not a particularly recent version, but
not sure exactly how old).

-Scott