Skip to contentSkip to conversation

ipipe-tracer-v5

PATCH In review Jan Kiszka 2 participants 4 messages Dec 28, 2005 – Dec 29, 2005 Kbuild

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 ipipe-tracer-v5

Hi Philippe,

this is a revised version of the tracer patch, addressing your concerns hopefully in the right way. Changes since the previous release:

* move __ipipe_init_trace_proc() declaration to linux/ipipe.h
* define __BUILTIN_RETURN_ADDRESSx in linux/ipipe.h unless some arch defines BROKEN_BUILTIN_RETURN_ADDRESS and provides its own variants
* improve ipipe_tsc2ns() for i386 so that it makes use of do_div (and also accepts 64 bit input)
* move ipipe_tsc2us() to asm-i386/ipipe.h
* fix a display bug in the output of "frozen"
* reformatting
* remove inclusion of linux/ipipe_trace.h from linux/sched.h

NOTE: The last change makes the local inclusion of ipipe_trace.h necessary when instrumenting some code. Previous instrumentation patches are likely broken now.

I think it should be ready for apply now, shouldn't it?

Jan

arch/i386/boot/compressed/misc.c renamed from arch/i386/boot/compressed/misc.c.orig +8 -1
@@ -15,6 +15,12 @@
 #include <asm/io.h>
 #include <asm/page.h>
 
+#ifdef CONFIG_IPIPE_TRACE
+void __attribute__ ((no_instrument_function)) mcount(void)
+{
+}
+#endif
+
 /*
  * gzip declarations
  */
@@ -112,7 +118,7 @@
 #define INPLACE_MOVE_ROUTINE  0x1000
 #define LOW_BUFFER_START      0x2000
 #define LOW_BUFFER_MAX       0x90000
-#define HEAP_SIZE             0x3000
+#define HEAP_SIZE             0x4000
 static unsigned int low_buffer_end, low_buffer_size;
 static int high_loaded =0;
 static uch *high_buffer_start /* = (uch *)(((ulg)&end) + HEAP_SIZE)*/;
@@ -125,6 +131,7 @@
 static void * xquad_portio = NULL;
 #endif
 
+#define ZLIB_INFLATE_NO_INFLATE_LOCK
 #include "../../../../lib/inflate.c"
 
 static void *malloc(int size)
arch/i386/kernel/ipipe-mcount.S renamed from arch/i386/kernel/ipipe-mcount.S.orig +45 -0

45 lines not on this page · raw message

arch/i386/kernel/Makefile renamed from arch/i386/kernel/Makefile.orig +1 -0
@@ -31,6 +31,7 @@
 obj-$(CONFIG_MODULES)		+= module.o
 obj-y				+= sysenter.o vsyscall.o
 obj-$(CONFIG_IPIPE)		+= ipipe-core.o ipipe-root.o
+obj-$(CONFIG_IPIPE_TRACE)	+= ipipe-mcount.o
 obj-$(CONFIG_ACPI_SRAT) 	+= srat.o
 obj-$(CONFIG_HPET_TIMER) 	+= time_hpet.o
 obj-$(CONFIG_EFI) 		+= efi.o efi_stub.o
Makefile renamed from Makefile.orig +4 -0
@@ -517,11 +517,15 @@
 CFLAGS		+= $(call add-align,CONFIG_CC_ALIGN_LOOPS,-loops)
 CFLAGS		+= $(call add-align,CONFIG_CC_ALIGN_JUMPS,-jumps)
 
+ifdef CONFIG_IPIPE_TRACE
+CFLAGS          += -pg -fno-omit-frame-pointer $(call cc-option,-fno-optimize-sibling-calls,)
+else
 ifdef CONFIG_FRAME_POINTER
 CFLAGS		+= -fno-omit-frame-pointer $(call cc-option,-fno-optimize-sibling-calls,)
 else
 CFLAGS		+= -fomit-frame-pointer
 endif
+endif
 
 ifdef CONFIG_DEBUG_INFO
 CFLAGS		+= -g
kernel/ipipe/Makefile renamed from kernel/ipipe/Makefile.orig +1 -0
@@ -1,2 +1,3 @@
 
 obj-$(CONFIG_IPIPE)	+= core.o generic.o
+obj-$(CONFIG_IPIPE_TRACE) += tracer.o
kernel/ipipe/core.c renamed from kernel/ipipe/core.c.orig +2 -1
@@ -415,7 +415,7 @@
 
 #include <linux/proc_fs.h>
 
