[PPC64] Remove silly debug path from get_vsid()

[PPC64] Remove silly debug path from get_vsid()

[David Gibson]

Andrew, please apply.

Currently the get_vsid() and get_kernel_vsid() functions have a test
which enables a different VSID algorithm for debugging.  Using a dumb
VSID algorithm for stressing the hash table is a reasonable debugging
tool, but switching it at runtime makes no sense at all.  Plus it adds
another test and memory access to the performance critical SLB miss
path.

This patch removes the test, replacing it with a compile time switch.
It seems to make a measurable, although small speedup to the SLB miss
path (maybe 0.5%).

Index: working-2.6/include/asm-ppc64/mmu_context.h
===================================================================
--- working-2.6.orig/include/asm-ppc64/mmu_context.h	2004-05-20 12:58:51.000000000 +1000
+++ working-2.6/include/asm-ppc64/mmu_context.h	2004-05-27 14:22:01.088506616 +1000
@@ -189,15 +189,15 @@
 	ordinal = (((ea >> 28) & 0x1fff) * LAST_USER_CONTEXT) | (ea >> 60);
 	vsid = (ordinal * VSID_RANDOMIZER) & VSID_MASK;
 
-	ifppcdebug(PPCDBG_HTABSTRESS) {
-		/* For debug, this path creates a very poor vsid distribuition.
-		 * A user program can access virtual addresses in the form
-		 * 0x0yyyyxxxx000 where yyyy = xxxx to cause multiple mappings
-		 * to hash to the same page table group.
-		 */ 
-		ordinal = ((ea >> 28) & 0x1fff) | (ea >> 44);
-		vsid = ordinal & VSID_MASK;
-	}
+#ifdef HTABSTRESS
+	/* For debug, this path creates a very poor vsid distribuition.
+	 * A user program can access virtual addresses in the form
+	 * 0x0yyyyxxxx000 where yyyy = xxxx to cause multiple mappings
+	 * to hash to the same page table group.
+	 */ 
+	ordinal = ((ea >> 28) & 0x1fff) | (ea >> 44);
+	vsid = ordinal & VSID_MASK;
+#endif /* HTABSTRESS */
 
 	return vsid;
 } 
@@ -212,11 +212,11 @@
 	ordinal = (((ea >> 28) & 0x1fff) * LAST_USER_CONTEXT) | context;
 	vsid = (ordinal * VSID_RANDOMIZER) & VSID_MASK;
 
-	ifppcdebug(PPCDBG_HTABSTRESS) {
-		/* See comment above. */
-		ordinal = ((ea >> 28) & 0x1fff) | (context << 16);
-		vsid = ordinal & VSID_MASK;
-	}
+#ifdef HTABSTRESS
+	/* See comment above. */
+	ordinal = ((ea >> 28) & 0x1fff) | (context << 16);
+	vsid = ordinal & VSID_MASK;
+#endif /* HTABSTRESS */
 
 	return vsid;
 }

-- 
David Gibson			| For every complex problem there is a
david AT gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Re: [PPC64] Remove silly debug path from get_vsid()

[Andreas Dilger]

On May 28, 2004  13:16 +1000, David Gibson wrote:
> This patch removes the test, replacing it with a compile time switch.
> It seems to make a measurable, although small speedup to the SLB miss
> path (maybe 0.5%).

Looking at include/asm-ppc64/ppcdebug.h the ifppcdebug() macro is defined:

#ifdef CONFIG_PPCDBG
#define ifppcdebug(FLAGS) if (udbg_ifdebug(FLAGS))
#else
#define ifppcdebug(...) if (0)
#endif

so presumably if you don't select CONFIG_PPCDBG it is also a no-op unless
the compiler is broken and doesn't optimize away "if (0)" stanzas.  The
original construct gives you the option to enable this particular debugging
at runtime (without necessarily enabling all debugging at one time) if wanted.

> Index: working-2.6/include/asm-ppc64/mmu_context.h
> ===================================================================
> --- working-2.6.orig/include/asm-ppc64/mmu_context.h	2004-05-20 12:58:51.000000000 +1000
> +++ working-2.6/include/asm-ppc64/mmu_context.h	2004-05-27 14:22:01.088506616 +1000
> @@ -189,15 +189,15 @@
>  	ordinal = (((ea >> 28) & 0x1fff) * LAST_USER_CONTEXT) | (ea >> 60);
>  	vsid = (ordinal * VSID_RANDOMIZER) & VSID_MASK;
>  
> -	ifppcdebug(PPCDBG_HTABSTRESS) {
> -		/* For debug, this path creates a very poor vsid distribuition.
> -		 * A user program can access virtual addresses in the form
> -		 * 0x0yyyyxxxx000 where yyyy = xxxx to cause multiple mappings
> -		 * to hash to the same page table group.
> -		 */ 
> -		ordinal = ((ea >> 28) & 0x1fff) | (ea >> 44);
> -		vsid = ordinal & VSID_MASK;
> -	}
> +#ifdef HTABSTRESS
> +	/* For debug, this path creates a very poor vsid distribuition.
> +	 * A user program can access virtual addresses in the form
> +	 * 0x0yyyyxxxx000 where yyyy = xxxx to cause multiple mappings
> +	 * to hash to the same page table group.
> +	 */ 
> +	ordinal = ((ea >> 28) & 0x1fff) | (ea >> 44);
> +	vsid = ordinal & VSID_MASK;
> +#endif /* HTABSTRESS */
>  
>  	return vsid;
>  } 
> @@ -212,11 +212,11 @@
>  	ordinal = (((ea >> 28) & 0x1fff) * LAST_USER_CONTEXT) | context;
>  	vsid = (ordinal * VSID_RANDOMIZER) & VSID_MASK;
>  
> -	ifppcdebug(PPCDBG_HTABSTRESS) {
> -		/* See comment above. */
> -		ordinal = ((ea >> 28) & 0x1fff) | (context << 16);
> -		vsid = ordinal & VSID_MASK;
> -	}
> +#ifdef HTABSTRESS
> +	/* See comment above. */
> +	ordinal = ((ea >> 28) & 0x1fff) | (context << 16);
> +	vsid = ordinal & VSID_MASK;
> +#endif /* HTABSTRESS */
>  
>  	return vsid;
>  }
> 
> -- 
> David Gibson			| For every complex problem there is a
> david AT gibson.dropbear.id.au	| solution which is simple, neat and
> 				| wrong.
> http://www.ozlabs.org/people/dgibson
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

Re: [PPC64] Remove silly debug path from get_vsid()

[Paul Mackerras]

Andreas Dilger writes:

> so presumably if you don't select CONFIG_PPCDBG it is also a no-op unless
> the compiler is broken and doesn't optimize away "if (0)" stanzas.  The
> original construct gives you the option to enable this particular debugging
> at runtime (without necessarily enabling all debugging at one time) if wanted.

And the effect of suddenly changing your VSID calculation algorithm in
mid-stream is pretty catastrophic.  There is no sensible reason
wanting to be able to turn this on and off at runtime.  This is not
just some extra checking.

Paul.