public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Linux Kernel Markers 0.11 for 2.6.17
@ 2006-09-25 15:16 Mathieu Desnoyers
  2006-09-25 16:01 ` Frank Ch. Eigler
       [not found] ` <45181CE9.1080204@goop.org>
  0 siblings, 2 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 15:16 UTC (permalink / raw)
  To: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Mathieu Desnoyers, Paul Mundt,
	linux-kernel, Jes Sorensen, Tom Zanussi, Richard J Moore,
	Michel Dagenais, Christoph Hellwig, Greg Kroah-Hartman,
	Thomas Gleixner, William Cohen, ltt-dev, systemtap, Alan Cox,
	Jeremy Fitzhardinge, Karim Yaghmour, Pavel Machek, Joe Perches,
	Randy.Dunlap, Jose R. Santos

Good morning everyone,

Following Jeremy Fitzhardinge's advice, I rewrote my marker mechanism taking in
consideration inline functions (and therefore also unrolled loops). This new
marker version is a complete rewrite of the previous one. It allows :

- Multiple occurrences of the same marker name.
- Declaration of a marker in an inline function.
- Declaration of a marker in an unrolled loop.
- It _does not_ change the compiler optimisations.

I just declare the markers in a separate section and allow duplicated entries.
To set a probe on a marker, an iteration on all the markers is done.

This marker mechanism is still portable : the non optimised version is a call to
an empty function. Per architecture optimisations adds a near jump around the
function call to that the call can be enabled or disabled by modifying the 1
byte offset of the jump.

Architectures that do not use include/asm-generic/vmlinux.lds.h should define
the marker section start/stop symbols.

Comments are welcome,

Mathieu


---BEGIN---

diff --git a/Makefile b/Makefile
index 1700d3f..78ed30f 100644
--- a/Makefile
+++ b/Makefile
@@ -301,7 +301,8 @@ # Use LINUXINCLUDE when you must referen
 # Needed to be compatible with the O= option
 LINUXINCLUDE    := -Iinclude \
                    $(if $(KBUILD_SRC),-Iinclude2 -I$(srctree)/include) \
-		   -include include/linux/autoconf.h
+		   -include include/linux/autoconf.h \
+		   -include include/linux/marker.h
 
 CPPFLAGS        := -D__KERNEL__ $(LINUXINCLUDE)
 
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 213c785..95c1d1b 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -630,6 +630,12 @@ source "fs/Kconfig"
 
 source "arch/alpha/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/alpha/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 08b7cc9..4c03f09 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -871,6 +871,12 @@ source "fs/Kconfig"
 
 source "arch/arm/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/arm/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/arm26/Kconfig b/arch/arm26/Kconfig
index cf4ebf4..f34e157 100644
--- a/arch/arm26/Kconfig
+++ b/arch/arm26/Kconfig
@@ -232,6 +232,12 @@ source "drivers/misc/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/arm26/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/cris/Kconfig b/arch/cris/Kconfig
index 856b665..84f8efd 100644
--- a/arch/cris/Kconfig
+++ b/arch/cris/Kconfig
@@ -179,6 +179,12 @@ source "sound/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/cris/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig
index 95a3892..8708a75 100644
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -345,6 +345,12 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/frv/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index cabf0bf..bdb67ed 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -197,6 +197,12 @@ endmenu
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/h8300/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index 8dfa305..e6cc7da 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -1081,6 +1081,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/i386/Kconfig.debug"
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 0f3076a..2342975 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -499,6 +499,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/ia64/Kconfig.debug"
diff --git a/arch/m32r/Kconfig b/arch/m32r/Kconfig
index 41fd490..50f0a8e 100644
--- a/arch/m32r/Kconfig
+++ b/arch/m32r/Kconfig
@@ -386,6 +386,12 @@ source "fs/Kconfig"
 
 source "arch/m32r/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/m32r/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 805b81f..5af9e00 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -652,6 +652,12 @@ endmenu
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/m68k/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/m68knommu/Kconfig b/arch/m68knommu/Kconfig
index 3cde682..bd01cc0 100644
--- a/arch/m68knommu/Kconfig
+++ b/arch/m68knommu/Kconfig
@@ -652,6 +652,12 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/m68knommu/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index e8ff09f..4d5dbf9 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1850,6 +1850,12 @@ source "fs/Kconfig"
 
 source "arch/mips/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/mips/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 910fb3a..a0a7dd7 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -251,6 +251,12 @@ source "fs/Kconfig"
 
 source "arch/parisc/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/parisc/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6729c98..44f25b5 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1011,6 +1011,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/powerpc/Kconfig.debug"
diff --git a/arch/ppc/Kconfig b/arch/ppc/Kconfig
index e9a8f5d..ca871ab 100644
--- a/arch/ppc/Kconfig
+++ b/arch/ppc/Kconfig
@@ -1412,6 +1412,12 @@ source "lib/Kconfig"
 
 source "arch/powerpc/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/ppc/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 01c5c08..57600b5 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -476,6 +476,12 @@ source "fs/Kconfig"
 
 source "arch/s390/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/s390/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 2bcecf4..0fd24d3 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -644,6 +644,12 @@ source "fs/Kconfig"
 
 source "arch/sh/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/sh/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/sh64/Kconfig b/arch/sh64/Kconfig
index 58c678e..f4ecb4d 100644
--- a/arch/sh64/Kconfig
+++ b/arch/sh64/Kconfig
@@ -276,6 +276,12 @@ source "fs/Kconfig"
 
 source "arch/sh64/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/sh64/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 9431e96..de02311 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -289,6 +289,12 @@ endmenu
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/sparc/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 43a66f5..971f5e2 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -419,6 +419,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/sparc64/Kconfig.debug"
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 76e85bb..9481883 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -312,4 +312,10 @@ config INPUT
 	bool
 	default n
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/um/Kconfig.debug"
diff --git a/arch/v850/Kconfig b/arch/v850/Kconfig
index 37ec644..6ce651f 100644
--- a/arch/v850/Kconfig
+++ b/arch/v850/Kconfig
@@ -324,6 +324,12 @@ source "sound/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/v850/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/arch/x86_64/Kconfig b/arch/x86_64/Kconfig
index 408d44a..6feb011 100644
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -611,6 +611,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/x86_64/Kconfig.debug"
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index dbeb350..413abb8 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -255,6 +255,12 @@ config EMBEDDED_RAMDISK_IMAGE
 	  provide one yourself.
 endmenu
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/xtensa/Kconfig.debug"
 
 source "security/Kconfig"
