[PATCH] - kernel profiler & spinlock_contention

[PATCH] - kernel profiler & spinlock_contention

[Jack Steiner]

Profiling is more useful if addresses in spinlock_contention
were attributed to the caller of spinlock_contention.



diff -Naur linux_base/arch/ia64/kernel/head.S linux/arch/ia64/kernel/head.S
--- linux_base/arch/ia64/kernel/head.S	Tue Nov 25 10:03:46 2003
+++ linux/arch/ia64/kernel/head.S	Tue Nov 25 15:21:58 2003
@@ -888,6 +888,8 @@
 	cmp4.eq p14,p0=r30,r0
 (p14)	br.cond.sptk.few b6	// lock is now free, try to acquire
 	br.cond.sptk.few .wait
+	.global ia64_spinlock_contention_pre3_4_end    // for kernprof
+ia64_spinlock_contention_pre3_4_end:
 END(ia64_spinlock_contention_pre3_4)
 
 #else



diff -Naur linux_base/arch/ia64/kernel/time.c linux/arch/ia64/kernel/time.c
--- linux_base/arch/ia64/kernel/time.c	Tue Nov 25 10:03:46 2003
+++ linux/arch/ia64/kernel/time.c	Tue Nov 25 10:07:21 2003
@@ -202,6 +202,9 @@
 {
 	unsigned long ip, slot;
 	extern cpumask_t prof_cpu_mask;
+#ifdef CONFIG_SMP
+	extern char ia64_spinlock_contention_pre3_4[], ia64_spinlock_contention_pre3_4_end[];
+#endif
 
 	profile_hook(regs);
 
@@ -216,7 +219,14 @@
 	 * bits 2 and 3 rather than bits 0 and 1.
 	 */
 	slot = ip & 3;
-	ip = (ip & ~3UL) + 4*slot;
+	ip = (ip & ~3UL);
+#ifdef CONFIG_SMP
+	if (ip >= (unsigned long)ia64_spinlock_contention_pre3_4 &&
+			ip < (unsigned long)ia64_spinlock_contention_pre3_4_end)
+		ip = regs->r28;
+	else
+#endif
+	ip += 4*slot;
 
 	/*
 	 * Only measure the CPUs specified by /proc/irq/prof_cpu_mask.
-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

Re: [PATCH] - kernel profiler & spinlock_contention

[David Mosberger]

>>>>> On Wed, 26 Nov 2003 09:23:10 -0600, Jack Steiner <steiner@sgi.com> said:

  Jack> Profiling is more useful if addresses in spinlock_contention
  Jack> were attributed to the caller of spinlock_contention.

Execuse the language, but I'm reading:

	my tools suck so let's make the kernel suck!

Let's defer this discussion until next week, when I had a chance to
release my profiling tool.  I think you'll then agree that it's much
better to leave the code as is.  In fact, we should also have an
out-of-line contention handler for the read/write locks.

	--david

Re: [PATCH] - kernel profiler & spinlock_contention

[Jack Steiner]

On Wed, Nov 26, 2003 at 09:31:02AM -0800, David Mosberger wrote:
> >>>>> On Wed, 26 Nov 2003 09:23:10 -0600, Jack Steiner <steiner@sgi.com> said:
> 
>   Jack> Profiling is more useful if addresses in spinlock_contention
>   Jack> were attributed to the caller of spinlock_contention.
> 
> Execuse the language, but I'm reading:
> 
> 	my tools suck so let's make the kernel suck!

I dont quite agree. The way it work right now, profiling on large
systems is useless. The single hotest spot in the kernel is
spinlock_contention & there is no clue why. The patch makes
the kernel work the way it did before spinning for locks was
moved out-of-line. I think that is a big improvement.

However, if your new tools allows me to determine the caller
of spinlock_contention (I'm guessing that it does), that is even better!!
If a prerelease version of the tools is available, I'll be happy
to try it on our system

> 
> Let's defer this discussion until next week, when I had a chance to
> release my profiling tool.  I think you'll then agree that it's much
> better to leave the code as is.  In fact, we should also have an
> out-of-line contention handler for the read/write locks.

Agree!


> 
> 	--david

-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

Re: [PATCH] - kernel profiler & spinlock_contention

[David Mosberger]

>>>>> On Wed, 26 Nov 2003 13:02:30 -0600, Jack Steiner <steiner@sgi.com> said:

  Jack> However, if your new tools allows me to determine the caller
  Jack> of spinlock_contention (I'm guessing that it does), that is
  Jack> even better!!

Absolutely.  Best of all, it requires absolutely no kernel modes.
All it requires is a recent 2.6 kernel and an Itanium 2 CPU
(sorry, Merced is out for this feature).

  Jack> If a prerelease version of the tools is available, I'll be
  Jack> happy to try it on our system

I'll see what I can do.

	--david