On Thu, 2010-11-18 at 13:32 +1100, Michael Neuling wrote:
> In: 
>   powerpc/mm: Fix pgtable cache cleanup with CONFIG_PPC_SUBPAGE_PROT
>   commit d28513bc7f675d28b479db666d572e078ecf182d
>   Author: David Gibson <david@gibson.dropbear.id.au>
> 
> subpage_protection() was changed to to take an mm rather a pgdir but it
> didn't change calling site in hashpage_preload().  The change wasn't
> noticed at compile time since hashpage_preload() used a void* as the
> parameter to subpage_protection().
> 
> This is obviously wrong and can trigger the following crash when
> CONFIG_SLAB, CONFIG_DEBUG_SLAB, CONFIG_PPC_64K_PAGES
> CONFIG_PPC_SUBPAGE_PROT are enabled.
> 
...
> 
> The following changes this subpage_protection() call.
> 
> Reported-by: Jim Keniston <jkenisto@linux.vnet.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Tested-by: Jim Keniston <jkenisto@linux.vnet.ibm.com>
> cc: David Gibson <david@gibson.dropbear.id.au>
> cc: stable@kernel.org (only 2.6.33 and newer)
> ---
> > It boots fine with pseries_defconfig.
> > 
> > Attached is the config file that I've been using, which is probably
> > descended over many generations of kernels from some config file in my
> > machine's distant past.  (Enable DEBUG_SLAB or DEBUG_PAGEALLOC and it
> > will fail to boot.)  After I run 'make menuconfig' starting from
> > pseries_defconfig...
> > $ diff config_ok .config | wc -l
> > 2052
> 
> I narrowed this down to adding the following on pseries_defconfig:
>   +CONFIG_SLAB=y
>   +CONFIG_PPC_64K_PAGES=y
>   +CONFIG_PPC_SUBPAGE_PROT=y
>   +CONFIG_DEBUG_SLAB=y
> To reproduce the fail.

I have indeed verified that when I apply those config changes atop
pseries_defconfig, the resulting kernel fails to boot on my Power 5
system without this fix, and boots correctly with this fix.  So the
Tested-by line is correct.

Unfortunately, this fix doesn't enable my legacy configuration to boot
with CONFIG_SLAB=y.  It still fails in the same way -- console output
attached.

FWIW, this failure isn't an obstacle for me.  I'm in no way attached to
my legacy configuration; pseries_defconfig is fine for me.  On the other
hand, I can continue testing fixes, and/or make my system available to
other IBMers when I'm not using it, if you want to continue to pursue
this problem.

Thanks.
Jim

>   
> 
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index 83f534d..5e95844 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -1123,7 +1123,7 @@ void hash_preload(struct mm_struct *mm, unsigned long ea,
>  	else
>  #endif /* CONFIG_PPC_HAS_HASH_64K */
>  		rc = __hash_page_4K(ea, access, vsid, ptep, trap, local, ssize,
> -				    subpage_protection(pgdir, ea));
> +				    subpage_protection(mm, ea));
> 
>  	/* Dump some info in case of hash insertion failure, they should
>  	 * never happen so it is really useful to know if/when they do
> 
>