diff --git a/include/asm-alpha/marker.h b/include/asm-alpha/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-alpha/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-arm/marker.h b/include/asm-arm/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-arm/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-arm26/marker.h b/include/asm-arm26/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-arm26/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-cris/marker.h b/include/asm-cris/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-cris/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-frv/marker.h b/include/asm-frv/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-frv/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-generic/marker.h b/include/asm-generic/marker.h
new file mode 100644
index 0000000..cb44bb3
--- /dev/null
+++ b/include/asm-generic/marker.h
@@ -0,0 +1,22 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Generic header.
+ * 
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#define MARK_CALL(name, format, args...) \
+	do { \
+		static marker_probe_func *__mark_call_##name = \
+					__mark_empty_function; \
+		asm volatile(	".section .markers, \"a\";\n\t" \
+				".long %0, %1;\n\t" \
+				".previous;\n\t" : : \
+			"m" (__mark_call_##name), \
+			"m" (*format));\
+		preempt_disable(); \
+		(*__mark_call_##name)(format, ## args); \
+		preempt_enable_no_resched(); \
+	} while(0)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 9d11550..d029043 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -90,6 +90,12 @@ #define RODATA								\
         __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {	\
 		*(__ksymtab_strings)					\
 	}								\
+	/* Kernel markers : pointers */					\
+	.markers : AT(ADDR(.markers) - LOAD_OFFSET) {			\
+		VMLINUX_SYMBOL(__start___markers) = .;			\
+		*(.markers)						\
+		VMLINUX_SYMBOL(__stop___markers) = .;			\
+	}								\
 	__end_rodata = .;						\
 	. = ALIGN(4096);						\
 									\
diff --git a/include/asm-h8300/marker.h b/include/asm-h8300/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-h8300/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-i386/marker.h b/include/asm-i386/marker.h
new file mode 100644
index 0000000..665866b
--- /dev/null
+++ b/include/asm-i386/marker.h
@@ -0,0 +1,37 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. i386 architecture optimisations.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
+
+#define ARCH_HAS_MARK_JUMP
+
+/* Note : max 256 bytes between over_label and near_jump */
+#define MARK_JUMP(name, format, args...) \
+	do { \
+		__label__ over_label, call_label, jump_select_label; \
+		asm volatile(	".section .markers, \"a\";\n\t" \
+				".long %0, %1, %2;\n\t" \
+				".previous;\n\t" : : \
+			"m" (*&&jump_select_label), \
+			"m" (*&&call_label), \
+			"m" (*&&over_label)); \
+		asm volatile (	".align 16;\n\t" : : ); \
+		asm volatile (	".byte 0xeb;\n\t" : : ); \
+jump_select_label: \
+		asm volatile (	".byte %0-%1;\n\t" : : \
+				"m" (*&&over_label), "m" (*&&call_label)); \
+call_label: \
+		asm volatile ("" : : ); \
+		MARK_CALL(name, format, ## args); \
+		asm volatile ("" : : ); \
+over_label: \
+		asm volatile ("" : : ); \
+	} while(0)
diff --git a/include/asm-ia64/marker.h b/include/asm-ia64/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-ia64/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-m32r/marker.h b/include/asm-m32r/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-m32r/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-m68k/marker.h b/include/asm-m68k/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-m68k/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-m68knommu/marker.h b/include/asm-m68knommu/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-m68knommu/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-mips/marker.h b/include/asm-mips/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-mips/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-parisc/marker.h b/include/asm-parisc/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-parisc/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-powerpc/marker.h b/include/asm-powerpc/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-powerpc/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-ppc/marker.h b/include/asm-ppc/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-ppc/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-ppc64/marker.h b/include/asm-ppc64/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-ppc64/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-s390/marker.h b/include/asm-s390/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-s390/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-sh/marker.h b/include/asm-sh/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-sh/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-sh64/marker.h b/include/asm-sh64/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-sh64/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-sparc/marker.h b/include/asm-sparc/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-sparc/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-sparc64/marker.h b/include/asm-sparc64/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-sparc64/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-um/marker.h b/include/asm-um/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-um/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-v850/marker.h b/include/asm-v850/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-v850/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-x86_64/marker.h b/include/asm-x86_64/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-x86_64/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/asm-xtensa/marker.h b/include/asm-xtensa/marker.h
new file mode 100644
index 0000000..8d9467f
--- /dev/null
+++ b/include/asm-xtensa/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
diff --git a/include/linux/marker.h b/include/linux/marker.h
new file mode 100644
index 0000000..734660d
--- /dev/null
+++ b/include/linux/marker.h
@@ -0,0 +1,90 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing.
+ *
+ * Example :
+ *
+ * MARK(subsystem_event, "%d %s", someint, somestring);
+ * Where :
+ * - Subsystem is the name of your subsystem.
+ * - event is the name of the event to mark.
+ * - "%d %s" is the formatted string for printk.
+ * - someint is an integer.
+ * - somestring is a char *.
+ *
+ * Dynamically overridable function call based on marker mechanism
+ *          from Frank Ch. Eigler <fche@redhat.com>.
+ *
+ * The marker mechanism supports multiple instances of the same marker.
+ * Markers can be put in inline functions, inlined static functions and
+ * unrolled loops without changing the optimisations.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifndef __ASSEMBLY__
+
+#include <linux/linkage.h>
+#include <asm/marker.h>
+
+#if !defined(ARCH_HAS_MARK_JUMP) || defined(CONFIG_MARKERS_FORCE_DIRECT_CALL)
+#undef MARK_JUMP
+#undef ARCH_HAS_MARK_JUMP
+#define MARK_JUMP MARK_CALL
+#endif
+
+#define MARK_NOARGS " "
+#define MARK_MAX_FORMAT_LEN	1024
+
+#ifdef CONFIG_MARKERS
+#define MARK(name, format, args...) \
+	do { \
+		__label__ here; \
+here:   	asm volatile(	".section .markers, \"a\";\n\t" \
+				".long %0, %1;\n\t" \
+				".previous;\n\t" : : \
+			"m" (*(#name)), \
+			"m" (*&&here)); \
+		__mark_check_format(format, ## args); \
+		MARK_JUMP(name, format, ## args); \
+	} while(0)
+#else
+#define MARK(name, format, args...) \
+		__mark_check_format(format, ## args)
+#endif
+
+typedef asmlinkage void marker_probe_func(const char *fmt, ...);
+
+struct __mark_marker {
+	const char *name;
+	const void *location;
+#ifdef ARCH_HAS_MARK_JUMP
+	char *select;
+	const void *jump_call;
+	const void *jump_over;
+#endif
+	marker_probe_func **call;
+	const char *format;
+};
+
+static inline __attribute__ ((format (printf, 1, 2)))
+void __mark_check_format(const char *fmt, ...)
+{ }
+
+extern marker_probe_func __mark_empty_function;
+
+extern int marker_set_probe(const char *name, const char *format,
+				marker_probe_func *probe);
+
+extern int marker_remove_probe(marker_probe_func *probe);
+extern int marker_list_probe(marker_probe_func *probe);
+
+#endif
+#endif
diff --git a/include/linux/module.h b/include/linux/module.h
index eaec13d..5689857 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -254,6 +254,8 @@ struct module
 	const struct kernel_symbol *syms;
 	unsigned int num_syms;
 	const unsigned long *crcs;
+	const struct __mark_marker *markers;
+	unsigned int num_markers;
 
 	/* GPL-only exported symbols. */
 	const struct kernel_symbol *gpl_syms;
diff --git a/kernel/Kconfig.marker b/kernel/Kconfig.marker
new file mode 100644
index 0000000..2a828c7
--- /dev/null
+++ b/kernel/Kconfig.marker
@@ -0,0 +1,18 @@
+# Code markers configuration
+
+config MARKERS
+	bool "Activate markers"
+	select MODULES
+	default n
+	help
+	  Place an empty function call at each marker site. Can by
+	  dynamically changed for a probe function.
+
+config MARKERS_FORCE_DIRECT_CALL
+	bool "Disable markers jump optimisations"
+	depends EMBEDDED
+	default n
+	help
+	  Disable code replacement jump optimisations. Especially useful if your
+	  code is in a read-only rom/flash.
+
diff --git a/kernel/module.c b/kernel/module.c
index bbe0486..826cf69 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -123,6 +123,8 @@ extern const struct kernel_symbol __stop
 extern const unsigned long __start___kcrctab[];
 extern const unsigned long __start___kcrctab_gpl[];
 extern const unsigned long __start___kcrctab_gpl_future[];
+extern const struct __mark_marker __start___markers[];
+extern const struct __mark_marker __stop___markers[];
 
 #ifndef CONFIG_MODVERSIONS
 #define symversion(base, idx) NULL
@@ -236,6 +238,153 @@ static struct module *find_module(const 
 	return NULL;
 }
 
+#ifdef CONFIG_MARKERS
+asmlinkage void __mark_empty_function(const char *fmt, ...)
+{
+}
+EXPORT_SYMBOL(__mark_empty_function);
+
+static int marker_set_probe_range(const char *name, 
+	const char *format,
+	marker_probe_func *probe,
+	const struct __mark_marker *begin,
+	const struct __mark_marker *end)
+{
+	const struct __mark_marker *iter;
+	int found = 0;
+
+	for(iter = begin; iter < end; iter++) {
+		if (strcmp(name, iter->name) == 0) {
+			if (format && strcmp(format, iter->format) != 0) {
+				printk(KERN_NOTICE
+					"Format mismatch for probe %s "
+					"(%s), marker (%s)\n",
+					name,
+					format,
+					iter->format);
+				continue;
+			}
+			found++;
+			*iter->call = probe;
+#ifdef ARCH_HAS_MARK_JUMP
+			if (probe != __mark_empty_function)
+				*iter->select = 0;
+			else
+				*iter->select = iter->jump_over
+					- iter->jump_call;
+#endif
+		}
+	}
+	return found;
+}
+
+static int marker_remove_probe_range(marker_probe_func *probe,
+	const struct __mark_marker *begin,
+	const struct __mark_marker *end)
+{
+	const struct __mark_marker *iter;
+	int found = 0;
+
+	for(iter = begin; iter < end; iter++) {
+		if (*iter->call == probe) {
+#ifdef ARCH_HAS_MARK_JUMP
+			*iter->select = iter->jump_over
+				- iter->jump_call;
+#endif
+			*iter->call = __mark_empty_function;
+			found++;
+		}
+	}
+	return found;
+}
+
+static int marker_list_probe_range(marker_probe_func *probe,
+	const struct __mark_marker *begin,
+	const struct __mark_marker *end)
+{
+	const struct __mark_marker *iter;
+	int found = 0;
+
+	for(iter = begin; iter < end; iter++) {
+		if (probe)
+			if (probe != *iter->call) continue;
+		printk("name %s location 0x%p\n", iter->name, iter->location);
+#ifdef ARCH_HAS_MARK_JUMP
+		printk("  select %hu jump_call %p jump_over %p\n",
+			*iter->select, iter->jump_call, iter->jump_over);
+#endif
+		printk("  func 0x%p format \"%s\"\n",
+			*iter->call, iter->format);
+		found++;
+	}
+	return found;
+}
+/* markers use the modlist_lock to to synchronise */
+int marker_set_probe(const char *name, const char *format,
+				marker_probe_func *probe)
+{
+	struct module *mod;
+	int found = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&modlist_lock, flags);
+	/* Core kernel markers */
+	found += marker_set_probe_range(name, format, probe,
+			__start___markers, __stop___markers);
+	/* Markers in modules. */ 
+	list_for_each_entry(mod, &modules, list) {
+		found += marker_set_probe_range(name, format, probe,
+				mod->markers, mod->markers+mod->num_markers);
+	}
+	spin_unlock_irqrestore(&modlist_lock, flags);
+	return found;
+}
+EXPORT_SYMBOL(marker_set_probe);
+
+int marker_remove_probe(marker_probe_func *probe)
+{
+	struct module *mod;
+	int found = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&modlist_lock, flags);
+	/* Core kernel markers */
+	found += marker_remove_probe_range(probe,
+			__start___markers, __stop___markers);
+	/* Markers in modules. */ 
+	list_for_each_entry(mod, &modules, list) {
+		found += marker_remove_probe_range(probe,
+				mod->markers, mod->markers+mod->num_markers);
+	}
+	spin_unlock_irqrestore(&modlist_lock, flags);
+	return found;
+}
+EXPORT_SYMBOL(marker_remove_probe);
+
+int marker_list_probe(marker_probe_func *probe)
+{
+	struct module *mod;
+	int found = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&modlist_lock, flags);
+	/* Core kernel markers */
+	printk("Listing kernel markers\n");
+	found += marker_list_probe_range(probe,
+			__start___markers, __stop___markers);
+	/* Markers in modules. */ 
+	printk("Listing module markers\n");
+	list_for_each_entry(mod, &modules, list) {
+		printk("Listing markers for module %s\n", mod->name);
+		found += marker_list_probe_range(probe,
+				mod->markers, mod->markers+mod->num_markers);
+	}
+	spin_unlock_irqrestore(&modlist_lock, flags);
+	return found;
+}
+EXPORT_SYMBOL(marker_list_probe);
+#endif
+
 #ifdef CONFIG_SMP
 /* Number of blocks used and allocated. */
 static unsigned int pcpu_num_used, pcpu_num_allocated;
@@ -1412,7 +1561,7 @@ static struct module *load_module(void _
 	unsigned int i, symindex = 0, strindex = 0, setupindex, exindex,
 		exportindex, modindex, obsparmindex, infoindex, gplindex,
 		crcindex, gplcrcindex, versindex, pcpuindex, gplfutureindex,
-		gplfuturecrcindex;
+		gplfuturecrcindex, markersindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -1502,6 +1651,7 @@ #endif
 	versindex = find_sec(hdr, sechdrs, secstrings, "__versions");
 	infoindex = find_sec(hdr, sechdrs, secstrings, ".modinfo");
 	pcpuindex = find_pcpusec(hdr, sechdrs, secstrings);
+	markersindex = find_sec(hdr, sechdrs, secstrings, ".markers");
 
 	/* Don't keep modinfo section */
 	sechdrs[infoindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
@@ -1510,6 +1660,11 @@ #ifdef CONFIG_KALLSYMS
 	sechdrs[symindex].sh_flags |= SHF_ALLOC;
 	sechdrs[strindex].sh_flags |= SHF_ALLOC;
 #endif
+#ifdef CONFIG_MARKERS
+	sechdrs[markersindex].sh_flags |= SHF_ALLOC;
+#else
+	sechdrs[markersindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
+#endif
 
 	/* Check module struct version now, before we try to use module. */
 	if (!check_modstruct_version(sechdrs, versindex, mod)) {
@@ -1642,6 +1797,11 @@ #endif
 	mod->gpl_future_syms = (void *)sechdrs[gplfutureindex].sh_addr;
 	if (gplfuturecrcindex)
 		mod->gpl_future_crcs = (void *)sechdrs[gplfuturecrcindex].sh_addr;
+	if (markersindex) {
+		mod->markers = (void *)sechdrs[markersindex].sh_addr;
+		mod->num_markers =
+			sechdrs[markersindex].sh_size / sizeof(*mod->markers);
+	}
 
 #ifdef CONFIG_MODVERSIONS
 	if ((mod->num_syms && !crcindex) || 
---END---


---BEGIN---
/* probe.c
 *
 * Loads a function at a marker call site.
 *
 * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
 *
 * This file is released under the GPLv2.
 * See the file COPYING for more details.
 */

#include <linux/marker.h>
#include <linux/module.h>
#include <linux/kallsyms.h>

/* function to install */
#define DO_MARK1_FORMAT "%d"
asmlinkage void do_mark1(const char *format, int value)
{
	__mark_check_format(DO_MARK1_FORMAT, value);
	printk("value is %d\n", value);
}

#define DO_MARK2_FORMAT "%d %s"
asmlinkage void do_mark2(const char *format, int value, const char *string)
{
	__mark_check_format(DO_MARK2_FORMAT, value, string);
	printk("value is %d %s\n", value, string);
}

#define DO_MARK3_FORMAT "%d %s %s"
asmlinkage void do_mark3(const char *format, int value, const char *s1,
				const char *s2)
{
	__mark_check_format(DO_MARK3_FORMAT, value, s1, s2);
	printk("value is %d %s %s\n", value, s1, s2);
}

int init_module(void)
{
	int result;
	result = marker_set_probe("subsys_mark1", DO_MARK1_FORMAT,
			(marker_probe_func*)do_mark1);
	if(!result) goto end;
	result = marker_set_probe("subsys_mark2", DO_MARK2_FORMAT,
			(marker_probe_func*)do_mark2);
	if(!result) goto cleanup1;
	result = marker_set_probe("subsys_mark3", DO_MARK3_FORMAT,
			(marker_probe_func*)do_mark3);
	if(!result) goto cleanup2;

	return 0;

cleanup2:
	marker_remove_probe((marker_probe_func*)do_mark2);
cleanup1:
	marker_remove_probe((marker_probe_func*)do_mark1);
end:
	return -EPERM;
}

void cleanup_module(void)
{
	marker_remove_probe((marker_probe_func*)do_mark1);
	marker_remove_probe((marker_probe_func*)do_mark2);
	marker_remove_probe((marker_probe_func*)do_mark3);
}

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Mathieu Desnoyers");
MODULE_DESCRIPTION("Probe");
---END---



---BEGIN---
/* test-mark.c
 *
 */

#include <linux/marker.h>
#include <linux/module.h>
#include <linux/proc_fs.h>
#include <linux/sched.h>
#include <asm/ptrace.h>

volatile int x=7;

struct proc_dir_entry *pentry = NULL;

static inline void test(struct pt_regs * regs)
{
	MARK(kernel_debug_test, "%d %ld %p", 2, regs->eip, regs);
}

static int my_open(struct inode *inode, struct file *file)
{
	unsigned int i;

	for(i=0; i<2; i++) {
		MARK(subsys_mark1, "%d", 1);
		x=i;
		barrier();
	}
	MARK(subsys_mark2, "%d %s", 2, "blah2");
	MARK(subsys_mark3, "%d %s %s", x, "blah3", "blah5");
	test(NULL);
	test(NULL);

	return -EPERM;
}


static struct file_operations my_operations = {
	.open = my_open,
};

int init_module(void)
{
	unsigned int i;

	pentry = create_proc_entry("testmark", 0444, NULL);
	if (pentry)
		pentry->proc_fops = &my_operations;

	marker_list_probe(NULL);

	return 0;
}

void cleanup_module(void)
{
	remove_proc_entry("testmark", NULL);
}

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Mathieu Desnoyers");
MODULE_DESCRIPTION("Marker Test");
---END---

OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
  2006-09-25 15:16 [PATCH] Linux Kernel Markers 0.11 for 2.6.17 Mathieu Desnoyers
@ 2006-09-25 16:01 ` Frank Ch. Eigler
  2006-09-25 23:28   ` Mathieu Desnoyers
       [not found] ` <45181CE9.1080204@goop.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Frank Ch. Eigler @ 2006-09-25 16:01 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Mathieu Desnoyers, Paul Mundt,
	linux-kernel, Jes Sorensen, Tom Zanussi, Richard J Moore,
	Michel Dagenais, Christoph Hellwig, Greg Kroah-Hartman,
	Thomas Gleixner, William Cohen, ltt-dev, systemtap, Alan Cox,
	Jeremy Fitzhardinge, Karim Yaghmour, Pavel Machek, Joe Perches,
	Randy.Dunlap, Jose R. Santos

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

Hi -

> [...]
> - It _does not_ change the compiler optimisations.

Like any similar mechanism, it does force the compiler to change its
code generation, so one can't claim this too strongly.

> [...]  Comments are welcome,

I'm still uneasy about the use of varargs.  The current code now uses
the formatting string as metadata to be matched (strcmp) between
producer and consumer.  A general tool that would use them would have
to start parsing general printf directives.  I believe they are not
quite general enough either e.g. to describe a raw binary blob.

I realize they serve a useful purpose in abbreviating what otherwise
one might have to do (like that multiplicity of STAP_MARK_* type/arity
permutations).  But maybe there is a better way.

Also, while regparm(0) may provide some comfort on x86, is there good
reason to believe that the same trick works (and will continue to
work) on non-x86 platforms to invoke a non-varargs callee with a
varargs caller?


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
       [not found] ` <45181CE9.1080204@goop.org>
@ 2006-09-25 20:16   ` Mathieu Desnoyers
       [not found]     ` <45183B20.2080907@goop.org>
  2006-09-25 23:19   ` Mathieu Desnoyers
  1 sibling, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 20:16 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Paul Mundt, linux-kernel,
	Jes Sorensen, Tom Zanussi, Richard J Moore, Michel Dagenais,
	Christoph Hellwig, Greg Kroah-Hartman, Thomas Gleixner,
	William Cohen, ltt-dev, systemtap, Alan Cox, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >Good morning everyone,
> >
> >Following Jeremy Fitzhardinge's advice, I rewrote my marker mechanism 
> >taking in
> >consideration inline functions (and therefore also unrolled loops). This 
> >new
> >marker version is a complete rewrite of the previous one. It allows :
> >
> >- Multiple occurrences of the same marker name.
> >- Declaration of a marker in an inline function.
> >- Declaration of a marker in an unrolled loop.
> >- It _does not_ change the compiler optimisations.
> >  
> 
> Well, it will a little bit. If you put a mark on a statement which would 
> have otherwise been removed, then it will not be removed; the labels 
> effectively change the potential control flow graph as far as the 
> compiler is concerned. But if marks are used appropriately the impact 
> should be pretty minimal.
> 

Yes, I agree : data dependencies are added.

> [MARK_CALL]
> >+		asm volatile(	".section .markers, \"a\";\n\t" \
> >+				".long %0, %1;\n\t" \
> >+				".previous;\n\t" : : \
> >  
> [MARK_JUMP]
> >+		asm volatile(	".section .markers, \"a\";\n\t" \
> >+				".long %0, %1, %2;\n\t" \
> >+				".previous;\n\t" : : \
> >+			"m" (*&&jump_select_label), \
> >+			"m" (*&&call_label), \
> >+			"m" (*&&over_label)); \
> >  
> 
> If you're going to put different types in the .markers section 
> (presumably per-architecture, rather than different types for within one 
> architecture) you should probably also define a structure in the same 
> place, if nothing
> 

Yes, it would probably be useful in some situations, I will correct this.

> >+		asm volatile (	".align 16;\n\t" : : ); \
> >+		asm volatile (	".byte 0xeb;\n\t" : : ); \
> >+jump_select_label: \
> >+		asm volatile (	".byte %0-%1;\n\t" : : \
> >+				"m" (*&&over_label), "m" (*&&call_label)); \
> >  
> 
> There's absolutely nothing to guarantee that these three asm() will be 
> kept together in the generated code, or in the same place with respect 
> to any other asms.
> 

I could declare my jump_select_label directly in assembly then.

> >+call_label: \
> >+		asm volatile ("" : : ); \
> >+		MARK_CALL(name, format, ## args); \
> >+		asm volatile ("" : : ); \
> >+over_label: \
> >+		asm volatile ("" : : ); \
> >  
> 
> These asm volatiles won't do anything at all. What are you trying to 
> achieve?
> 

I want to make sure that the call_label's address will be exactly after the 2nd
byte of the jump instruction. The over_label does not really matter, as long as
it points to a correct spot in the execution flow. The most important is that
it stays near the jump instruction.

I could probably do all this in assembly too.

> >+#ifdef CONFIG_MARKERS
> >+#define MARK(name, format, args...) \
> >+	do { \
> >+		__label__ here; \
> >+here:   	asm volatile(	".section .markers, \"a\";\n\t" \
> >+				".long %0, %1;\n\t" \
> >+				".previous;\n\t" : : \
> >+			"m" (*(#name)), \
> >+			"m" (*&&here)); \
> >  
> 
> Seems like a bad idea that MARK() can put one type of record in 
> .markers, but MARK_JUMP and MARK_CALL can put different records in the 
> same section? How do you distinguish them? Or are they certain to be 
> exclusive? Either way, I'd probably put different mark records in 
> different sections: .markers.jump, .markers.call, markers.labels. And 
> define appropriate structures for the record types in each section.
> 


struct __mark_marker {
        const char *name;
        const void *location;
        char *select;
        const void *jump_call;
        const void *jump_over;
        marker_probe_func **call;
        const char *format;
};

is the structure which defines a complete record in the mark section. They are
all tied to the same marker site, so I think it makes sense to keep them in the
same record.


> Also, expecting to call a varargs function from a non-varargs callsite 
> is skating on very thin ice. Lots of architectures have very different 
> calling conventions for varadic vs non-varadic functions, and I wouldn't 
> rely on being able to make any sweeping generalizations about it. 
> regparm is only documented to do anything on i386; it almost certainly 
> won't make a non-varadic callsite look like a varadic call to a varadic 
> function on architectures who's ABIs use different conventions for the 
> two types of function.
> 

Right, well, I wanted to keep a generic caller and try to make assumptions about
the stack layout in the called function but if there is now way to do this, we
can think of using the varargs in the probe.

Mathieu


OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
       [not found]     ` <45183B20.2080907@goop.org>
@ 2006-09-25 20:45       ` Mathieu Desnoyers
  2006-09-25 20:57         ` Mathieu Desnoyers
  0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 20:45 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Paul Mundt, linux-kernel,
	Jes Sorensen, Tom Zanussi, Richard J Moore, Michel Dagenais,
	Christoph Hellwig, Greg Kroah-Hartman, Thomas Gleixner,
	William Cohen, ltt-dev, systemtap, Alan Cox, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >I could declare my jump_select_label directly in assembly then.
> >  
> 
> Maybe, but it could be tricky to make that label visible to C code.
> 
> >>>+call_label: \
> >>>+		asm volatile ("" : : ); \
> >>>+		MARK_CALL(name, format, ## args); \
> >>>+		asm volatile ("" : : ); \
> >>>+over_label: \
> >>>+		asm volatile ("" : : ); \
> >>> 
> >>>      
> >>These asm volatiles won't do anything at all. What are you trying to 
> >>achieve?
> >>    
> >
> >I want to make sure that the call_label's address will be exactly after 
> >the 2nd
> >byte of the jump instruction. The over_label does not really matter, as 
> >long as
> >it points to a correct spot in the execution flow. The most important is 
> >that
> >it stays near the jump instruction.
> >  
> 
> The "volatile" modifier for "asm" *only* means that the asm emitted if 
> the code is reachable at all; it doesn't make any constraints about 
> relative ordering of the various asm volatile statement with respect to 
> each other, or with respect to other code.
> 
> >I could probably do all this in assembly too.
> >  
> 
> Perhaps, though doing as much as possible visible to gcc has its 
> benefits.  Tricky either way.
> 

Would it be correct if we put dependencies on a label corresponding to the
previous asm in the read constraints for each asm ?

> >>>+#ifdef CONFIG_MARKERS
> >>>+#define MARK(name, format, args...) \
> >>>+	do { \
> >>>+		__label__ here; \
> >>>+here:   	asm volatile(	".section .markers, \"a\";\n\t" \
> >>>+				".long %0, %1;\n\t" \
> >>>+				".previous;\n\t" : : \
> >>>+			"m" (*(#name)), \
> >>>+			"m" (*&&here)); \
> >>> 
> >>>      
> >>Seems like a bad idea that MARK() can put one type of record in 
> >>.markers, but MARK_JUMP and MARK_CALL can put different records in the 
> >>same section? How do you distinguish them? Or are they certain to be 
> >>exclusive? Either way, I'd probably put different mark records in 
> >>different sections: .markers.jump, .markers.call, markers.labels. And 
> >>define appropriate structures for the record types in each section.
> >>
> >>    
> >
> >
> >struct __mark_marker {
> >        const char *name;
> >        const void *location;
> >        char *select;
> >        const void *jump_call;
> >        const void *jump_over;
> >        marker_probe_func **call;
> >        const char *format;
> >};
> >
> >is the structure which defines a complete record in the mark section. They 
> >are
> >all tied to the same marker site, so I think it makes sense to keep them 
> >in the
> >same record.
> >  
> 
> I don't understand.  Your asms put things into the marker section with 
> ".long A, B, C".  Does does that correspond to this structure?
> 

Yes, those are all pointers and a single MARK declares 7 of them. Please tell
me if I goofed up in assembly typing.

Regards,

Mathieu

OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
  2006-09-25 20:45       ` Mathieu Desnoyers
@ 2006-09-25 20:57         ` Mathieu Desnoyers
       [not found]           ` <45184885.8020807@goop.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 20:57 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Paul Mundt, linux-kernel,
	Jes Sorensen, Tom Zanussi, Richard J Moore, Michel Dagenais,
	Christoph Hellwig, Greg Kroah-Hartman, Thomas Gleixner,
	William Cohen, ltt-dev, systemtap, Alan Cox, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

* Mathieu Desnoyers (compudj@krystal.dyndns.org) wrote:
> 
> Would it be correct if we put dependencies on a label corresponding to the
> previous asm in the read constraints for each asm ?
> 

Better idea : we could put a read/write dependency on a memory location.

Mathieu

OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
       [not found]           ` <45184885.8020807@goop.org>
@ 2006-09-25 21:42             ` Mathieu Desnoyers
  0 siblings, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 21:42 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Paul Mundt, linux-kernel,
	Jes Sorensen, Tom Zanussi, Richard J Moore, Michel Dagenais,
	Christoph Hellwig, Greg Kroah-Hartman, Thomas Gleixner,
	William Cohen, ltt-dev, systemtap, Alan Cox, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >Better idea : we could put a read/write dependency on a memory location.
> >  
> 
> Yes, that works well.  And it needn't even exist:
> 
> 	extern int __marker_sequencer;		/* doesn't exist, never 
> 	referenced */
> 
> 	asm volatile("first asm" : "+m" (__marker_sequencer));
> 
> 	asm volatile("second asm" : "+m" (__marker_sequencer));
> 
> This keeps the asms ordered with respect to each other (and prevents to 
> independent markers from being intermingled), but it doesn't prevent 
> them from being re-ordered with respect to other code.
> 
I will declare the pointers around the jmp instruction directly in assembly : I
wouldn't want gcc to put some other code there by mistake.

I will use the "name" variable, as it is already there.

A new version coming soon...

Thank you very much!

Mathieu

OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
       [not found] ` <45181CE9.1080204@goop.org>
  2006-09-25 20:16   ` Mathieu Desnoyers
@ 2006-09-25 23:19   ` Mathieu Desnoyers
  1 sibling, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 23:19 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Martin Bligh, Frank Ch. Eigler, Masami Hiramatsu, prasanna,
	Andrew Morton, Ingo Molnar, Paul Mundt, linux-kernel,
	Jes Sorensen, Tom Zanussi, Richard J Moore, Michel Dagenais,
	Christoph Hellwig, Greg Kroah-Hartman, Thomas Gleixner,
	William Cohen, ltt-dev, systemtap, Alan Cox, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> If you're going to put different types in the .markers section 
> (presumably per-architecture, rather than different types for within one 
> architecture) you should probably also define a structure in the same 
> place, if nothing
> 

For now, I only expect two kinds of structures :

One for the architectures which implements the MARK_JUMP optimisation and one
where they don't. Does it make sense to assume that each architecture will offer
the possibility to write a 1 byte offset calculated from the difference between
two addresses ? If not, then, it can be useful to think of a per archtecture
structure, but module.c shall be modified accordingly too. So for now, I would
stay with only one definition in linux/marker.h and if we see the need for it,
we can put the structures in asm-*/marker.h and adapt module.c per architecture
accordingly (probably by using a macro).

Mathieu

OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
  2006-09-25 16:01 ` Frank Ch. Eigler
@ 2006-09-25 23:28   ` Mathieu Desnoyers
  2006-09-26 16:40     ` Frank Ch. Eigler
  0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-25 23:28 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Martin Bligh, Masami Hiramatsu, prasanna, Andrew Morton,
	Ingo Molnar, Paul Mundt, linux-kernel, Jes Sorensen, Tom Zanussi,
	Richard J Moore, Michel Dagenais, Christoph Hellwig,
	Greg Kroah-Hartman, Thomas Gleixner, William Cohen, ltt-dev,
	systemtap, Alan Cox, Jeremy Fitzhardinge, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]

Hi,

* Frank Ch. Eigler (fche@redhat.com) wrote:
> Hi -
> 
> > [...]
> > - It _does not_ change the compiler optimisations.
> 
> Like any similar mechanism, it does force the compiler to change its
> code generation, so one can't claim this too strongly.
> 
Yes, memory dependencies are changed, you are right. I was principally talking
about the inline and unrolled loops optimisations.


> > [...]  Comments are welcome,
> 
> I'm still uneasy about the use of varargs.  The current code now uses
> the formatting string as metadata to be matched (strcmp) between
> producer and consumer.  A general tool that would use them would have
> to start parsing general printf directives.

If you want to generate probes automatically, yes.

> I believe they are not
> quite general enough either e.g. to describe a raw binary blob.
> 
If you want to dump a raw binary blob, what about :
MARK(mysubsys_myevent, "char %p %u", blobptr, blobsize);
where %p is a pointer to an array of char and %u the length ?

My idea is to use the string to identify what is referred by a pointer, so it
can be casted into this type with some kind of coherency between the marker and
the probe.

> I realize they serve a useful purpose in abbreviating what otherwise
> one might have to do (like that multiplicity of STAP_MARK_* type/arity
> permutations).  But maybe there is a better way.
> 

I think that duplicating the number of marker macros could easily make
them unflexible and ugly. This is why I am trying to come with this generic
macro.

> Also, while regparm(0) may provide some comfort on x86, is there good
> reason to believe that the same trick works (and will continue to
> work) on non-x86 platforms to invoke a non-varargs callee with a
> varargs caller?
> 

Good point, I will setup a va_args in the probe. When correctly used, however,
there is no need to use the format string : we can directly get the variables
from the var arg list if we know in advance what the string will be.

Mathieu

OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
  2006-09-25 23:28   ` Mathieu Desnoyers
@ 2006-09-26 16:40     ` Frank Ch. Eigler
  2006-09-27  1:30       ` Mathieu Desnoyers
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Ch. Eigler @ 2006-09-26 16:40 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Martin Bligh, Masami Hiramatsu, prasanna, Andrew Morton,
	Ingo Molnar, Paul Mundt, linux-kernel, Jes Sorensen, Tom Zanussi,
	Richard J Moore, Michel Dagenais, Christoph Hellwig,
	Greg Kroah-Hartman, Thomas Gleixner, William Cohen, ltt-dev,
	systemtap, Alan Cox, Jeremy Fitzhardinge, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

Mathieu Desnoyers <compudj@krystal.dyndns.org> writes:

> [...]
> > I believe [printf formatting directives] are not
> > quite general enough either e.g. to describe a raw binary blob.
>
> If you want to dump a raw binary blob, what about :
> MARK(mysubsys_myevent, "char %p %u", blobptr, blobsize); where %p is
> a pointer to an array of char and %u the length ?

That involves new conventions beyond printf.  Why not "%p %p %u %u"
for two blobs ... or why implicitly dereference the given pointers.  A
probe handler unaware of a specific marker's semantics would not know
whether or not this is implied.


> My idea is to use the string to identify what is referred by a
> pointer, so it can be casted into this type with some kind of
> coherency between the marker and the probe.

I understand what you're using them for.  To me, they just don't look
like a good fit.


> > I realize they serve a useful purpose in abbreviating what otherwise
> > one might have to do (like that multiplicity of STAP_MARK_* type/arity
> > permutations).  [...]
> 
> I think that duplicating the number of marker macros could easily make
> them unflexible and ugly. [...]

Inflexible and ugly in what way?  Remember, the macro definitions can
be automatically generated.  At the macro call site, there needs to be
little difference.


> [...]  Good point, I will setup a va_args in the probe.  When
> correctly used, however, there is no need to use the format string :
> we can directly get the variables from the var arg list if we know
> in advance what the string will be.

Do I understand you correctly that the probe handlers would be given
va_list values, and would have to call va_arg to yank out individual
actual arguments?  So again type safety is a matter of explicit coding
(equivalent to correctly casting each type)?


- FChE

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
  2006-09-26 16:40     ` Frank Ch. Eigler
@ 2006-09-27  1:30       ` Mathieu Desnoyers
  2006-09-27 16:12         ` Frank Ch. Eigler
  0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2006-09-27  1:30 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Martin Bligh, Masami Hiramatsu, prasanna, Andrew Morton,
	Ingo Molnar, Paul Mundt, linux-kernel, Jes Sorensen, Tom Zanussi,
	Richard J Moore, Michel Dagenais, Christoph Hellwig,
	Greg Kroah-Hartman, Thomas Gleixner, William Cohen, ltt-dev,
	systemtap, Alan Cox, Jeremy Fitzhardinge, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

Hi Frank,

* Frank Ch. Eigler (fche@redhat.com) wrote:
> Mathieu Desnoyers <compudj@krystal.dyndns.org> writes:
> 
> > [...]
> > > I believe [printf formatting directives] are not
> > > quite general enough either e.g. to describe a raw binary blob.
> >
> > If you want to dump a raw binary blob, what about :
> > MARK(mysubsys_myevent, "char %p %u", blobptr, blobsize); where %p is
> > a pointer to an array of char and %u the length ?
> 
> That involves new conventions beyond printf.  Why not "%p %p %u %u"
> for two blobs ... or why implicitly dereference the given pointers.  A
> probe handler unaware of a specific marker's semantics would not know
> whether or not this is implied.
> 

The idea is to be able to access to a data unit (here, a bin blob) described in
one or more consecutive statements in the string. Why doing %p %u %p %u for two
binary blobs ? Suppose you are creating a probe sub-function that takes a
va_list as parameter and has to extract a such a bin blob. It will expect one
pointer and a size. It simply has to get them sequentially with va_arg. It makes
things much more difficult if you decide to put all the sizes at the end.

So yes, there is a semantic to create, but I don't see the problem with that.

And why would the probe actually know what to do with a pointer ? If it only
wants to record the pointer's address or if it wants to access data inside this
pointer, it's up to the probe (or automatic probe generator, hum ?) to do it.

> 
> > My idea is to use the string to identify what is referred by a
> > pointer, so it can be casted into this type with some kind of
> > coherency between the marker and the probe.
> 
> I understand what you're using them for.  To me, they just don't look
> like a good fit.
> 
> 
> > > I realize they serve a useful purpose in abbreviating what otherwise
> > > one might have to do (like that multiplicity of STAP_MARK_* type/arity
> > > permutations).  [...]
> > 
> > I think that duplicating the number of marker macros could easily make
> > them unflexible and ugly. [...]
> 
> Inflexible and ugly in what way?  Remember, the macro definitions can
> be automatically generated.  At the macro call site, there needs to be
> little difference.
> 

I don't expect the kernel programmer community to accept that their code will
call an automatically generated macro. It removes all the idea of "I can see
what code is actually generated by my function", which I believe is necessary.

Also, people are used to the simplicity and flexibility of printf style format
strings. Do you really expect people to start using various macros like 
MARK_u_p_llu and start defining their own marker macro each time they want to
add a specific type ?

> 
> > [...]  Good point, I will setup a va_args in the probe.  When
> > correctly used, however, there is no need to use the format string :
> > we can directly get the variables from the var arg list if we know
> > in advance what the string will be.
> 
> Do I understand you correctly that the probe handlers would be given
> va_list values, and would have to call va_arg to yank out individual
> actual arguments?

Yes. I want to minimize the visual impact of the marker in the code while
making it self-describing and easy to inspect by a kernel programmer.

> So again type safety is a matter of explicit coding
> (equivalent to correctly casting each type)?
> 

If someone chooses to create their own probes, yes, they must to exactly that.
However, if you want to create probes that are type-safe, you can then create a
script that will extract all the format strings in the markers section of the
object and automatically generate all the probes with their respective va_args
setup at the beginning of the probe. By using the string verification upon
connexion of the probe, type safety should be insured.

My point is that type safety should not exclusively be the marker mechanism's
burden : there are other tools (probe generators) involved in that process.

Mathieu


OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Linux Kernel Markers 0.11 for 2.6.17
  2006-09-27  1:30       ` Mathieu Desnoyers
@ 2006-09-27 16:12         ` Frank Ch. Eigler
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Ch. Eigler @ 2006-09-27 16:12 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Martin Bligh, Masami Hiramatsu, prasanna, Andrew Morton,
	Ingo Molnar, Paul Mundt, linux-kernel, Jes Sorensen, Tom Zanussi,
	Richard J Moore, Michel Dagenais, Christoph Hellwig,
	Greg Kroah-Hartman, Thomas Gleixner, William Cohen, ltt-dev,
	systemtap, Alan Cox, Jeremy Fitzhardinge, Karim Yaghmour,
	Pavel Machek, Joe Perches, Randy.Dunlap, Jose R. Santos

Hi -

Mathieu Desnoyers <compudj@krystal.dyndns.org> writes:

> [...]
> > That involves new conventions beyond printf.  Why not "%p %p %u %u"
> > for two blobs ... or why implicitly dereference the given pointers.  A
> > probe handler unaware of a specific marker's semantics would not know
> > whether or not this is implied.
> 
> [...]
> So yes, there is a semantic to create, but I don't see the problem with that.

That's a part of my point.  The marker data types marked up with
printf directives do not fully describe the data - in this case
whether it is a raw pointer or a data blob that is being marked.

> And why would the probe actually know what to do with a pointer ? If
> it only wants to record the pointer's address or if it wants to
> access data inside this pointer, it's up to the probe (or automatic
> probe generator, hum ?) to do it.

Of course, but that precludes a general client tool, such as (say) a
trace-only handler.

> > > I think that duplicating the number of marker macros could easily make
> > > them unflexible and ugly. [...]
> > 
> > Inflexible and ugly in what way?  [...]
> 
> I don't expect the kernel programmer community to accept that their code will
> call an automatically generated macro. It removes all the idea of "I can see
> what code is actually generated by my function", which I believe is necessary.

Not at all - the generated macros can sit in-tree and are easily
inspected.  Check out gen-stapmark.h and stapmark.h at
<http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/?cvsroot=systemtap>.

> Also, people are used to the simplicity and flexibility of printf
> style format strings.

True, but is this in context of the existing tracing/probing
facilities?  Unless I'm mistaken, ltt functions doesn't use them; nor
does blktrace.  Other than printk, are there any?

> Do you really expect people to start using various macros like
> MARK_u_p_llu

I don't know.  Using gcc extensions such as __builtin_typeof() could
automate the typing aspect, leaving only the arity as a
programmer-visible text.

> and start defining their own marker macro each time they want to add
> a specific type ?

Well, adding a new type would be at last as hard in the printf case.


> [...] However, if you want to create probes that are type-safe, you
> can then create a script that will extract all the format strings in
> the markers section of the object and automatically generate all the
> probes with their respective va_args setup at the beginning of the
> probe. [...]

This could work.  OTOH this relies on an as-yet-unwritten script, and
additional run-time costs (the parameter-by-parameter va_arg copying).

I wonder if writing a functional back-end for these markers should be
considered a corequisite for this work; or in the alternative, whether
it's good enough to start putting markers into the code, and revamp
their implementation later if necessary.


- FChE

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2006-09-27 16:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-09-25 15:16 [PATCH] Linux Kernel Markers 0.11 for 2.6.17 Mathieu Desnoyers
2006-09-25 16:01 ` Frank Ch. Eigler
2006-09-25 23:28   ` Mathieu Desnoyers
2006-09-26 16:40     ` Frank Ch. Eigler
2006-09-27  1:30       ` Mathieu Desnoyers
2006-09-27 16:12         ` Frank Ch. Eigler
     [not found] ` <45181CE9.1080204@goop.org>
2006-09-25 20:16   ` Mathieu Desnoyers
     [not found]     ` <45183B20.2080907@goop.org>
2006-09-25 20:45       ` Mathieu Desnoyers
2006-09-25 20:57         ` Mathieu Desnoyers
     [not found]           ` <45184885.8020807@goop.org>
2006-09-25 21:42             ` Mathieu Desnoyers
2006-09-25 23:19   ` Mathieu Desnoyers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).