[PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[Mikael Pettersson]

Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
written by David Gibson <david@gibson.dropbear.id.au>.
ppc64 is sufficiently different from ppc32 that this driver
is kept separate from my ppc32 driver. This shouldn't matter unless
people actually want to run ppc32 kernels on ppc64 processors.

ppc64 perfctr driver from David Gibson <david@gibson.dropbear.id.au>:
- ppc64 arch hooks: Kconfig, syscalls numbers and tables, task struct,
  and process management ops (switch_to, exit, fork)

Signed-off-by: Mikael Pettersson <mikpe@csd.uu.se>

 arch/ppc64/Kconfig            |    1 +
 arch/ppc64/kernel/misc.S      |   12 ++++++++++++
 arch/ppc64/kernel/process.c   |    6 ++++++
 include/asm-ppc64/processor.h |    2 ++
 include/asm-ppc64/unistd.h    |    8 +++++++-
 5 files changed, 28 insertions(+), 1 deletion(-)

diff -rupN linux-2.6.12-rc1-mm4/arch/ppc64/Kconfig linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/arch/ppc64/Kconfig
--- linux-2.6.12-rc1-mm4/arch/ppc64/Kconfig	2005-03-31 21:08:24.000000000 +0200
+++ linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/arch/ppc64/Kconfig	2005-03-31 23:28:07.000000000 +0200
@@ -297,6 +297,7 @@ config SECCOMP
 
 endmenu
 
+source "drivers/perfctr/Kconfig"
 
 menu "General setup"
 
diff -rupN linux-2.6.12-rc1-mm4/arch/ppc64/kernel/misc.S linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/arch/ppc64/kernel/misc.S
--- linux-2.6.12-rc1-mm4/arch/ppc64/kernel/misc.S	2005-03-31 21:08:24.000000000 +0200
+++ linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/arch/ppc64/kernel/misc.S	2005-03-31 23:28:07.000000000 +0200
@@ -956,6 +956,12 @@ _GLOBAL(sys_call_table32)
 	.llong .sys32_request_key
 	.llong .compat_sys_keyctl
 	.llong .compat_sys_waitid
+	.llong .sys_ni_syscall		/* 273 reserved for sys_ioprio_set */
+	.llong .sys_ni_syscall		/* 274 reserved for sys_ioprio_get */
+	.llong .sys_vperfctr_open	/* 275 */
+	.llong .sys_vperfctr_control
+	.llong .sys_vperfctr_write
+	.llong .sys_vperfctr_read
 
 	.balign 8
 _GLOBAL(sys_call_table)
@@ -1232,3 +1238,9 @@ _GLOBAL(sys_call_table)
 	.llong .sys_request_key		/* 270 */
 	.llong .sys_keyctl
 	.llong .sys_waitid
+	.llong .sys_ni_syscall		/* 273 reserved for sys_ioprio_set */
+	.llong .sys_ni_syscall		/* 274 reserved for sys_ioprio_get */
+	.llong .sys_vperfctr_open	/* 275 */
+	.llong .sys_vperfctr_control
+	.llong .sys_vperfctr_write
+	.llong .sys_vperfctr_read
diff -rupN linux-2.6.12-rc1-mm4/arch/ppc64/kernel/process.c linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/arch/ppc64/kernel/process.c
--- linux-2.6.12-rc1-mm4/arch/ppc64/kernel/process.c	2005-03-31 21:07:46.000000000 +0200
+++ linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/arch/ppc64/kernel/process.c	2005-03-31 23:28:07.000000000 +0200
@@ -36,6 +36,7 @@
 #include <linux/kallsyms.h>
 #include <linux/interrupt.h>
 #include <linux/utsname.h>
+#include <linux/perfctr.h>
 
 #include <asm/pgtable.h>
 #include <asm/uaccess.h>
@@ -225,7 +226,9 @@ struct task_struct *__switch_to(struct t
 
 
 	local_irq_save(flags);
+	perfctr_suspend_thread(&prev->thread);
 	last = _switch(old_thread, new_thread);
+	perfctr_resume_thread(&current->thread);
 
 	local_irq_restore(flags);
 
@@ -323,6 +326,7 @@ void exit_thread(void)
 		last_task_used_altivec = NULL;
 #endif /* CONFIG_ALTIVEC */
 #endif /* CONFIG_SMP */
+	perfctr_exit_thread(&current->thread);
 }
 
 void flush_thread(void)
@@ -425,6 +429,8 @@ copy_thread(int nr, unsigned long clone_
  	 */
 	kregs->nip = *((unsigned long *)ret_from_fork);
 
+	perfctr_copy_task(p, regs);
+
 	return 0;
 }
 
diff -rupN linux-2.6.12-rc1-mm4/include/asm-ppc64/processor.h linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/include/asm-ppc64/processor.h
--- linux-2.6.12-rc1-mm4/include/asm-ppc64/processor.h	2005-03-31 21:08:31.000000000 +0200
+++ linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/include/asm-ppc64/processor.h	2005-03-31 23:28:07.000000000 +0200
@@ -574,6 +574,8 @@ struct thread_struct {
 	unsigned long	vrsave;
 	int		used_vr;	/* set if process has used altivec */
 #endif /* CONFIG_ALTIVEC */
+	/* performance counters */
+	struct vperfctr *perfctr;
 };
 
 #define ARCH_MIN_TASKALIGN 16
diff -rupN linux-2.6.12-rc1-mm4/include/asm-ppc64/unistd.h linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/include/asm-ppc64/unistd.h
--- linux-2.6.12-rc1-mm4/include/asm-ppc64/unistd.h	2005-03-31 21:07:54.000000000 +0200
+++ linux-2.6.12-rc1-mm4.perfctr27-ppc64-arch-hooks/include/asm-ppc64/unistd.h	2005-03-31 23:28:07.000000000 +0200
@@ -283,8 +283,14 @@
 #define __NR_request_key	270
 #define __NR_keyctl		271
 #define __NR_waitid		272
+/* 273 is reserved for ioprio_set */
+/* 274 is reserved for ioprio_get */
+#define __NR_vperfctr_open	275
+#define __NR_vperfctr_control	(__NR_vperfctr_open+1)
+#define __NR_vperfctr_write	(__NR_vperfctr_open+2)
+#define __NR_vperfctr_read	(__NR_vperfctr_open+3)
 
-#define __NR_syscalls		273
+#define __NR_syscalls		279
 #ifdef __KERNEL__
 #define NR_syscalls	__NR_syscalls
 #endif

Re: [PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[Andrew Morton]

Mikael Pettersson <mikpe@csd.uu.se> wrote:
>
> Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> written by David Gibson <david@gibson.dropbear.id.au>.

Well that seems like progress.  Where do we feel that we stand wrt
preparedness for merging all this up?

Re: [PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[David Gibson]

On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
> Mikael Pettersson <mikpe@csd.uu.se> wrote:
> >
> > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> > written by David Gibson <david@gibson.dropbear.id.au>.
> 
> Well that seems like progress.  Where do we feel that we stand wrt
> preparedness for merging all this up?

I'm still uneasy about it.  There were sufficient changes made getting
this one ready to go that I'm not confident there aren't more
important things to be found.

-- 
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/people/dgibson

Re: [PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[Andrew Morton]

David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
> > Mikael Pettersson <mikpe@csd.uu.se> wrote:
> > >
> > > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> > > written by David Gibson <david@gibson.dropbear.id.au>.
> > 
> > Well that seems like progress.  Where do we feel that we stand wrt
> > preparedness for merging all this up?
> 
> I'm still uneasy about it.  There were sufficient changes made getting
> this one ready to go that I'm not confident there aren't more
> important things to be found.

That's a bit open-ended.  How do we determine whether more things will be
needed?  How do we know when we're done?

Re: [PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[Mikael Pettersson]

Andrew Morton writes:
 > David Gibson <david@gibson.dropbear.id.au> wrote:
 > >
 > > On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
 > > > Mikael Pettersson <mikpe@csd.uu.se> wrote:
 > > > >
 > > > > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
 > > > > written by David Gibson <david@gibson.dropbear.id.au>.
 > > > 
 > > > Well that seems like progress.  Where do we feel that we stand wrt
 > > > preparedness for merging all this up?
 > > 
 > > I'm still uneasy about it.  There were sufficient changes made getting
 > > this one ready to go that I'm not confident there aren't more
 > > important things to be found.
 > 
 > That's a bit open-ended.  How do we determine whether more things will be
 > needed?  How do we know when we're done?

I have two planned changes that will be done RSN:
- On x86/x86-64, user-space uses the mmap()ed state's TSC start
  value as a way to detect if a user-space sampling operation
  (which needs to be "virtually atomic") was preempted by the kernel.
  On ppc{32,64} we've used the TB for the same thing up to now,
  but that doesn't quite work because the TB is about a magnitude
  or two too slow. So the plan is to change ppc to store a
  software generation counter in the mmap()ed state, and change
  the ppc user-space to check that one instead.
- Move <asm-$arch/perfctr.h> common stuff to <asm-generic/perfctr.h>.

In addition, there is one unresolved issue:
- A counter's value is represented by a 64-bit software sum,
  a 32-bit start value containing the HW counter's value at the
  start of the current time slice, and the current HW counter's value
  (now). The actual value is computed as sum + (now - start).
  This is reflected in the mmap()ed state, which contains a variable-
  length { u32 map; u32 start; u64 sum; } pmc[] array.
  This layout is very cache-efficient on current 32 and 64-bit CPUs,
  but there is a _possible_ concern that it won't do on 10+ GHz CPUs.
  So the question is, should we change it to use 64-bit start values
  already now (and take more cache misses), or should that wait a few
  years until it becomes a necessity (causing ABI change issues)?

/Mikael

Re: [PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[Andrew Morton]

Mikael Pettersson <mikpe@csd.uu.se> wrote:
>
> In addition, there is one unresolved issue:
>  - A counter's value is represented by a 64-bit software sum,
>    a 32-bit start value containing the HW counter's value at the
>    start of the current time slice, and the current HW counter's value
>    (now). The actual value is computed as sum + (now - start).
>    This is reflected in the mmap()ed state, which contains a variable-
>    length { u32 map; u32 start; u64 sum; } pmc[] array.
>    This layout is very cache-efficient on current 32 and 64-bit CPUs,
>    but there is a _possible_ concern that it won't do on 10+ GHz CPUs.
>    So the question is, should we change it to use 64-bit start values
>    already now (and take more cache misses), or should that wait a few
>    years until it becomes a necessity (causing ABI change issues)?

I'd be inclined to make the change now, personally.  ABI changes are a pain
for everyone.

Re: [PATCH 2.6.12-rc1-mm5 1/3] perfctr: ppc64 arch hooks

[David Gibson]

On Fri, Apr 01, 2005 at 02:46:53PM +0200, Mikael Pettersson wrote:
> Andrew Morton writes:
>  > David Gibson <david@gibson.dropbear.id.au> wrote:
>  > >
>  > > On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
>  > > > Mikael Pettersson <mikpe@csd.uu.se> wrote:
>  > > > >
>  > > > > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
>  > > > > written by David Gibson <david@gibson.dropbear.id.au>.
>  > > > 
>  > > > Well that seems like progress.  Where do we feel that we stand wrt
>  > > > preparedness for merging all this up?
>  > > 
>  > > I'm still uneasy about it.  There were sufficient changes made getting
>  > > this one ready to go that I'm not confident there aren't more
>  > > important things to be found.
>  > 
>  > That's a bit open-ended.  How do we determine whether more things will be
>  > needed?  How do we know when we're done?
> 
> I have two planned changes that will be done RSN:
> - On x86/x86-64, user-space uses the mmap()ed state's TSC start
>   value as a way to detect if a user-space sampling operation
>   (which needs to be "virtually atomic") was preempted by the kernel.
>   On ppc{32,64} we've used the TB for the same thing up to now,
>   but that doesn't quite work because the TB is about a magnitude
>   or two too slow. So the plan is to change ppc to store a
>   software generation counter in the mmap()ed state, and change
>   the ppc user-space to check that one instead.

If we're going to do it for ppc, we might as well do it for all
platforms.  That gets us one step closer to eliminating cstatus from
the user visible stuff, too, which I think should be done.

> - Move <asm-$arch/perfctr.h> common stuff to <asm-generic/perfctr.h>.
> 
> In addition, there is one unresolved issue:
> - A counter's value is represented by a 64-bit software sum,
>   a 32-bit start value containing the HW counter's value at the
>   start of the current time slice, and the current HW counter's value
>   (now). The actual value is computed as sum + (now - start).
>   This is reflected in the mmap()ed state, which contains a variable-
>   length { u32 map; u32 start; u64 sum; } pmc[] array.
>   This layout is very cache-efficient on current 32 and 64-bit CPUs,
>   but there is a _possible_ concern that it won't do on 10+ GHz CPUs.
>   So the question is, should we change it to use 64-bit start values
>   already now (and take more cache misses), or should that wait a few
>   years until it becomes a necessity (causing ABI change issues)?

Is there any way we could rearrange the user visible stuff to not
include the 'map' field?  After all userspace set up the counters, so
it ought to know what the mapping is already...

That would mean we could fit in a 64-bit start value without having to
mess around to get good alignment.

-- 
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/people/dgibson