[-V2,07/26] powerpc: Add size argument to pgtable_cache_add

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

We will use this later with THP changes to request for pmd table of double the size.
THP code does PTE page allocation along with large page request and deposit them
for later use. This is to ensure that we won't have any failures when we split
huge pages to regular pages.

On powerpc we want to use the deposited PTE page for storing hash pte slot and
secondary bit information for the HPTEs. Hence we save them in the second half
of the pmd table.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pgtable-ppc64.h |    7 ++++++-
 arch/powerpc/mm/init_64.c                |    4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

On Wed, Mar 06, 2013 at 11:40:08AM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> 
> We will use this later with THP changes to request for pmd table of double the size.
> THP code does PTE page allocation along with large page request and deposit them
> for later use. This is to ensure that we won't have any failures when we split
> huge pages to regular pages.
> 
> On powerpc we want to use the deposited PTE page for storing hash pte slot and
> secondary bit information for the HPTEs. Hence we save them in the second half
> of the pmd table.

That much is fine, but...

> -void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
> +extern void __pgtable_cache_add(unsigned shift, unsigned long table_size,
> +				void (*ctor)(void *));
>  void pgtable_cache_init(void);
> +static inline void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
> +{
> +	return __pgtable_cache_add(shift, sizeof(void *) << shift, ctor);
> +}

NAK to this (and in fact the rest of this patch).

If you need twice the size, use the next slot along in pgtable_cache,
which will give you the size you need, rather than breaking the
relationship between slot index and allocation size.

Paul.

Paul Mackerras <paulus@samba.org> writes:

> On Wed, Mar 06, 2013 at 11:40:08AM +0530, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> We will use this later with THP changes to request for pmd table of double the size.
>> THP code does PTE page allocation along with large page request and deposit them
>> for later use. This is to ensure that we won't have any failures when we split
>> huge pages to regular pages.
>> 
>> On powerpc we want to use the deposited PTE page for storing hash pte slot and
>> secondary bit information for the HPTEs. Hence we save them in the second half
>> of the pmd table.
>
> That much is fine, but...
>
>> -void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
>> +extern void __pgtable_cache_add(unsigned shift, unsigned long table_size,
>> +				void (*ctor)(void *));
>>  void pgtable_cache_init(void);
>> +static inline void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
>> +{
>> +	return __pgtable_cache_add(shift, sizeof(void *) << shift, ctor);
>> +}
>
> NAK to this (and in fact the rest of this patch).
>
> If you need twice the size, use the next slot along in pgtable_cache,
> which will give you the size you need, rather than breaking the
> relationship between slot index and allocation size.

That would mean we will have to do 

 static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
 {
-	return kmem_cache_alloc(PGT_CACHE(PMD_INDEX_SIZE),
+	return kmem_cache_alloc(PGT_CACHE(PMD_INDEX_SIZE + 1),
 				GFP_KERNEL|__GFP_REPEAT);
 }

is that ok ?

-aneesh

On Wed, Mar 13, 2013 at 02:55:17PM +0530, Aneesh Kumar K.V wrote:
> Paul Mackerras <paulus@samba.org> writes:
> > NAK to this (and in fact the rest of this patch).
> >
> > If you need twice the size, use the next slot along in pgtable_cache,
> > which will give you the size you need, rather than breaking the
> > relationship between slot index and allocation size.
> 
> That would mean we will have to do 
> 
>  static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
>  {
> -	return kmem_cache_alloc(PGT_CACHE(PMD_INDEX_SIZE),
> +	return kmem_cache_alloc(PGT_CACHE(PMD_INDEX_SIZE + 1),
>  				GFP_KERNEL|__GFP_REPEAT);
>  }
> 
> is that ok ?

I would define a symbol such as PMD_CACHE_INDEX and arrange for that
to be either PMD_INDEX_SIZE or PMD_INDEX_SIZE + 1, as needed, and then
use PGT_CACHE(PMD_CACHE_INDEX) everywhere instead of
PGT_CACHE(PMD_INDEX_SIZE).  If you want, you could also do
PUD_CACHE_INDEX and PGD_CACHE_INDEX for consistency, but it's not
totally necessary.

Paul.