-static struct proc_dir_entry *ipipe_proc_root;
+struct proc_dir_entry *ipipe_proc_root;
 
 static int __ipipe_version_info_proc(char *page,
 				     char **start,
@@ -657,6 +657,7 @@
 {
 	ipipe_proc_root = create_proc_entry("ipipe",S_IFDIR, 0);
 	create_proc_read_entry("version",0444,ipipe_proc_root,&__ipipe_version_info_proc,NULL);
+	__ipipe_init_trace_proc();
 	__ipipe_add_domain_proc(ipipe_root_domain);
 }
 
kernel/ipipe/tracer.c renamed from kernel/ipipe/tracer.c.orig +823 -0

823 lines not on this page · raw message

kernel/ipipe/Kconfig renamed from kernel/ipipe/Kconfig.orig +8 -0
@@ -14,5 +14,13 @@
 	  while the I-pipe is operating. This option adds a small overhead, but
 	  is useful to detect unexpected latency points.
 
+config IPIPE_TRACE
+	bool "Trace IRQ latencies"
+	depends on IPIPE && PROC_FS
+	default n
+	---help---
+	  Activate this option if I-pipe shall trace the longest stalled path
+	  of the domain with the highest priority.
+
 config IPIPE_EXTENDED
 	def_bool IPIPE
include/linux/ipipe.h renamed from include/linux/ipipe.h.orig +12 -0
@@ -33,6 +33,11 @@
 				 (IPIPE_MINOR_NUMBER <<  8) | \
 				 (IPIPE_PATCH_NUMBER))
 
+#ifndef BROKEN_BUILTIN_RETURN_ADDRESS
+#define __BUILTIN_RETURN_ADDRESS0 ((unsigned long)__builtin_return_address(0))
+#define __BUILTIN_RETURN_ADDRESS1 ((unsigned long)__builtin_return_address(1))
+#endif /* !BUILTIN_RETURN_ADDRESS */
+
 #define IPIPE_ROOT_PRIO		100
 #define IPIPE_ROOT_ID		0
 #define IPIPE_ROOT_NPTDKEYS	4	/* Must be <= BITS_PER_LONG */
@@ -309,6 +314,13 @@
 
 #ifdef CONFIG_PROC_FS
 void ipipe_init_proc(void);
+
+#ifdef CONFIG_IPIPE_TRACE
+void __ipipe_init_trace_proc(void);
+#else /* !CONFIG_IPIPE_TRACE */
+#define __ipipe_init_trace_proc()   do { } while(0)
+#endif /* CONFIG_IPIPE_TRACE */
+
 #else	/* !CONFIG_PROC_FS */
 #define ipipe_init_proc()	do { } while(0)
 #endif	/* CONFIG_PROC_FS */
include/linux/ipipe_trace.h renamed from include/linux/ipipe_trace.h.orig +31 -0
@@ -0,0 +1,31 @@
+/* -*- linux-c -*-
+ * include/linux/ipipe_trace.h
+ *
+ * Copyright (C) 2005 Luotao Fu.
+ *               2005 Jan Kiszka.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, Inc., 675 Mass Ave, Cambridge MA 02139,
+ * USA; either version 2 of the License, or (at your option) any later
+ * version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#ifndef _LINUX_IPIPE_TRACE_H
+#define _LINUX_IPIPE_TRACE_H
+
+void ipipe_trace_begin(unsigned long v);
+void ipipe_trace_end(unsigned long v);
+void ipipe_trace_freeze(unsigned long v);
+void ipipe_trace_special(unsigned char special_id, unsigned long v);
+
+#endif	/* !__LINUX_IPIPE_H */
include/linux/linkage.h renamed from include/linux/linkage.h.orig +4 -0
@@ -51,4 +51,8 @@
 #define fastcall
 #endif
 
+#ifndef notrace
+#define notrace		__attribute__((no_instrument_function))
+#endif
+
 #endif
include/asm-i386/ipipe.h renamed from include/asm-i386/ipipe.h.orig +14 -1
@@ -157,7 +157,20 @@
 
 #define ipipe_read_tsc(t)  __asm__ __volatile__("rdtsc" : "=A" (t))
 #define ipipe_cpu_freq() ({ unsigned long long __freq = cpu_has_tsc?(1000LL * cpu_khz):CLOCK_TICK_RATE; __freq; })
-#define ipipe_tsc2ns(t)  (((t) * 1000) / (cpu_khz / 1000))
+
+#define ipipe_tsc2ns(t) \
+({ \
+	unsigned long long delta = (t)*1000; \
+	do_div(delta, cpu_khz/1000+1); \
+	(unsigned long)delta; \
+})
+
+#define ipipe_tsc2us(t) \
+({ \
+    unsigned long long delta = (t); \
+    do_div(delta, cpu_khz/1000+1); \
+    (unsigned long)delta; \
+})
 
 /* Private interface -- Internal use only */
 
#1 lore raw
Quoted 15 lines · Jan Kiszka

Hi Philippe,

this is a revised version of the tracer patch, addressing your concerns hopefully in the right way. Changes since the previous release:

