2005-03-31 22:08:27

by Mikael Pettersson

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

Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
written by David Gibson <[email protected]>.
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 <[email protected]>:
- ppc64 arch hooks: Kconfig, syscalls numbers and tables, task struct,
and process management ops (switch_to, exit, fork)

Signed-off-by: Mikael Pettersson <[email protected]>

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


2005-03-31 23:11:37

by Andrew Morton

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

Mikael Pettersson <[email protected]> wrote:
>
> Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> written by David Gibson <[email protected]>.

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

2005-04-01 01:34:57

by Andrew Morton

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

David Gibson <[email protected]> wrote:
>
> On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
> > Mikael Pettersson <[email protected]> wrote:
> > >
> > > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> > > written by David Gibson <[email protected]>.
> >
> > 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?

2005-04-01 00:52:43

by David Gibson

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

On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
> Mikael Pettersson <[email protected]> wrote:
> >
> > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> > written by David Gibson <[email protected]>.
>
> 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

2005-04-01 12:52:47

by Mikael Pettersson

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

Andrew Morton writes:
> David Gibson <[email protected]> wrote:
> >
> > On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
> > > Mikael Pettersson <[email protected]> wrote:
> > > >
> > > > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> > > > written by David Gibson <[email protected]>.
> > >
> > > 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

2005-04-01 18:28:52

by Andrew Morton

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

Mikael Pettersson <[email protected]> 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.

2005-04-04 03:35:04

by David Gibson

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

On Fri, Apr 01, 2005 at 02:46:53PM +0200, Mikael Pettersson wrote:
> Andrew Morton writes:
> > David Gibson <[email protected]> wrote:
> > >
> > > On Thu, Mar 31, 2005 at 03:11:29PM -0800, Andrew Morton wrote:
> > > > Mikael Pettersson <[email protected]> wrote:
> > > > >
> > > > > Here's a 3-part patch kit which adds a ppc64 driver to perfctr,
> > > > > written by David Gibson <[email protected]>.
> > > >
> > > > 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