Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

Just as a precision :

The following sequence (please refer to the code below for local symbols
1 and 2) :

1:
preempt_disable()
call (*__mark_call_##name)(format, ## args);
preempt_enable_no_resched()
2:

is insured because :

1 is part of an inline assembly with rw dependency on __marker_sequencer
the call is surrounded by memory barriers.
2 is part of an inline assembly with rw dependency on __marker_sequencer

Mathieu

* Mathieu Desnoyers (compudj@krystal.dyndns.org) wrote:
> +++ 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 %1, %2;\n\t" \
> +				".previous;\n\t" : "+m" (__marker_sequencer) : \
> +			"m" (__mark_call_##name), \
> +			"m" (*format)); \
> +		preempt_disable(); \
> +		(*__mark_call_##name)(format, ## args); \
> +		preempt_enable_no_resched(); \
> +	} while(0)

> --- /dev/null
> +++ b/include/asm-i386/marker.h
> @@ -0,0 +1,30 @@
> +/*
> + * 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 { \
> +		asm volatile(	".section .markers, \"a\";\n\t" \
> +				".long 0f, 1f, 2f;\n\t" \
> +				".previous;\n\t" \
> +				".align 16;\n\t" \
> +				".byte 0xeb;\n\t" \
> +				"0:\n\t" \
> +				".byte 2f-1f;\n\t" \
> +				"1:\n\t" \
> +			: "+m" (__marker_sequencer) : ); \
> +		MARK_CALL(name, format, ## args); \
> +		asm volatile (	"2:\n\t" : "+m" (__marker_sequencer) : ); \
> +	} while(0)
OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

[PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

Hi,

Here is a corrected version. Following Jeremy Fitzhardinge's advices :
- I now use memory constraints on an extern symbol to keep inline assembly in
  the right order.
- I declare the symbols around and inside the 2 bytes jump in assembly to make
  sure that gcc will not insert code, padding, etc... All the assembly
  alignment+instruction is all in one inline asm now.
- Use variable argument list in the probe (safe on every architecture). Thanks
  to Frank and Jeremy for pointing this out.

Thanks for all your comments,

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..f18ee68
--- /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 %1, %2;\n\t" \
+				".previous;\n\t" : "+m" (__marker_sequencer) : \
+			"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..4f1e30c
--- /dev/null
+++ b/include/asm-i386/marker.h
@@ -0,0 +1,30 @@
+/*
+ * 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 { \
+		asm volatile(	".section .markers, \"a\";\n\t" \
+				".long 0f, 1f, 2f;\n\t" \
+				".previous;\n\t" \
+				".align 16;\n\t" \
+				".byte 0xeb;\n\t" \
+				"0:\n\t" \
+				".byte 2f-1f;\n\t" \
+				"1:\n\t" \
+			: "+m" (__marker_sequencer) : ); \
+		MARK_CALL(name, format, ## args); \
+		asm volatile (	"2:\n\t" : "+m" (__marker_sequencer) : ); \
+	} 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..2916e53
--- /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 <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 %1, %2;\n\t" \
+				".previous;\n\t" : "+m" (__marker_sequencer) : \
+			"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 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);
+extern int __marker_sequencer; /* doesn't exist, never referenced */
+
+#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..083cb00 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
+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"
void do_mark1(const char *format, ...)
{
	va_list ap;
	int value;

	va_start(ap, format);
	value = va_arg(ap, int);
	printk("value is %d\n", value);
	
	va_end(ap);
}

void do_mark2(const char *format, ...)
{
	va_list ap;

	va_start(ap, format);
	vprintk(format, ap);
	va_end(ap);
	printk("\n");
}

#define DO_MARK3_FORMAT "%d %s %s"
void do_mark3(const char *format, ...)
{
	va_list ap;
	int value;
	const char *s1, *s2;
	
	va_start(ap, format);
	value = va_arg(ap, int);
	s1 = va_arg(ap, const char*);
	s2 = va_arg(ap, const char *);

	printk("value is %d %s %s\n",
		value, s1, s2);
	va_end(ap);
}

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", NULL,
			(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);
	}
	MARK(subsys_mark2, "%d %s %s", 2, "blah2", "blahx");
	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)
{
	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 

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >Just as a precision :
> >
> >The following sequence (please refer to the code below for local symbols
> >1 and 2) :
> >
> >1:
> >preempt_disable()
> >call (*__mark_call_##name)(format, ## args);
> >preempt_enable_no_resched()
> >2:
> >
> >is insured because :
> >
> >1 is part of an inline assembly with rw dependency on __marker_sequencer
> >the call is surrounded by memory barriers.
> >2 is part of an inline assembly with rw dependency on __marker_sequencer
> >  
> 
> What do you mean the call is surrounded by memory barriers?  Do you mean 
> a call has an implicit barrier, or something else?
> 
Yes, preempt_disable() has a barrier(), on gcc :
__asm__ __volatile__("": : :"memory").


> Either way, this doesn't prevent some otherwise unrelated 
> non-memory-using code from being scheduled in there, which would not be 
> executed.  The gcc manual really strongly discourages jumping between 
> inline asms, because they have basically unpredictable results.
> 

Ok, I will do the call in assembly then.

Mathieu


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

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> Yes, preempt_disable() has a barrier(), on gcc :
> __asm__ __volatile__("": : :"memory").
> 
> 
> > Either way, this doesn't prevent some otherwise unrelated 
> > non-memory-using code from being scheduled in there, which would not be 
> > executed.  The gcc manual really strongly discourages jumping between 
> > inline asms, because they have basically unpredictable results.
> > 
> 
> Ok, I will do the call in assembly then.
> 

Before I rush on a solution too fast... I have a question for you :

To protect code from being preempted, the macros preempt_disable and
preempt_enable must normally be used. Logically, this macro must make sure gcc
doesn't interleave preemptible code and non-preemptible code.

Starting with this hypothesis, what makes gcc aware of this ? If we check
preempt_disable (the disable call is almost symmetric) :

linux/preempt.h:
define add_preempt_count(val) do { preempt_count() += (val); } while (0)

#define inc_preempt_count() add_preempt_count(1)

#define preempt_disable() \
do { \
        inc_preempt_count(); \
        barrier(); \
} while (0)

So the magic must be in the barrier() macro :

linux/compiler-gcc.h:
/* Optimization barrier */
/* The "volatile" is due to gcc bugs */
#define barrier() __asm__ __volatile__("": : :"memory")

Which makes me think that if I put barriers around my asm, call, asm trio, no
other code will be interleaved. Is it right ?

Mathieu


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

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

First of all, yes, gcc seems not to allow a programmer to jump between asm
labels without painfully verifying everything, as stated in 
http://developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Extended-Asm.html
"Speaking of labels, jumps from one asm to another are not supported. The
compiler's optimizers do not know about these jumps, and therefore they
cannot take account of them when deciding how to optimize."

however... (see below)

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >To protect code from being preempted, the macros preempt_disable and
> >preempt_enable must normally be used. Logically, this macro must make sure 
> >gcc
> >doesn't interleave preemptible code and non-preemptible code.
> >  
> 
> No, it only needs to prevent globally visible side-effects from being 
> moved into/out of preemptable blocks.  In practice that means memory 
> updates (including the implicit ones that calls to external functions 
> are assumed to make).
> 
> >Which makes me think that if I put barriers around my asm, call, asm trio, 
> >no
> >other code will be interleaved. Is it right ?
> >  
> 
> No global side effects, but code with local side effects could be moved 
> around without changing the meaning of preempt.
> 
> For example:
> 
> 	int foo;
> 	extern int global;
> 
> 	foo = some_function();
> 
> 	foo += 42;
> 
> 	preempt_disable();
> 	// stuff
> 	preempt_enable();
> 
> 	global = foo;
> 	foo += other_thing();
> 
> Assume here that some_function and other_function are extern, and so gcc 
> has no insight into their behaviour and therefore conservatively assumes 
> they have global side-effects.
> 
> The memory barriers in preempt_disable/enable will prevent gcc from 
> moving any of the function calls into the non-preemptable region. But 
> because "foo" is local and isn't visible to any other code, there's no 
> reason why the "foo += 42" couldn't move into the preempt region.  

I am not sure about this last statement. The same reference :
http://developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Extended-Asm.html

States :
"If your assembler instructions access memory in an unpredictable fashion,
add memory to the list of clobbered registers. This will cause GCC to not
keep memory values cached in registers across the assembler instruction
and not optimize stores or loads to that memory. You will also want to
add the volatile keyword if the memory affected is not listed in the
inputs or outputs of the asm, as the memory clobber does not count as
a side-effect of the asm. If you know how large the accessed memory is,
you can add it as input or output but if this is not known, you should
add memory.  As an example, if you access ten bytes of a string, you can
use a memory input like:

  {"m"( ({ struct { char x[10]; } *p = (void *)ptr ; *p; }) )} "


I am just wondering how gcc can assume that I will not modify variables on the
stack from within a function with a memory clobber. If I would like to do some
nasty things in my assembly code, like accessing directly to a local variable by
using an offset from the stack pointer, I would expect gcc not to relocate this
local variable around my asm volatile memory clobbered statement, as it falls
under the category "access memory in an unpredictable fashion".

Mathieu

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

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

Hi,

Ok, so as far as I can see, we can only control the execution flow by modifying
values in the output list of the asm.

Do you think the following would work ?


#define MARK_JUMP(name, format, args...) \
        do { \
                char condition; \
                asm volatile(   ".section .markers, \"a\";\n\t" \
                                        ".long 0f;\n\t" \
                                        ".previous;\n\t" \
                                        "0:\n\t" \
                                        "movb $0,%1;\n\t" \
                                : "+m" (__marker_sequencer), \
                                "=r" (condition) : ); \
                if(unlikely(condition)) { \
                        MARK_CALL(name, format, ## args); \
                } \
        } while(0)

The jump is left to gcc, we only modify an immediate value (a byte) to change
the selection. The is no memory load involved on the fast path :

...
   6:   b0 00                   mov    $0x0,%al
   8:   84 c0                   test   %al,%al
   a:   75 07                   jne    13 <my_open+0x13>
   c:   b8 ff ff ff ff          mov    $0xffffffff,%eax
  11:   c9                      leave  
  12:   c3                      ret    
  13:   b8 01 00 00 00          mov    $0x1,%eax
  18:   e8 fc ff ff ff          call   19 <my_open+0x19>
  1d:   c7 44 24 0c 00 00 00    movl   $0x0,0xc(%esp)
  24:   00 
  25:   c7 44 24 08 06 00 00    movl   $0x6,0x8(%esp)
  2c:   00 
  2d:   c7 44 24 04 02 00 00    movl   $0x2,0x4(%esp)
  34:   00 
  35:   c7 04 24 0c 00 00 00    movl   $0xc,(%esp)
  3c:   ff 15 94 00 00 00       call   *0x94
  42:   b8 01 00 00 00          mov    $0x1,%eax
  47:   e8 fc ff ff ff          call   48 <my_open+0x48>
  4c:   eb be                   jmp    c <my_open+0xc>


Mathieu

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >>Mathieu Desnoyers wrote:
> >>    
> >>>To protect code from being preempted, the macros preempt_disable and
> >>>preempt_enable must normally be used. Logically, this macro must make 
> >>>sure gcc
> >>>doesn't interleave preemptible code and non-preemptible code.
> >>> 
> >>>      
> >>No, it only needs to prevent globally visible side-effects from being 
> >>moved into/out of preemptable blocks.  In practice that means memory 
> >>updates (including the implicit ones that calls to external functions 
> >>are assumed to make).
> >>
> >>    
> >>>Which makes me think that if I put barriers around my asm, call, asm 
> >>>trio, no
> >>>other code will be interleaved. Is it right ?
> >>> 
> >>>      
> >>No global side effects, but code with local side effects could be moved 
> >>around without changing the meaning of preempt.
> >>
> >>For example:
> >>
> >>	int foo;
> >>	extern int global;
> >>
> >>	foo = some_function();
> >>
> >>	foo += 42;
> >>
> >>	preempt_disable();
> >>	// stuff
> >>	preempt_enable();
> >>
> >>	global = foo;
> >>	foo += other_thing();
> >>
> >>Assume here that some_function and other_function are extern, and so gcc 
> >>has no insight into their behaviour and therefore conservatively assumes 
> >>they have global side-effects.
> >>
> >>The memory barriers in preempt_disable/enable will prevent gcc from 
> >>moving any of the function calls into the non-preemptable region. But 
> >>because "foo" is local and isn't visible to any other code, there's no 
> >>reason why the "foo += 42" couldn't move into the preempt region.  
> >>    
> >
> >I am not sure about this last statement. The same reference :
> >http://developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Extended-Asm.html
> >  
> (This is pretty old, and this is an area which changes quite a lot.  You 
> should refer to something more recent;
> http://www.cims.nyu.edu/cgi-systems/info2html?/usr/local/info(gcc)Top 
> for example, though in this case the quoted text looks the same.)
> 
> >I am just wondering how gcc can assume that I will not modify variables on 
> >the
> >stack from within a function with a memory clobber. If I would like to do 
> >some
> >nasty things in my assembly code, like accessing directly to a local 
> >variable by
> >using an offset from the stack pointer, I would expect gcc not to relocate 
> >this
> >local variable around my asm volatile memory clobbered statement, as it 
> >falls
> >under the category "access memory in an unpredictable fashion".
> >  
> 
> That not really what it means.  gcc is free to put local variables in 
> memory or register, and unless you pass the local to your asm as a 
> parameter, your code has no way of knowing how to find the current 
> location of the local.  You could trash your stack frame from within the 
> asm if you like, but I don't think gcc is under any obligation to behave 
> in a deterministic way if you do.
> 
> "Unpredictable" in this case means that the memory modified isn't easily 
> specified as a normal asm parameter.  For example, if you have an asm 
> which does a memset(), the modified memory's size is a runtime variable 
> rather than a compile-time constant.  Or perhaps your asm follows a 
> linked list and modifies memory as it traverses the list.
> 
> 
>    J
> 
OpenPGP public key:              http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint:     8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68 

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
> >Hi,
> >
> >Ok, so as far as I can see, we can only control the execution flow by 
> >modifying
> >values in the output list of the asm.
> >
> >Do you think the following would work ?
> >
> >
> >#define MARK_JUMP(name, format, args...) \
> >        do { \
> >                char condition; \
> >                asm volatile(   ".section .markers, \"a\";\n\t" \
> >                                        ".long 0f;\n\t" \
> >                                        ".previous;\n\t" \
> >                                        "0:\n\t" \
> >                                        "movb $0,%1;\n\t" \
> >                                : "+m" (__marker_sequencer), \
> >                                "=r" (condition) : ); \
> >                if(unlikely(condition)) { \
> >                        MARK_CALL(name, format, ## args); \
> >                } \
> >        } while(0)
> >  
> 
> Yep, that looks reasonable.  Though you could just directly test a 
> per-marker enable flag, rather than using "condition"...
> 

The goal of the local variable "condition" here is that it will be forced to be
a register (=r in asm output), so there is no memory load involved (immediate
value).

I am not sure I understand your suggestion correctly.. do you mean having
a per-marker flag that would be loaded and tested at every marker site ?


Mathieu

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

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Frank Ch. Eigler]

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

> [...]
> > Yep, that looks reasonable.  Though you could just directly test a 
> > per-marker enable flag, rather than using "condition"...
> [...]
> I am not sure I understand your suggestion correctly.. do you mean having
> a per-marker flag that would be loaded and tested at every marker site ?

I gather that one reason for working so hard with the inline assembly
is a race condition problem with the plain STAP_MARK style of marker
disconnection:

        if (pointer) (*pointer)(args ...);

Granted, but this problem could almost certainly be dealt with simpler
than that.  How about a compxchg or other atomic-fetch of the static
pointer with a local variable?  That should solve the worry of an
(*NULL) call.

If we then become concerned with a valid pointer become obsolete (the
probe handler function wanting to unload), we might be able to use
some RCU-type deferral mechanism and/or preempt controls to ensure
that this does not happen.

- FChE

Re: [PATCH] Linux Kernel Markers 0.13 for 2.6.17

[Mathieu Desnoyers]

* Frank Ch. Eigler (fche@redhat.com) wrote:
> Mathieu Desnoyers <compudj@krystal.dyndns.org> writes:
> 
> > [...]
> > > Yep, that looks reasonable.  Though you could just directly test a 
> > > per-marker enable flag, rather than using "condition"...
> > [...]
> > I am not sure I understand your suggestion correctly.. do you mean having
> > a per-marker flag that would be loaded and tested at every marker site ?
> 
> I gather that one reason for working so hard with the inline assembly
> is a race condition problem with the plain STAP_MARK style of marker
> disconnection:
> 
>         if (pointer) (*pointer)(args ...);
> 
> Granted, but this problem could almost certainly be dealt with simpler
> than that.  How about a compxchg or other atomic-fetch of the static
> pointer with a local variable?  That should solve the worry of an
> (*NULL) call.
> 

I don't really see how cmpxchg might be needed here.

Atomic fetch of a static variable is how I will do it in my next version for the
non optimized case :

volatile static var = 0;
if(var) {
  preempt disable
  call
  preempt_enable_no_resched
}

But, still, in the optimized case, the if(var) will depend on an immediate
value, therefore saving the memory read.


> If we then become concerned with a valid pointer become obsolete (the
> probe handler function wanting to unload), we might be able to use
> some RCU-type deferral mechanism and/or preempt controls to ensure
> that this does not happen.
> 

This is exactly why the preemption is disabled around the call. However, RCU
must always _see_ a coherent version of the structure in memory.

Calling an empty function, disabling preemption around the call and calling
synchronize_sched() before deleting the removed function looks very much like
a RCU-style protection (actually, that's what it is).

Mathieu


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