* move __ipipe_init_trace_proc() declaration to linux/ipipe.h
* define __BUILTIN_RETURN_ADDRESSx in linux/ipipe.h unless some arch defines BROKEN_BUILTIN_RETURN_ADDRESS and provides its own variants
* improve ipipe_tsc2ns() for i386 so that it makes use of do_div (and also accepts 64 bit input)
* move ipipe_tsc2us() to asm-i386/ipipe.h
* fix a display bug in the output of "frozen"
* reformatting
* remove inclusion of linux/ipipe_trace.h from linux/sched.h

Quoted 6 lines

NOTE: The last change makes the local inclusion of ipipe_trace.h necessary when instrumenting some code. Previous instrumentation patches are likely broken now.

I think it should be ready for apply now, shouldn't it?

Yes; you guys did a nice job. v5 is on the merge queue, and will be applied tomorrow. We'll work on that basis for enhancements/fixes. Thanks.

Signature · 1 line
Philippe.
author
#2 lore raw
In reply to Jan Kiszka
Quoted 27 lines · Jan Kiszka

------------------------------------------------------------------------

arch/i386/boot/compressed/misc.c renamed from arch/i386/boot/compressed/misc.c.orig +7 -1
@@ -15,6 +15,12 @@
 #include <asm/io.h>
 #include <asm/page.h>
 
+#ifdef CONFIG_IPIPE_TRACE
+void __attribute__ ((no_instrument_function)) mcount(void)
+{
+}
+#endif
+
 /*
  * gzip declarations
  */
@@ -112,7 +118,7 @@
 #define INPLACE_MOVE_ROUTINE  0x1000
 #define LOW_BUFFER_START      0x2000
 #define LOW_BUFFER_MAX       0x90000
-#define HEAP_SIZE             0x3000
+#define HEAP_SIZE             0x4000
 static unsigned int low_buffer_end, low_buffer_size;
 static int high_loaded =0;
 static uch *high_buffer_start /* = (uch *)(((ulg)&end) + HEAP_SIZE)*/;

arch/i386/boot/compressed/misc.c line 136

Quoted 6 lines
arch/i386/boot/compressed/misc.c renamed from arch/i386/boot/compressed/misc.c.orig +1 -0
@@ -125,6 +131,7 @@
 static void * xquad_portio = NULL;
 #endif
 
+#define ZLIB_INFLATE_NO_INFLATE_LOCK
 #include "../../../../lib/inflate.c"
 

With the recent advent of a broken-out latency trace patch for the standard kernel, it became clear what the last patched line is for:
nothing in this case, it only makes sense as the full reduce-zlib-stack-hack.patch available from

http://people.redhat.com/mingo/latency-tracing-patches/patches

One only need this add-on patch when running the tracing kernel with 4k stacks.

Philippe, I think we should not merge this particular line change. In contrast, the HEAP_SIZE change also appears in the tracer patch itself.

Jan

#3 lore raw
Quoted 42 lines · Jan Kiszka
Quoted 27 lines · Jan Kiszka

------------------------------------------------------------------------

arch/i386/boot/compressed/misc.c renamed from arch/i386/boot/compressed/misc.c.orig +7 -1
@@ -15,6 +15,12 @@
 #include <asm/io.h>
 #include <asm/page.h>
 
+#ifdef CONFIG_IPIPE_TRACE
+void __attribute__ ((no_instrument_function)) mcount(void)
+{
+}
+#endif
+
 /*
 * gzip declarations
 */
@@ -112,7 +118,7 @@
 #define INPLACE_MOVE_ROUTINE  0x1000
 #define LOW_BUFFER_START      0x2000
 #define LOW_BUFFER_MAX       0x90000
-#define HEAP_SIZE             0x3000
+#define HEAP_SIZE             0x4000
 static unsigned int low_buffer_end, low_buffer_size;
 static int high_loaded =0;
 static uch *high_buffer_start /* = (uch *)(((ulg)&end) + HEAP_SIZE)*/;

arch/i386/boot/compressed/misc.c line 136

Quoted 6 lines
arch/i386/boot/compressed/misc.c renamed from arch/i386/boot/compressed/misc.c.orig +1 -0
@@ -125,6 +131,7 @@
 static void * xquad_portio = NULL;
 #endif
 
+#define ZLIB_INFLATE_NO_INFLATE_LOCK
 #include "../../../../lib/inflate.c"
 

With the recent advent of a broken-out latency trace patch for the standard kernel, it became clear what the last patched line is for:
nothing in this case, it only makes sense as the full reduce-zlib-stack-hack.patch available from

Quoted 8 lines

http://people.redhat.com/mingo/latency-tracing-patches/patches

One only need this add-on patch when running the tracing kernel with 4k stacks.

Philippe, I think we should not merge this particular line change. In contrast, the HEAP_SIZE change also appears in the tracer patch itself.

Ack.

Signature · 1 line
Philippe.