Skip to contentSkip to conversation

Instrumentation Support Enhancement (2.6.24-rc8-mm1)

PATCH In review Mathieu Desnoyers 9 participants 48 messages Jan 24, 2008 – Jan 25, 2008

Subsystems (20)
lore
Share & tools
lore
#0 lore raw

cover Instrumentation Support Enhancement (2.6.24-rc8-mm1)

Hi Andrew,

Here are the latest instrumentation support enhancements. These include :

* Markers support for proprierary modules
(this is an updated version of the one I sent earlier, nothing big, just a small fuzz checked and refreshed)
* Instrumentation menu removal
* Text Edit Lock
* Immediate Values
* Linux Kernel Markers use Immediate Values

This patchset is fully bissectable, so if, for instance, someone is not happy with the last functionnality (Linux Kernel Markers use Immediate Values), it can be left out and the rest is completely usable as is.

Which apply on top of 2.6.24-rc8-mm1 in this order :

markers-support-for-proprietary-modules.patch
#
#Instrumentation menu removal fix-arm-to-play-nicely-with-generic-instrumentation-menu.patch move-kconfig-instrumentation-to-arch.patch
#
#Text Edit Lock
kprobes-use-mutex-for-insn-pages.patch kprobes-dont-use-kprobes-mutex-in-arch-code.patch kprobes-declare-kprobes-mutex-static.patch declare-array.patch text-edit-lock-architecture-independent-code.patch text-edit-lock-alternative-i386-and-x86_64.patch text-edit-lock-kprobes-architecture-independent.patch text-edit-lock-kprobes-x86.patch text-edit-lock-i386-standardize-debug-rodata.patch text-edit-lock-x86_64-standardize-debug-rodata.patch
#
#Immediate Values immediate-values-architecture-independent-code.patch immediate-values-kconfig-embedded.patch immediate-values-x86-optimization.patch add-text-poke-to-powerpc.patch immediate-values-powerpc-optimization.patch immediate-values-documentation.patch
#
profiling-use-immediate-values.patch
#
#Linux Kernel Markers use Immediate Values immediate-values-move-kprobes-x86-restore-interrupt-to-kdebug-h.patch add-discard-section-to-x86.patch immediate-values-x86-optimization-nmi-mce-support.patch immediate-values-powerpc-optimization-nmi-mce-support.patch immediate-values-use-arch-nmi-mce-support.patch linux-kernel-markers-immediate-values.patch

Mathieu

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#1 lore raw

PATCH 1/26 Linux Kernel Markers Support for Proprierary Modules

There seems to be good arguments for markers to support proprierary modules. So I am throwing this one-liner in and let's see how people react. It only makes sure that a module that has been "forced" to be loaded won't have its markers used. It is important to leave this check to make sure the kernel does not crash by expecting the markers part of the struct module by mistake in the case there is an incorrect checksum.

Discussion so far :
http://lkml.org/lkml/2008/1/22/226

Jon Masters <[email protected]> writes:
I notice in module.c:

#ifdef CONFIG_MARKERS
      if (!mod->taints)
              marker_update_probe_range(mod->markers,
                      mod->markers + mod->num_markers, NULL, NULL);

#endif

Is this an attempt to not set a marker for proprietary modules? [...]

* Frank Ch. Eigler ([email protected]) wrote:
I can't seem to find any discussion about this aspect. If this is the intent, it seems misguided to me. There may instead be a relationship to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?

- FChE

On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
On my part, its mostly a matter of not crashing the kernel when someone tries to force modprobe of a proprietary module (where the checksums doesn't match) on a kernel that supports the markers. Not doing so causes the markers to try to find the marker-specific information in struct module which doesn't exist and OOPSes.

Christoph's point of view is rather more drastic than mine : it's not interesting for the kernel community to help proprietary modules writers, so it's a good idea not to give them marker support. (I CC'ed him so he can clarify his position).

* Frank Ch. Eigler ([email protected]) wrote:
[...]
Another way of looking at this though is that by allowing/encouraging proprietary module writers to include markers, we and their users get new diagnostic capabilities. It constitutes a little bit of opening up, which IMO we should reward rather than punish.

* Vladis Ketnieks ([email protected]) wrote:

Quoted 6 lines · Mathieu Desnoyers

This specific one is a kernel policy matter, and I personally don't have a strong opinion about it. I agree that you raise a good counter argument : it can be useful to proprietary modules users to be able to extract tracing information from those modules to argue with their vendors that their driver/hardware is broken (a tracer is _very_ useful in that kind of situation).

Amen, brother. Been there, done that, got the tshirt (not on Linux, but other operating systems).

Quoted 3 lines

However, it is also useful to proprieraty
module writers who can benefit from the merged kernel/modules traces. Do we want to give them this ability ?

The proprietary module writer has the *source* for the kernel and their module. There's no way you can prevent the proprietary module writers from using this feature as long as you allow other module writers to use it.

Quoted 2 lines

It would surely help writing
better proprieraty kernel modules.

The biggest complaint against proprietary modules is that they make it impossible for *us* to debug. And you want to argue *against* a feature that would allow them to develop better code that causes less crashes, and therefor less people *asking* for us to debug it?

Remember - when a user tries a Linux box with a proprietary module, and the experience sucks because the module sucks, they will walk away thinking "Linux sucks", not "That module sucks".

CC "Frank Ch. Eigler" <[email protected]>
CC Jon Masters <[email protected]>
CC Rusty Russell <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Linus Torvalds <[email protected]>
CC Andrew Morton <[email protected]>
---
 kernel/module.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/module.c +1 -1
Index: linux-2.6-lttng.mm/kernel/module.c
===================================================================
@@ -1989,7 +1989,7 @@ static struct module *load_module(void _
 	add_kallsyms(mod, sechdrs, symindex, strindex, secstrings);
 
 #ifdef CONFIG_MARKERS
-	if (!mod->taints)
+	if (!(mod->taints & TAINT_FORCED_MODULE))
 		marker_update_probe_range(mod->markers,
 			mod->markers + mod->num_markers);
 #endif
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
Acked-by on v1 1/26
#2 lore raw

Re: [patch 01/26] Linux Kernel Markers Support for Proprierary Modules

Quoted 6 lines · Mathieu Desnoyers

There seems to be good arguments for markers to support proprierary modules. So I am throwing this one-liner in and let's see how people react. It only makes sure that a module that has been "forced" to be loaded won't have its markers used. It is important to leave this check to make sure the kernel does not crash by expecting the markers part of the struct module by mistake in the case there is an incorrect checksum.

#3 lore raw

PATCH 2/26 Fix ARM to play nicely with generic Instrumentation menu

In reply to Mathieu Desnoyers

The conflicting commit for move-kconfiginstrumentation-to-arch-kconfig-and-init-kconfig.patch is the ARM fix from Linus :

commit 38ad9aebe70dc72df08851bbd1620d89329129ba

He just seemed to agree that my approach (just putting the missing ARM config options in arch/arm/Kconfig) works too. The main advantage it has is that it is smaller, does not need a cleanup in the future and does not break the following patches unnecessarily.

It's just been discussed here

http://lkml.org/lkml/2008/1/15/267

However, Linus might prefer to stay with his own patch and I would totally understand it that late in the release cycle. Therefore I submit this for the next release cycle.

After applying this patch, move-kconfiginstrumentation-to-arch-kconfig-and-init-kconfig.patch applies nicely in the -mm tree without any modification.

---
 arch/arm/Kconfig                 |   19 +++++++++++++-
 arch/arm/Kconfig.instrumentation |   52 ---------------------------------------
 2 files changed, 18 insertions(+), 53 deletions(-)
arch/arm/Kconfig +18 -1
Index: linux-2.6-lttng.mm/arch/arm/Kconfig
===================================================================
@@ -144,6 +144,23 @@ config FIQ
 config ARCH_MTD_XIP
 	bool
 
+if OPROFILE
+
+config OPROFILE_ARMV6
+	def_bool y
+	depends on CPU_V6 && !SMP
+	select OPROFILE_ARM11_CORE
+
+config OPROFILE_MPCORE
+	def_bool y
+	depends on CPU_V6 && SMP
+	select OPROFILE_ARM11_CORE
+
+config OPROFILE_ARM11_CORE
+	bool
+
+endif
+
 config VECTORS_BASE
 	hex
 	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
@@ -1132,7 +1149,7 @@ endmenu
 
 source "fs/Kconfig"
 
-source "arch/arm/Kconfig.instrumentation"
+source "kernel/Kconfig.instrumentation"
 
 source "arch/arm/Kconfig.debug"
 
arch/arm/Kconfig.instrumentation deleted +0 -52

52 lines not on this page · raw message

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#4 lore raw

Re: [patch 02/26] Fix ARM to play nicely with generic Instrumentation menu

Quoted 5 lines · Mathieu Desnoyers

The conflicting commit for move-kconfiginstrumentation-to-arch-kconfig-and-init-kconfig.patch is the ARM fix from Linus :

commit 38ad9aebe70dc72df08851bbd1620d89329129ba

Since we have changes which depend on the location of these options, please hold off until the ARM tree merges in the next window:

----------------------- arch/arm/Kconfig.instrumentation ----------------------- index 63b8c6d..453ad8e 100644
@@ -43,6 +43,16 @@ config OPROFILE_MPCORE
config OPROFILE_ARM11_CORE
bool

+config KPROBES
+ bool "Kprobes"
+ depends on KALLSYMS && MODULES && !UML && !XIP_KERNEL
+ help

+	  Kprobes allows you to trap at almost any kernel address and
+	  execute a callback function.  register_kprobe() establishes
+	  a probepoint and specifies the callback.  Kprobes is useful
+	  for kernel debugging, non-intrusive instrumentation and testing.
+	  If in doubt, say "N".

+
config MARKERS
bool "Activate markers"
help

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#5 lore raw

Re: [patch 02/26] Fix ARM to play nicely with generic Instrumentation menu

Quoted 10 lines · * Russell King ([email protected]) wrote:
Quoted 5 lines · Mathieu Desnoyers

The conflicting commit for move-kconfiginstrumentation-to-arch-kconfig-and-init-kconfig.patch is the ARM fix from Linus :

commit 38ad9aebe70dc72df08851bbd1620d89329129ba

Since we have changes which depend on the location of these options, please hold off until the ARM tree merges in the next window:

Can you add me to the CC list of this patch please ?

Quoted 25 lines

----------------------- arch/arm/Kconfig.instrumentation ----------------------- index 63b8c6d..453ad8e 100644

(quoted diff) +10 -0
@@ -43,6 +43,16 @@ config OPROFILE_MPCORE
 config OPROFILE_ARM11_CORE
 	bool
 
+config KPROBES
+	bool "Kprobes"
+	depends on KALLSYMS && MODULES && !UML && !XIP_KERNEL
+	help
+	  Kprobes allows you to trap at almost any kernel address and
+	  execute a callback function.  register_kprobe() establishes
+	  a probepoint and specifies the callback.  Kprobes is useful
+	  for kernel debugging, non-intrusive instrumentation and testing.
+	  If in doubt, say "N".
+
 config MARKERS
 	bool "Activate markers"
 	help
Signature · 4 lines
-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#6 lore raw

Re: [patch 02/26] Fix ARM to play nicely with generic Instrumentation menu

Quoted 6 lines · Mathieu Desnoyers
Quoted 5 lines · * Russell King ([email protected]) wrote:
Quoted 4 lines · Mathieu Desnoyers

The conflicting commit for move-kconfiginstrumentation-to-arch-kconfig-and-init-kconfig.patch is the ARM fix from Linus :

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

commit 38ad9aebe70dc72df08851bbd1620d89329129ba

Since we have changes which depend on the location of these options, please hold off until the ARM tree merges in the next window:

Can you add me to the CC list of this patch please ?

Not really because it's merged, and any discussion happened a few months back (though it was updated when Linus fixed your broken changes.)

You can read the patch set at 2007-12-17, via:

http://lists.arm.linux.org.uk/lurker/search/20380101.000000.00000000@ml:linux-arm-kernel,kprobes.en.html

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#7 lore raw

PATCH 3/26 Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig

In reply to Mathieu Desnoyers

Move the instrumentation Kconfig to

arch/Kconfig for architecture dependent options
- oprofile
- kprobes

and

init/Kconfig for architecture independent options
- profiling
- markers

Remove the "Instrumentation Support" menu. Everything moves to "General setup". Delete the kernel/Kconfig.instrumentation file.

CC Linus Torvalds <[email protected]>
CC Sam Ravnborg <[email protected]>
---
 arch/Kconfig                   |   28 ++++++++++++++++++++
 arch/alpha/Kconfig             |    2 -
 arch/arm/Kconfig               |    2 -
 arch/blackfin/Kconfig          |    2 -
 arch/cris/Kconfig              |    2 -
 arch/frv/Kconfig               |    2 -
 arch/h8300/Kconfig             |    2 -
 arch/ia64/Kconfig              |    2 -
 arch/m32r/Kconfig              |    2 -
 arch/m68k/Kconfig              |    2 -
 arch/m68knommu/Kconfig         |    2 -
 arch/mips/Kconfig              |    2 -
 arch/parisc/Kconfig            |    2 -
 arch/powerpc/Kconfig           |    2 -
 arch/ppc/Kconfig               |    2 -
 arch/s390/Kconfig              |    2 -
 arch/sh/Kconfig                |    2 -
 arch/sh64/Kconfig              |    2 -
 arch/sparc/Kconfig             |    2 -
 arch/sparc64/Kconfig           |    2 -
 arch/um/Kconfig                |    2 -
 arch/v850/Kconfig              |    2 -
 arch/x86/Kconfig               |    2 -
 arch/xtensa/Kconfig            |    2 -
 init/Kconfig                   |   12 ++++++++
 kernel/Kconfig.instrumentation |   55 -----------------------------------------
 26 files changed, 40 insertions(+), 101 deletions(-)
arch/Kconfig +28 -0
Index: linux-2.6-lttng.mm/arch/Kconfig
===================================================================
@@ -1,3 +1,31 @@
 #
 # General architecture dependent options
 #
+
+config OPROFILE
+	tristate "OProfile system profiling (EXPERIMENTAL)"
+	depends on PROFILING
+	depends on HAVE_OPROFILE
+	help
+	  OProfile is a profiling system capable of profiling the
+	  whole system, include the kernel, kernel modules, libraries,
+	  and applications.
+
+	  If unsure, say N.
+
+config HAVE_OPROFILE
+	def_bool n
+
+config KPROBES
+	bool "Kprobes"
+	depends on KALLSYMS && MODULES
+	depends on HAVE_KPROBES
+	help
+	  Kprobes allows you to trap at almost any kernel address and
+	  execute a callback function.  register_kprobe() establishes
+	  a probepoint and specifies the callback.  Kprobes is useful
+	  for kernel debugging, non-intrusive instrumentation and testing.
+	  If in doubt, say "N".
+
+config HAVE_KPROBES
+	def_bool n
init/Kconfig +12 -0
Index: linux-2.6-lttng.mm/init/Kconfig
===================================================================
@@ -702,6 +702,18 @@ config PROC_PAGE_MONITOR
 	  /proc/kpagecount, and /proc/kpageflags. Disabling these
           interfaces will reduce the size of the kernel by approximately 4kb.
 
+config PROFILING
+	bool "Profiling support (EXPERIMENTAL)"
+	help
+	  Say Y here to enable the extended profiling support mechanisms used
+	  by profilers such as OProfile.
+
+config MARKERS
+	bool "Activate markers"
+	help
+	  Place an empty function call at each marker site. Can be
+	  dynamically changed for a probe function.
+
 source "arch/Kconfig"
 
 endmenu		# General setup
arch/alpha/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/alpha/Kconfig
===================================================================
@@ -653,8 +653,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/alpha/Kconfig.debug"
 
 # DUMMY_CONSOLE may be defined in drivers/video/console/Kconfig
arch/arm/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/arm/Kconfig
===================================================================
@@ -1149,8 +1149,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/arm/Kconfig.debug"
 
 source "security/Kconfig"
arch/blackfin/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/blackfin/Kconfig
===================================================================
@@ -1003,8 +1003,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/blackfin/Kconfig.debug"
 
 source "security/Kconfig"
arch/cris/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/cris/Kconfig
===================================================================
@@ -214,8 +214,6 @@ source "drivers/pci/Kconfig"
 
 source "drivers/usb/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/cris/Kconfig.debug"
 
 source "security/Kconfig"
arch/frv/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/frv/Kconfig
===================================================================
@@ -384,8 +384,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/frv/Kconfig.debug"
 
 source "security/Kconfig"
arch/h8300/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/h8300/Kconfig
===================================================================
@@ -226,8 +226,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/h8300/Kconfig.debug"
 
 source "security/Kconfig"
arch/ia64/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/ia64/Kconfig
===================================================================
@@ -602,8 +602,6 @@ config IRQ_PER_CPU
 
 source "arch/ia64/hp/sim/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/ia64/Kconfig.debug"
 
 source "security/Kconfig"
arch/m32r/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/m32r/Kconfig
===================================================================
@@ -430,8 +430,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/m32r/Kconfig.debug"
 
 source "security/Kconfig"
arch/m68k/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/m68k/Kconfig
===================================================================
@@ -667,8 +667,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/m68k/Kconfig.debug"
 
 source "security/Kconfig"
arch/m68knommu/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/m68knommu/Kconfig
===================================================================
@@ -710,8 +710,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/m68knommu/Kconfig.debug"
 
 source "security/Kconfig"
arch/mips/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/mips/Kconfig
===================================================================
@@ -2096,8 +2096,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/mips/Kconfig.debug"
 
 source "security/Kconfig"
arch/parisc/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/parisc/Kconfig
===================================================================
@@ -276,8 +276,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/parisc/Kconfig.debug"
 
 source "security/Kconfig"
arch/powerpc/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/powerpc/Kconfig
===================================================================
@@ -713,8 +713,6 @@ source "arch/powerpc/sysdev/qe_lib/Kconf
 
 source "lib/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/powerpc/Kconfig.debug"
 
 source "security/Kconfig"
arch/ppc/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/ppc/Kconfig
===================================================================
@@ -1319,8 +1319,6 @@ endmenu
 
 source "lib/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/ppc/Kconfig.debug"
 
 source "security/Kconfig"
arch/s390/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/s390/Kconfig
===================================================================
@@ -528,8 +528,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/s390/Kconfig.debug"
 
 source "security/Kconfig"
arch/sh/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/sh/Kconfig
===================================================================
@@ -899,8 +899,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sh/Kconfig.debug"
 
 source "security/Kconfig"
arch/sh64/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/sh64/Kconfig
===================================================================
@@ -279,8 +279,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sh64/Kconfig.debug"
 
 source "security/Kconfig"
arch/sparc/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/sparc/Kconfig
===================================================================
@@ -324,8 +324,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sparc/Kconfig.debug"
 
 source "security/Kconfig"
arch/sparc64/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/sparc64/Kconfig
===================================================================
@@ -470,8 +470,6 @@ source "drivers/sbus/char/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sparc64/Kconfig.debug"
 
 source "security/Kconfig"
arch/um/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/um/Kconfig
===================================================================
@@ -271,6 +271,4 @@ config INPUT
 	bool
 	default n
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/um/Kconfig.debug"
arch/v850/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/v850/Kconfig
===================================================================
@@ -334,8 +334,6 @@ source "sound/Kconfig"
 
 source "drivers/usb/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/v850/Kconfig.debug"
 
 source "security/Kconfig"
arch/xtensa/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/xtensa/Kconfig
===================================================================
@@ -246,8 +246,6 @@ config EMBEDDED_RAMDISK_IMAGE
 	  provide one yourself.
 endmenu
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/xtensa/Kconfig.debug"
 
 source "security/Kconfig"
arch/x86/Kconfig +0 -2
Index: linux-2.6-lttng.mm/arch/x86/Kconfig
===================================================================
@@ -1609,8 +1609,6 @@ source "drivers/firmware/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/x86/Kconfig.debug"
 
 source "security/Kconfig"
kernel/Kconfig.instrumentation deleted +0 -55

55 lines not on this page · raw message

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#8 lore raw

Re: [patch 03/26] Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig

Quoted 6 lines · On Thu, 24 Jan 2008 15:27:09 -0500 Mathieu Desnoyers wrote:

Move the instrumentation Kconfig to

arch/Kconfig for architecture dependent options
- oprofile
- kprobes

Quoted 7 lines

and

init/Kconfig for architecture independent options
- profiling
- markers

Remove the "Instrumentation Support" menu. Everything moves to "General setup".

Was this done based on a suggestion from someone (Linus)? I prefer an Instrumentation menu, but whatever...

Quoted 25 lines

Delete the kernel/Kconfig.instrumentation file.

CC Linus Torvalds <[email protected]>
CC Sam Ravnborg <[email protected]>
---
 arch/Kconfig                   |   28 ++++++++++++++++++++
 arch/alpha/Kconfig             |    2 -
 arch/arm/Kconfig               |    2 -
 arch/blackfin/Kconfig          |    2 -
 arch/cris/Kconfig              |    2 -
 arch/frv/Kconfig               |    2 -
 arch/h8300/Kconfig             |    2 -
 arch/ia64/Kconfig              |    2 -
 arch/m32r/Kconfig              |    2 -
 arch/m68k/Kconfig              |    2 -
 arch/m68knommu/Kconfig         |    2 -
 arch/mips/Kconfig              |    2 -
 arch/parisc/Kconfig            |    2 -
 arch/powerpc/Kconfig           |    2 -
 arch/ppc/Kconfig               |    2 -
 arch/s390/Kconfig              |    2 -
 arch/sh/Kconfig                |    2 -
 arch/sh64/Kconfig              |    2 -
 arch/sparc/Kconfig             |    2 -
Quoted 8 lines
 arch/sparc64/Kconfig           |    2 -
 arch/um/Kconfig                |    2 -
 arch/v850/Kconfig              |    2 -
 arch/x86/Kconfig               |    2 -
 arch/xtensa/Kconfig            |    2 -
 init/Kconfig                   |   12 ++++++++
 kernel/Kconfig.instrumentation |   55 -----------------------------------------
 26 files changed, 40 insertions(+), 101 deletions(-)

---
~Randy

#9 lore raw

Re: [patch 03/26] Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig

Quoted 10 lines · * Randy Dunlap ([email protected]) wrote:
Quoted 8 lines · On Thu, 24 Jan 2008 15:27:09 -0500 Mathieu Desnoyers wrote:

Move the instrumentation Kconfig to

arch/Kconfig for architecture dependent options
- oprofile
- kprobes

and

Quoted 9 lines
Quoted 5 lines

init/Kconfig for architecture independent options
- profiling
- markers

Remove the "Instrumentation Support" menu. Everything moves to "General setup".

Was this done based on a suggestion from someone (Linus)? I prefer an Instrumentation menu, but whatever...

Yes, it was Linus'suggestion. I prefer the instrumentation menu too, but in order to be consistent, we would have to create an instrumentation/ directory in the kernel tree.

Mathieu

Quoted 37 lines
Quoted 26 lines

Delete the kernel/Kconfig.instrumentation file.

CC Linus Torvalds <[email protected]>
CC Sam Ravnborg <[email protected]>
---
 arch/Kconfig                   |   28 ++++++++++++++++++++
 arch/alpha/Kconfig             |    2 -
 arch/arm/Kconfig               |    2 -
 arch/blackfin/Kconfig          |    2 -
 arch/cris/Kconfig              |    2 -
 arch/frv/Kconfig               |    2 -
 arch/h8300/Kconfig             |    2 -
 arch/ia64/Kconfig              |    2 -
 arch/m32r/Kconfig              |    2 -
 arch/m68k/Kconfig              |    2 -
 arch/m68knommu/Kconfig         |    2 -
 arch/mips/Kconfig              |    2 -
 arch/parisc/Kconfig            |    2 -
 arch/powerpc/Kconfig           |    2 -
 arch/ppc/Kconfig               |    2 -
 arch/s390/Kconfig              |    2 -
 arch/sh/Kconfig                |    2 -
 arch/sh64/Kconfig              |    2 -
 arch/sparc/Kconfig             |    2 -
Quoted 8 lines
 arch/sparc64/Kconfig           |    2 -
 arch/um/Kconfig                |    2 -
 arch/v850/Kconfig              |    2 -
 arch/x86/Kconfig               |    2 -
 arch/xtensa/Kconfig            |    2 -
 init/Kconfig                   |   12 ++++++++
 kernel/Kconfig.instrumentation |   55 -----------------------------------------
 26 files changed, 40 insertions(+), 101 deletions(-)

---
~Randy

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#10 lore raw

Re: [patch 03/26] Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig

In reply to Mathieu Desnoyers
Quoted 6 lines · * Mathieu Desnoyers ([email protected]) wrote:

Move the instrumentation Kconfig to

arch/Kconfig for architecture dependent options
- oprofile
- kprobes

Quoted 9 lines

and

init/Kconfig for architecture independent options
- profiling
- markers

Remove the "Instrumentation Support" menu. Everything moves to "General setup". Delete the kernel/Kconfig.instrumentation file.

Please wait for updated version. Waiting for the next ARM merge adding kprobes to ARM, which will likely break this.

Quoted 459 lines
CC Linus Torvalds <[email protected]>
CC Sam Ravnborg <[email protected]>
---
 arch/Kconfig                   |   28 ++++++++++++++++++++
 arch/alpha/Kconfig             |    2 -
 arch/arm/Kconfig               |    2 -
 arch/blackfin/Kconfig          |    2 -
 arch/cris/Kconfig              |    2 -
 arch/frv/Kconfig               |    2 -
 arch/h8300/Kconfig             |    2 -
 arch/ia64/Kconfig              |    2 -
 arch/m32r/Kconfig              |    2 -
 arch/m68k/Kconfig              |    2 -
 arch/m68knommu/Kconfig         |    2 -
 arch/mips/Kconfig              |    2 -
 arch/parisc/Kconfig            |    2 -
 arch/powerpc/Kconfig           |    2 -
 arch/ppc/Kconfig               |    2 -
 arch/s390/Kconfig              |    2 -
 arch/sh/Kconfig                |    2 -
 arch/sh64/Kconfig              |    2 -
 arch/sparc/Kconfig             |    2 -
 arch/sparc64/Kconfig           |    2 -
 arch/um/Kconfig                |    2 -
 arch/v850/Kconfig              |    2 -
 arch/x86/Kconfig               |    2 -
 arch/xtensa/Kconfig            |    2 -
 init/Kconfig                   |   12 ++++++++
 kernel/Kconfig.instrumentation |   55 -----------------------------------------
 26 files changed, 40 insertions(+), 101 deletions(-)

Index: linux-2.6-lttng.mm/arch/Kconfig
===================================================================

arch/Kconfig +28 -0
@@ -1,3 +1,31 @@
 #
 # General architecture dependent options
 #
+
+config OPROFILE
+	tristate "OProfile system profiling (EXPERIMENTAL)"
+	depends on PROFILING
+	depends on HAVE_OPROFILE
+	help
+	  OProfile is a profiling system capable of profiling the
+	  whole system, include the kernel, kernel modules, libraries,
+	  and applications.
+
+	  If unsure, say N.
+
+config HAVE_OPROFILE
+	def_bool n
+
+config KPROBES
+	bool "Kprobes"
+	depends on KALLSYMS && MODULES
+	depends on HAVE_KPROBES
+	help
+	  Kprobes allows you to trap at almost any kernel address and
+	  execute a callback function.  register_kprobe() establishes
+	  a probepoint and specifies the callback.  Kprobes is useful
+	  for kernel debugging, non-intrusive instrumentation and testing.
+	  If in doubt, say "N".
+
+config HAVE_KPROBES
+	def_bool n

Index: linux-2.6-lttng.mm/init/Kconfig
===================================================================

init/Kconfig +12 -0
@@ -702,6 +702,18 @@ config PROC_PAGE_MONITOR
 	  /proc/kpagecount, and /proc/kpageflags. Disabling these
           interfaces will reduce the size of the kernel by approximately 4kb.
 
+config PROFILING
+	bool "Profiling support (EXPERIMENTAL)"
+	help
+	  Say Y here to enable the extended profiling support mechanisms used
+	  by profilers such as OProfile.
+
+config MARKERS
+	bool "Activate markers"
+	help
+	  Place an empty function call at each marker site. Can be
+	  dynamically changed for a probe function.
+
 source "arch/Kconfig"
 
 endmenu		# General setup

Index: linux-2.6-lttng.mm/arch/alpha/Kconfig
===================================================================

arch/alpha/Kconfig +0 -2
@@ -653,8 +653,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/alpha/Kconfig.debug"
 
 # DUMMY_CONSOLE may be defined in drivers/video/console/Kconfig

Index: linux-2.6-lttng.mm/arch/arm/Kconfig
===================================================================

arch/arm/Kconfig +0 -2
@@ -1149,8 +1149,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/arm/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/blackfin/Kconfig
===================================================================

arch/blackfin/Kconfig +0 -2
@@ -1003,8 +1003,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/blackfin/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/cris/Kconfig
===================================================================

arch/cris/Kconfig +0 -2
@@ -214,8 +214,6 @@ source "drivers/pci/Kconfig"
 
 source "drivers/usb/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/cris/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/frv/Kconfig
===================================================================

arch/frv/Kconfig +0 -2
@@ -384,8 +384,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/frv/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/h8300/Kconfig
===================================================================

arch/h8300/Kconfig +0 -2
@@ -226,8 +226,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/h8300/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/ia64/Kconfig
===================================================================

arch/ia64/Kconfig +0 -2
@@ -602,8 +602,6 @@ config IRQ_PER_CPU
 
 source "arch/ia64/hp/sim/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/ia64/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/m32r/Kconfig
===================================================================

arch/m32r/Kconfig +0 -2
@@ -430,8 +430,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/m32r/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/m68k/Kconfig
===================================================================

arch/m68k/Kconfig +0 -2
@@ -667,8 +667,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/m68k/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/m68knommu/Kconfig
===================================================================

arch/m68knommu/Kconfig +0 -2
@@ -710,8 +710,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/m68knommu/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/mips/Kconfig
===================================================================

arch/mips/Kconfig +0 -2
@@ -2096,8 +2096,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/mips/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/parisc/Kconfig
===================================================================

arch/parisc/Kconfig +0 -2
@@ -276,8 +276,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/parisc/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/powerpc/Kconfig
===================================================================

arch/powerpc/Kconfig +0 -2
@@ -713,8 +713,6 @@ source "arch/powerpc/sysdev/qe_lib/Kconf
 
 source "lib/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/powerpc/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/ppc/Kconfig
===================================================================

arch/ppc/Kconfig +0 -2
@@ -1319,8 +1319,6 @@ endmenu
 
 source "lib/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/ppc/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/s390/Kconfig
===================================================================

arch/s390/Kconfig +0 -2
@@ -528,8 +528,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/s390/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/sh/Kconfig
===================================================================

arch/sh/Kconfig +0 -2
@@ -899,8 +899,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sh/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/sh64/Kconfig
===================================================================

arch/sh64/Kconfig +0 -2
@@ -279,8 +279,6 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sh64/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/sparc/Kconfig
===================================================================

arch/sparc/Kconfig +0 -2
@@ -324,8 +324,6 @@ endmenu
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sparc/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/sparc64/Kconfig
===================================================================

arch/sparc64/Kconfig +0 -2
@@ -470,8 +470,6 @@ source "drivers/sbus/char/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/sparc64/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/um/Kconfig
===================================================================

arch/um/Kconfig +0 -2
@@ -271,6 +271,4 @@ config INPUT
 	bool
 	default n
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/um/Kconfig.debug"

Index: linux-2.6-lttng.mm/arch/v850/Kconfig
===================================================================

arch/v850/Kconfig +0 -2
@@ -334,8 +334,6 @@ source "sound/Kconfig"
 
 source "drivers/usb/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/v850/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/xtensa/Kconfig
===================================================================

arch/xtensa/Kconfig +0 -2
@@ -246,8 +246,6 @@ config EMBEDDED_RAMDISK_IMAGE
 	  provide one yourself.
 endmenu
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/xtensa/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/arch/x86/Kconfig
===================================================================

arch/x86/Kconfig +0 -2
@@ -1609,8 +1609,6 @@ source "drivers/firmware/Kconfig"
 
 source "fs/Kconfig"
 
-source "kernel/Kconfig.instrumentation"
-
 source "arch/x86/Kconfig.debug"
 
 source "security/Kconfig"

Index: linux-2.6-lttng.mm/kernel/Kconfig.instrumentation
===================================================================

kernel/Kconfig.instrumentation deleted +0 -55
@@ -1,55 +0,0 @@
-menuconfig INSTRUMENTATION
-	bool "Instrumentation Support"
-	default y
-	---help---
-	  Say Y here to get to see options related to performance measurement,
-	  system-wide debugging, and testing. This option alone does not add any
-	  kernel code.
-
-	  If you say N, all options in this submenu will be skipped and
-	  disabled. If you're trying to debug the kernel itself, go see the
-	  Kernel Hacking menu.
-
-if INSTRUMENTATION
-
-config PROFILING
-	bool "Profiling support (EXPERIMENTAL)"
-	help
-	  Say Y here to enable the extended profiling support mechanisms used
-	  by profilers such as OProfile.
-
-config OPROFILE
-	tristate "OProfile system profiling (EXPERIMENTAL)"
-	depends on PROFILING
-	depends on HAVE_OPROFILE
-	help
-	  OProfile is a profiling system capable of profiling the
-	  whole system, include the kernel, kernel modules, libraries,
-	  and applications.
-
-	  If unsure, say N.
-
-config HAVE_OPROFILE
-	def_bool n
-
-config KPROBES
-	bool "Kprobes"
-	depends on KALLSYMS && MODULES
-	depends on HAVE_KPROBES
-	help
-	  Kprobes allows you to trap at almost any kernel address and
-	  execute a callback function.  register_kprobe() establishes
-	  a probepoint and specifies the callback.  Kprobes is useful
-	  for kernel debugging, non-intrusive instrumentation and testing.
-	  If in doubt, say "N".
-
-config HAVE_KPROBES
-	def_bool n
-
-config MARKERS
-	bool "Activate markers"
-	help
-	  Place an empty function call at each marker site. Can be
-	  dynamically changed for a probe function.
-
-endif # INSTRUMENTATION
Signature · 9 lines
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#11 lore raw

Re: [patch 03/26] Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig

In reply to Mathieu Desnoyers
Quoted 2 lines · On Thu, 24 Jan 2008 15:27:09 -0500 Mathieu Desnoyers…

Remove the "Instrumentation Support" menu. Everything moves to "General setup". Delete the kernel/Kconfig.instrumentation file.

Hmm. In preparation for oprofile, I currently have the below in my avr32 tree (i.e. -mm). What's the best way to deal with that?

I intend to push this early once the merge window opens. Could you just remove the instrumentation menu from avr32 as well in your patch? It should still apply nicely to -mm.

Haavard

commit 5ad893c907a645def629bbdab3a1c114f619bf1c

Author: Haavard Skinnemoen <[email protected]>
Date:   Wed Oct 31 15:22:34 2007 +0100

[AVR32] Include instrumentation menu

Remove KPROBES option from Kconfig.debug and include kernel/Kconfig.instrumentation.

arch/avr32/Kconfig +2 -0
index b77abce..bc08c7f 100644
@@ -219,6 +219,8 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
+source "kernel/Kconfig.instrumentation"
+
 source "arch/avr32/Kconfig.debug"
 
 source "security/Kconfig"
arch/avr32/Kconfig.debug +0 -10
index 64ace00..2283933 100644
@@ -6,14 +6,4 @@ config TRACE_IRQFLAGS_SUPPORT
 
 source "lib/Kconfig.debug"
 
-config KPROBES
-	bool "Kprobes"
-	depends on DEBUG_KERNEL
-	help
-	  Kprobes allows you to trap at almost any kernel address and
-          execute a callback function.  register_kprobe() establishes
-          a probepoint and specifies the callback.  Kprobes is useful
-          for kernel debugging, non-intrusive instrumentation and testing.
-          If in doubt, say "N".
-
 endmenu
#12 lore raw

PATCH 4/26 Kprobes - use a mutex to protect the instruction pages list.

In reply to Mathieu Desnoyers

Protect the instruction pages list by a specific insn pages mutex, called in get_insn_slot() and free_insn_slot(). It makes sure that architectures that does not need to call arch_remove_kprobe() does not take an unneeded kprobes mutex.

---
 kernel/kprobes.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)
kernel/kprobes.c +21 -6
Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
@@ -95,6 +95,10 @@ enum kprobe_slot_state {
 	SLOT_USED = 2,
 };
 
+/*
+ * Protects the kprobe_insn_pages list. Can nest into kprobe_mutex.
+ */
+static DEFINE_MUTEX(kprobe_insn_mutex);
 static struct hlist_head kprobe_insn_pages;
 static int kprobe_garbage_slots;
 static int collect_garbage_slots(void);
@@ -131,7 +135,9 @@ kprobe_opcode_t __kprobes *get_insn_slot
 {
 	struct kprobe_insn_page *kip;
 	struct hlist_node *pos;
+	kprobe_opcode_t *ret;
 
+	mutex_lock(&kprobe_insn_mutex);
  retry:
 	hlist_for_each_entry(kip, pos, &kprobe_insn_pages, hlist) {
 		if (kip->nused < INSNS_PER_PAGE) {
@@ -140,7 +146,8 @@ kprobe_opcode_t __kprobes *get_insn_slot
 				if (kip->slot_used[i] == SLOT_CLEAN) {
 					kip->slot_used[i] = SLOT_USED;
 					kip->nused++;
-					return kip->insns + (i * MAX_INSN_SIZE);
+					ret = kip->insns + (i * MAX_INSN_SIZE);
+					goto end;
 				}
 			}
 			/* Surprise!  No unused slots.  Fix kip->nused. */
@@ -154,8 +161,10 @@ kprobe_opcode_t __kprobes *get_insn_slot
 	}
 	/* All out of space.  Need to allocate a new page. Use slot 0. */
 	kip = kmalloc(sizeof(struct kprobe_insn_page), GFP_KERNEL);
-	if (!kip)
-		return NULL;
+	if (!kip) {
+		ret = NULL;
+		goto end;
+	}
 
 	/*
 	 * Use module_alloc so this page is within +/- 2GB of where the
@@ -165,7 +174,8 @@ kprobe_opcode_t __kprobes *get_insn_slot
 	kip->insns = module_alloc(PAGE_SIZE);
 	if (!kip->insns) {
 		kfree(kip);
-		return NULL;
+		ret = NULL;
+		goto end;
 	}
 	INIT_HLIST_NODE(&kip->hlist);
 	hlist_add_head(&kip->hlist, &kprobe_insn_pages);
@@ -173,7 +183,10 @@ kprobe_opcode_t __kprobes *get_insn_slot
 	kip->slot_used[0] = SLOT_USED;
 	kip->nused = 1;
 	kip->ngarbage = 0;
-	return kip->insns;
+	ret = kip->insns;
+end:
+	mutex_unlock(&kprobe_insn_mutex);
+	return ret;
 }
 
 /* Return 1 if all garbages are collected, otherwise 0. */
@@ -207,7 +220,7 @@ static int __kprobes collect_garbage_slo
 	struct kprobe_insn_page *kip;
 	struct hlist_node *pos, *next;
 
-	/* Ensure no-one is preepmted on the garbages */
+	/* Ensure no-one is preempted on the garbages */
 	if (check_safety() != 0)
 		return -EAGAIN;
 
@@ -231,6 +244,7 @@ void __kprobes free_insn_slot(kprobe_opc
 	struct kprobe_insn_page *kip;
 	struct hlist_node *pos;
 
+	mutex_lock(&kprobe_insn_mutex);
 	hlist_for_each_entry(kip, pos, &kprobe_insn_pages, hlist) {
 		if (kip->insns <= slot &&
 		    slot < kip->insns + (INSNS_PER_PAGE * MAX_INSN_SIZE)) {
@@ -247,6 +261,7 @@ void __kprobes free_insn_slot(kprobe_opc
 
 	if (dirty && ++kprobe_garbage_slots > INSNS_PER_PAGE)
 		collect_garbage_slots();
+	mutex_unlock(&kprobe_insn_mutex);
 }
 #endif
 
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#13 lore raw

PATCH 5/26 Kprobes - do not use kprobes mutex in arch code

In reply to Mathieu Desnoyers

Remove the kprobes mutex from kprobes.h, since it does not belong there. Also remove all use of this mutex in the architecture specific code, replacing it by a proper mutex lock/unlock in the architecture agnostic code.

---
 arch/ia64/kernel/kprobes.c    |    2 --
 arch/powerpc/kernel/kprobes.c |    2 --
 arch/s390/kernel/kprobes.c    |    2 --
 arch/x86/kernel/kprobes.c     |    2 --
 include/linux/kprobes.h       |    2 --
 kernel/kprobes.c              |    2 ++
 6 files changed, 2 insertions(+), 10 deletions(-)
include/linux/kprobes.h +0 -2
Index: linux-2.6-lttng.mm/include/linux/kprobes.h
===================================================================
@@ -35,7 +35,6 @@
 #include <linux/percpu.h>
 #include <linux/spinlock.h>
 #include <linux/rcupdate.h>
-#include <linux/mutex.h>
 
 #ifdef CONFIG_KPROBES
 #include <asm/kprobes.h>
@@ -192,7 +191,6 @@ static inline int init_test_probes(void)
 #endif /* CONFIG_KPROBES_SANITY_TEST */
 
 extern spinlock_t kretprobe_lock;
-extern struct mutex kprobe_mutex;
 extern int arch_prepare_kprobe(struct kprobe *p);
 extern void arch_arm_kprobe(struct kprobe *p);
 extern void arch_disarm_kprobe(struct kprobe *p);
kernel/kprobes.c +2 -0
Index: linux-2.6-lttng.mm/kernel/kprobes.c
===================================================================
@@ -644,7 +644,9 @@ valid_p:
 			list_del_rcu(&p->list);
 			kfree(old_p);
 		}
+		mutex_lock(&kprobe_mutex);
 		arch_remove_kprobe(p);
+		mutex_unlock(&kprobe_mutex);
 	} else {
 		mutex_lock(&kprobe_mutex);
 		if (p->break_handler)
arch/ia64/kernel/kprobes.c +0 -2
Index: linux-2.6-lttng.mm/arch/ia64/kernel/kprobes.c
===================================================================
@@ -582,9 +582,7 @@ void __kprobes arch_disarm_kprobe(struct
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
-	mutex_lock(&kprobe_mutex);
 	free_insn_slot(p->ainsn.insn, 0);
-	mutex_unlock(&kprobe_mutex);
 }
 /*
  * We are resuming execution after a single step fault, so the pt_regs
arch/powerpc/kernel/kprobes.c +0 -2
Index: linux-2.6-lttng.mm/arch/powerpc/kernel/kprobes.c
===================================================================
@@ -88,9 +88,7 @@ void __kprobes arch_disarm_kprobe(struct
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
-	mutex_lock(&kprobe_mutex);
 	free_insn_slot(p->ainsn.insn, 0);
-	mutex_unlock(&kprobe_mutex);
 }
 
 static void __kprobes prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
arch/s390/kernel/kprobes.c +0 -2
Index: linux-2.6-lttng.mm/arch/s390/kernel/kprobes.c
===================================================================
@@ -220,9 +220,7 @@ void __kprobes arch_disarm_kprobe(struct
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
-	mutex_lock(&kprobe_mutex);
 	free_insn_slot(p->ainsn.insn, 0);
-	mutex_unlock(&kprobe_mutex);
 }
 
 static void __kprobes prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
arch/x86/kernel/kprobes.c +0 -2
Index: linux-2.6-lttng.mm/arch/x86/kernel/kprobes.c
===================================================================
@@ -376,9 +376,7 @@ void __kprobes arch_disarm_kprobe(struct
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
-	mutex_lock(&kprobe_mutex);
 	free_insn_slot(p->ainsn.insn, (p->ainsn.boostable == 1));
-	mutex_unlock(&kprobe_mutex);
 }
 
 static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#14 lore raw

PATCH 6/26 Kprobes - declare kprobe_mutex static

In reply to Mathieu Desnoyers

Since it will not be used by other kernel objects, it makes sense to declare it static.

---
 kernel/kprobes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/kprobes.c +1 -1
Index: linux-2.6-lttng/kernel/kprobes.c
===================================================================
@@ -68,7 +68,7 @@ static struct hlist_head kretprobe_inst_
 /* NOTE: change this value only with kprobe_mutex held */
 static bool kprobe_enabled;
 
-DEFINE_MUTEX(kprobe_mutex);		/* Protects kprobe_table */
+static DEFINE_MUTEX(kprobe_mutex);	/* Protects kprobe_table */
 DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
 static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
 
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#15 lore raw

PATCH 7/26 Add INIT_ARRAY() to kernel.h

In reply to Mathieu Desnoyers

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)
include/linux/kernel.h +2 -0
Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type [len]) { [0 ... (len)-1] = (val) })
+
 #endif
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#16 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

Quoted 2 lines · Mathieu Desnoyers

such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

text_poke(addr, (unsigned char[]){[0 ... len-1] = val});

Well ugliness is debatable, but at least:

Quoted 1 line

+#define INIT_ARRAY(type, val, len) ((type [len]) { [0 ... (len)-1] = (val) })

#define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)})

You can use type[].

#17 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h (updated)

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Changelog :
- [len] -> []
- Remove whitespaces.
- Fixed Jan's suggestion which was not checkpatch.pl-safe.

---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)
include/linux/kernel.h +2 -0
Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type[]) {[0 ... (len)-1] = (val)})
+
 #endif
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#18 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h (updated)

Quoted 10 lines · Mathieu Desnoyers

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Changelog :
- [len] -> []
- Remove whitespaces.
- Fixed Jan's suggestion which was not checkpatch.pl-safe.

diff -dpru /dev/null <(echo ' #define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)}) ') | scripts/checkpatch.pl -
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 2 lines checked

Where did checkpatch play unfair again on your side? :)

#19 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h (updated)

Quoted 13 lines · * Jan Engelhardt ([email protected]) wrote:
Quoted 10 lines · Mathieu Desnoyers

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Changelog :
- [len] -> []
- Remove whitespaces.
- Fixed Jan's suggestion which was not checkpatch.pl-safe.

Quoted 6 lines

diff -dpru /dev/null <(echo ' #define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)}) ') | scripts/checkpatch.pl -
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 2 lines checked

Where did checkpatch play unfair again on your side? :)

Hrm, I don't know why checkpatch does not notice the problem when invoked with your command, but creating a patch with your change results in :

compudj@dijkstra:~/git/morestable/linux-2.6-lttng.mm$ scripts/checkpatch.pl patches/test.patch 
ERROR: need a space before the open brace '{'
#27: FILE: include/linux/kernel.h:426:
+#define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)})

total: 1 errors, 0 warnings, 6 lines checked

Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

but I guess we are deep down in nitpicking land ;)

Cheers,

Mathieu

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#20 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

In reply to Mathieu Desnoyers
Quoted 2 lines · On Thu, 24 Jan 2008 15:27:13 -0500 Mathieu Desnoyers wrote:

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters

Hi Mathieu,

Maybe you have explained this previously, but please give a short explanation of "brackets that would pollute kernel code".

Thanks.

Quoted 13 lines

such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================

include/linux/kernel.h struct sysinfo { · line 428

Quoted 6 lines
include/linux/kernel.h +2 -0
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type [len]) { [0 ... (len)-1] = (val) })
+
 #endif

---
~Randy

#21 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

Quoted 5 lines · * Randy Dunlap ([email protected]) wrote:
Quoted 2 lines · On Thu, 24 Jan 2008 15:27:13 -0500 Mathieu Desnoyers wrote:

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h. It is used to declare arguments passed as function parameters

Quoted 7 lines

Hi Mathieu,

Maybe you have explained this previously, but please give a short explanation of "brackets that would pollute kernel code".

Thanks.

Try opening such code in vim, and the syntax highlighting gets all messed up :-/

The other option is to do the *right thing* and fix vim, of course, but this change won't be in distros for a while.

Mathieu

Quoted 22 lines
Quoted 13 lines

such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================

include/linux/kernel.h struct sysinfo { · line 428

Quoted 6 lines
include/linux/kernel.h +2 -0
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type [len]) { [0 ... (len)-1] = (val) })
+
 #endif

---
~Randy

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#22 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

Quoted 4 lines · Mathieu Desnoyers
Quoted 3 lines · * Randy Dunlap ([email protected]) wrote:

Maybe you have explained this previously, but please give a short explanation of "brackets that would pollute kernel code".

Quoted 8 lines
Quoted 2 lines

Thanks.

Try opening such code in vim, and the syntax highlighting gets all messed up :-/

The other option is to do the *right thing* and fix vim, of course, but this change won't be in distros for a while.

Is it about unbalanced brackets?

Anyway, if you add it because other kinds of initializers (which ones?) aren't displayed nicely in syntax highlighting editors, just say so in the changelog. Your log isn't informative at all.

Signature · 3 lines
Stefan Richter
-=====-==--- ---= ==---
http://arcgraph.de/sr/
#23 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h (update 2)

Array initializers in the body of a function causes unbalanced brackets which syntax highlighting editors such as vim have problems with. Fix this by creating the INIT_ARRAY() macro.

It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Changelog :
- [len] -> []
- Remove whitespaces.
- Fixed Jan's suggestion which was not checkpatch.pl-safe.
- More informative patch header.

CC Jan Engelhardt <[email protected]>
CC Stefan Richter <[email protected]>
---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)
include/linux/kernel.h +2 -0
Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type[]) {[0 ... (len)-1] = (val)})
+
 #endif
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#24 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h (update 2)

Quoted 8 lines · Mathieu Desnoyers

Array initializers in the body of a function causes unbalanced brackets which syntax highlighting editors such as vim have problems with. Fix this by creating the INIT_ARRAY() macro.

It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

+#define INIT_ARRAY(type, val, len) ((type[]) {[0 ... (len)-1] = (val)})

While we are at useless macro territory, let's name it MAKE_ARRAY because it's exactly what it does.

#25 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

In reply to Mathieu Desnoyers
Quoted 7 lines · Mathieu Desnoyers

Try opening such code in vim, and the syntax highlighting gets all messed up :-/

The other option is to do the *right thing* and fix vim, of course, but this change won't be in distros for a while.

Let's get this straight...

You're suggesting introducing a pointless, ugly macro into the kernel because of a vim bug?

*Plonk!*

-hpa

#26 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

Quoted 6 lines · * H. Peter Anvin ([email protected]) wrote:
Quoted 4 lines · Mathieu Desnoyers

Try opening such code in vim, and the syntax highlighting gets all messed up :-/
The other option is to do the *right thing* and fix vim, of course, but this change won't be in distros for a while.

Quoted 7 lines

Let's get this straight...

You're suggesting introducing a pointless, ugly macro into the kernel because of a vim bug?

*Plonk!*

heh, yup, I guess we should drop this patch. As you point out, it makes no sense.

Mathieu

Quoted 1 line

-hpa

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#27 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

In reply to Mathieu Desnoyers
Quoted 4 lines · Mathieu Desnoyers

* Randy Dunlap ([email protected]) wrote:

Try opening such code in vim, and the syntax highlighting gets all messed up :-/

Then we'd also have to fix dcache.h:

#define DCACHE_NFSFS_RENAMED 0x0002 /* this dentry has been "silly
* renamed" and has to be
* deleted on the last dput() */

by adding \ at the end of the comments, to unbreak cooledit and mc. (The problem is not the comment, but the seemingly stray " in the 2nd line)

#28 lore raw

Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

In reply to Mathieu Desnoyers
Quoted 2 lines · Mathieu Desnoyers

Add initialization of an array, which needs brackets that would pollute kernel code, to kernel.h

"brackets which would pollute kernel code"???

-hpa

#29 lore raw

PATCH 8/26 Text Edit Lock - Architecture Independent Code

In reply to Mathieu Desnoyers

This is an architecture independant synchronization around kernel text modifications through use of a global mutex.

A mutex has been chosen so that kprobes, the main user of this, can sleep during memory allocation between the memory read of the instructions it must replace and the memory write of the breakpoint.

Other user of this interface: immediate values.

Paravirt and alternatives are always done when SMP is inactive, so there is no need to use locks.

---
 include/linux/memory.h |    7 +++++++
 mm/memory.c            |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
include/linux/memory.h +7 -0
Index: linux-2.6-lttng.mm/include/linux/memory.h
===================================================================
@@ -92,4 +92,11 @@ extern int memory_notify(unsigned long v
 #define hotplug_memory_notifier(fn, pri) do { } while (0)
 #endif
 
+/*
+ * Take and release the kernel text modification lock, used for code patching.
+ * Users of this lock can sleep.
+ */
+extern void kernel_text_lock(void);
+extern void kernel_text_unlock(void);
+
 #endif /* _LINUX_MEMORY_H_ */
mm/memory.c +34 -0
Index: linux-2.6-lttng.mm/mm/memory.c
===================================================================
@@ -51,6 +51,8 @@
 #include <linux/init.h>
 #include <linux/writeback.h>
 #include <linux/memcontrol.h>
+#include <linux/kprobes.h>
+#include <linux/mutex.h>
 
 #include <asm/pgalloc.h>
 #include <asm/uaccess.h>
@@ -85,6 +87,12 @@ EXPORT_SYMBOL(high_memory);
 
 int randomize_va_space __read_mostly = 1;
 
+/*
+ * mutex protecting text section modification (dynamic code patching).
+ * some users need to sleep (allocating memory...) while they hold this lock.
+ */
+static DEFINE_MUTEX(text_mutex);
+
 static int __init disable_randmaps(char *s)
 {
 	randomize_va_space = 0;
@@ -2674,3 +2682,29 @@ int access_process_vm(struct task_struct
 
 	return buf - old_buf;
 }
+
+/**
+ * kernel_text_lock     -   Take the kernel text modification lock
+ *
+ * Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */
+void __kprobes kernel_text_lock(void)
+{
+	mutex_lock(&text_mutex);
+}
+EXPORT_SYMBOL_GPL(kernel_text_lock);
+
+/**
+ * kernel_text_unlock   -   Release the kernel text modification lock
+ *
+ * Insures mutual write exclusion of kernel and modules text live text
+ * modification. Should be used for code patching.
+ * Users of this lock can sleep.
+ */
+void __kprobes kernel_text_unlock(void)
+{
+	mutex_unlock(&text_mutex);
+}
+EXPORT_SYMBOL_GPL(kernel_text_unlock);
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#30 lore raw

PATCH 9/26 Text Edit Lock - Alternative code for x86

In reply to Mathieu Desnoyers

Fix a memcpy that should be a text_poke (in apply_alternatives).

Use kernel_wp_save/kernel_wp_restore in text_poke to support DEBUG_RODATA correctly and so the CPU HOTPLUG special case can be removed.

Add text_poke_early, for alternatives and paravirt boot-time and module load time patching.

Notes:
- A macro is used instead of an inline function to deal with circular header include otherwise necessary for read_cr0 and preempt_disable/enable.

Changelog:

- Fix text_set and text_poke alignment check (mixed up bitwise and and or)
- Remove text_set
- Use the new macro INIT_ARRAY() to stop polluting the C files with ({ }) brackets (which breaks some c parsers in editors).
- Export add_nops, so it can be used by others.
- Remove x86 test for "wp_works_ok", it will just be ignored by the architecture if not supported.
- Document text_poke_early.
- Remove clflush, since it breaks some VIA architectures and is not strictly necessary.
- Add kerneldoc to text_poke and text_poke_early.
- Remove arg cr0 from kernel_wp_save/restore. Change the macro name for kernel_wp_disable/enable.

CC Andi Kleen <[email protected]>
CC Thomas Gleixner <[email protected]>
CC Ingo Molnar <[email protected]>
CC H. Peter Anvin <[email protected]>
---
 arch/x86/kernel/alternative.c |   56 ++++++++++++++++++++++++++++++++++--------
 include/asm-x86/alternative.h |   36 ++++++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 11 deletions(-)
arch/x86/kernel/alternative.c +46 -10
Index: linux-2.6-lttng.mm/arch/x86/kernel/alternative.c
===================================================================
@@ -173,7 +173,7 @@ static const unsigned char*const * find_
 #endif /* CONFIG_X86_64 */
 
 /* Use this to add nops to a buffer, then text_poke the whole buffer. */
-static void add_nops(void *insns, unsigned int len)
+void add_nops(void *insns, unsigned int len)
 {
 	const unsigned char *const *noptable = find_nop_table();
 
@@ -186,6 +186,7 @@ static void add_nops(void *insns, unsign
 		len -= noplen;
 	}
 }
+EXPORT_SYMBOL_GPL(add_nops);
 
 extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
 extern u8 *__smp_locks[], *__smp_locks_end[];
@@ -219,7 +220,7 @@ void apply_alternatives(struct alt_instr
 		memcpy(insnbuf, a->replacement, a->replacementlen);
 		add_nops(insnbuf + a->replacementlen,
 			 a->instrlen - a->replacementlen);
-		text_poke(instr, insnbuf, a->instrlen);
+		text_poke_early(instr, insnbuf, a->instrlen);
 	}
 }
 
@@ -234,7 +235,8 @@ static void alternatives_smp_lock(u8 **s
 			continue;
 		if (*ptr > text_end)
 			continue;
-		text_poke(*ptr, ((unsigned char []){0xf0}), 1); /* add lock prefix */
+		/* add lock prefix */
+		text_poke(*ptr, INIT_ARRAY(unsigned char, 0xf0, 1), 1);
 	};
 }
 
@@ -397,7 +399,7 @@ void apply_paravirt(struct paravirt_patc
 
 		/* Pad the rest with nops */
 		add_nops(insnbuf + used, p->len - used);
-		text_poke(p->instr, insnbuf, p->len);
+		text_poke_early(p->instr, insnbuf, p->len);
 	}
 }
 extern struct paravirt_patch_site __start_parainstructions[],
@@ -458,18 +460,52 @@ void __init alternative_instructions(voi
 #endif
 }
 
-/*
- * Warning:
+/**
+ * text_poke_early - Update instructions on a live kernel at boot time
+ * @addr: address to modify
+ * @opcode: source of the copy
+ * @len: length to copy
+ *
  * When you use this code to patch more than one byte of an instruction
  * you need to make sure that other CPUs cannot execute this code in parallel.
- * Also no thread must be currently preempted in the middle of these instructions.
- * And on the local CPU you need to be protected again NMI or MCE handlers
- * seeing an inconsistent instruction while you patch.
+ * Also no thread must be currently preempted in the middle of these
+ * instructions.  And on the local CPU you need to be protected again NMI or MCE
+ * handlers seeing an inconsistent instruction while you patch.
+ * Warning: read_cr0 is modified by paravirt, this is why we have _early
+ * versions. They are not in the __init section because they can be used at
+ * module load time.
  */
-void __kprobes text_poke(void *addr, unsigned char *opcode, int len)
+void *text_poke_early(void *addr, const void *opcode, size_t len)
 {
 	memcpy(addr, opcode, len);
 	sync_core();
 	/* Could also do a CLFLUSH here to speed up CPU recovery; but
 	   that causes hangs on some VIA CPUs. */
+	return addr;
 }
+
+/**
+ * text_poke - Update instructions on a live kernel
+ * @addr: address to modify
+ * @opcode: source of the copy
+ * @len: length to copy
+ *
+ * Only atomic text poke/set should be allowed when not doing early patching.
+ * It means the size must be writable atomically and the address must be aligned
+ * in a way that permits an atomic write.
+ */
+void *__kprobes text_poke(void *addr, const void *opcode, size_t len)
+{
+	BUG_ON(len > sizeof(long));
+	BUG_ON((((long)addr + len - 1) & ~(sizeof(long) - 1))
+		- ((long)addr & ~(sizeof(long) - 1)));
+	kernel_wp_disable();
+	memcpy(addr, opcode, len);
+	kernel_wp_enable();
+	sync_core();
+	/* Could also do a CLFLUSH here to speed up CPU recovery; but
+	   that causes hangs on some VIA CPUs. */
+	return addr;
+}
+
+
include/asm-x86/alternative.h +35 -1
Index: linux-2.6-lttng.mm/include/asm-x86/alternative.h
===================================================================
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 #include <linux/stddef.h>
 #include <asm/asm.h>
+#include <asm/processor-flags.h>
 
 /*
  * Alternative inline assembly for SMP.
@@ -156,6 +157,39 @@ apply_paravirt(struct paravirt_patch_sit
 #define __parainstructions_end	NULL
 #endif
 
-extern void text_poke(void *addr, unsigned char *opcode, int len);
+extern void add_nops(void *insns, unsigned int len);
+
+/*
+ * Clear and restore the kernel write-protection flag on the local CPU.
+ * Allows the kernel to edit read-only pages.
+ * Side-effect: any interrupt handler running between save and restore will have
+ * the ability to write to read-only pages.
+ *
+ * Warning:
+ * Code patching in the UP case is safe if NMIs and MCE handlers are stopped and
+ * no thread can be preempted in the instructions being modified (no iret to an
+ * invalid instruction possible) or if the instructions are changed from a
+ * consistent state to another consistent state atomically.
+ * More care must be taken when modifying code in the SMP case because of
+ * Intel's errata.
+ * On the local CPU you need to be protected again NMI or MCE handlers seeing an
+ * inconsistent instruction while you patch.
+ * The _early version does not use read_cr0(), which can be paravirtualized.
+ */
+
+extern void *text_poke(void *addr, const void *opcode, size_t len);
+extern void *text_poke_early(void *addr, const void *opcode, size_t len);
+
+#define kernel_wp_disable()			\
+do {						\
+	preempt_disable();			\
+	write_cr0(read_cr0() & ~X86_CR0_WP);	\
+} while (0)
+
+#define kernel_wp_enable()			\
+do {						\
+	write_cr0(read_cr0() | X86_CR0_WP);	\
+	preempt_enable();			\
+} while (0)
 
 #endif /* _ASM_X86_ALTERNATIVE_H */
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#31 lore raw

PATCH 10/26 Text Edit Lock - kprobes architecture independent support

In reply to Mathieu Desnoyers

Use the mutual exclusion provided by the text edit lock in the kprobes code. It allows coherent manipulation of the kernel code by other subsystems.

Changelog:

Move the kernel_text_lock/unlock out of the for loops.

---
 kernel/kprobes.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
kernel/kprobes.c +13 -6
Index: linux-2.6-lttng.mm/kernel/kprobes.c
===================================================================
@@ -43,6 +43,7 @@
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
 #include <linux/kdebug.h>
+#include <linux/memory.h>
 
 #include <asm-generic/sections.h>
 #include <asm/cacheflush.h>
@@ -568,9 +569,10 @@ static int __kprobes __register_kprobe(s
 		goto out;
 	}
 
+	kernel_text_lock();
 	ret = arch_prepare_kprobe(p);
 	if (ret)
-		goto out;
+		goto out_unlock_text;
 
 	INIT_HLIST_NODE(&p->hlist);
 	hlist_add_head_rcu(&p->hlist,
@@ -578,7 +580,8 @@ static int __kprobes __register_kprobe(s
 
 	if (kprobe_enabled)
 		arch_arm_kprobe(p);
-
+out_unlock_text:
+	kernel_text_unlock();
 out:
 	mutex_unlock(&kprobe_mutex);
 
@@ -621,8 +624,11 @@ valid_p:
 		 * enabled - otherwise, the breakpoint would already have
 		 * been removed. We save on flushing icache.
 		 */
-		if (kprobe_enabled)
+		if (kprobe_enabled) {
+			kernel_text_lock();
 			arch_disarm_kprobe(p);
+			kernel_text_unlock();
+		}
 		hlist_del_rcu(&old_p->hlist);
 		cleanup_p = 1;
 	} else {
@@ -644,9 +650,7 @@ valid_p:
 			list_del_rcu(&p->list);
 			kfree(old_p);
 		}
-		mutex_lock(&kprobe_mutex);
 		arch_remove_kprobe(p);
-		mutex_unlock(&kprobe_mutex);
 	} else {
 		mutex_lock(&kprobe_mutex);
 		if (p->break_handler)
@@ -717,7 +721,6 @@ static int __kprobes pre_handler_kretpro
 		ri->rp = rp;
 		ri->task = current;
 		arch_prepare_kretprobe(ri, regs);
-
 		/* XXX(hch): why is there no hlist_move_head? */
 		hlist_del(&ri->uflist);
 		hlist_add_head(&ri->uflist, &ri->rp->used_instances);
@@ -940,11 +943,13 @@ static void __kprobes enable_all_kprobes
 	if (kprobe_enabled)
 		goto already_enabled;
 
+	kernel_text_lock();
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
 		hlist_for_each_entry_rcu(p, node, head, hlist)
 			arch_arm_kprobe(p);
 	}
+	kernel_text_unlock();
 
 	kprobe_enabled = true;
 	printk(KERN_INFO "Kprobes globally enabled\n");
@@ -969,6 +974,7 @@ static void __kprobes disable_all_kprobe
 
 	kprobe_enabled = false;
 	printk(KERN_INFO "Kprobes globally disabled\n");
+	kernel_text_lock();
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
 		hlist_for_each_entry_rcu(p, node, head, hlist) {
@@ -976,6 +982,7 @@ static void __kprobes disable_all_kprobe
 				arch_disarm_kprobe(p);
 		}
 	}
+	kernel_text_unlock();
 
 	mutex_unlock(&kprobe_mutex);
 	/* Allow all currently running kprobes to complete */
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#32 lore raw

PATCH 11/26 Text Edit Lock - kprobes x86

In reply to Mathieu Desnoyers

Make kprobes use INIT_ARRAY().

---
 arch/x86/kernel/kprobes.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/kprobes.c +3 -2
Index: linux-2.6-lttng.mm/arch/x86/kernel/kprobes.c
===================================================================
@@ -366,12 +366,13 @@ int __kprobes arch_prepare_kprobe(struct
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
-	text_poke(p->addr, ((unsigned char []){BREAKPOINT_INSTRUCTION}), 1);
+	text_poke(p->addr, INIT_ARRAY(unsigned char, BREAKPOINT_INSTRUCTION, 1),
+		1);
 }
 
 void __kprobes arch_disarm_kprobe(struct kprobe *p)
 {
-	text_poke(p->addr, &p->opcode, 1);
+	text_poke(p->addr, INIT_ARRAY(unsigned char, p->opcode, 1), 1);
 }
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#33 lore raw

PATCH 12/26 Text Edit Lock - x86_32 standardize debug rodata

In reply to Mathieu Desnoyers

Standardize DEBUG_RODATA, removing special cases for hotplug and kprobes.

CC Andi Kleen <[email protected]>
CC Thomas Gleixner <[email protected]>
CC Ingo Molnar <[email protected]>
CC H. Peter Anvin <[email protected]>
---
 arch/x86/mm/init_32.c |   20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)
arch/x86/mm/init_32.c +7 -13
Index: linux-2.6-lttng.mm/arch/x86/mm/init_32.c
===================================================================
@@ -807,28 +807,21 @@ static int noinline do_test_wp_bit(void)
 }
 
 #ifdef CONFIG_DEBUG_RODATA
-
 void mark_rodata_ro(void)
 {
 	unsigned long start = PFN_ALIGN(_text);
 	unsigned long size = PFN_ALIGN(_etext) - start;
 
-#ifndef CONFIG_KPROBES
-#ifdef CONFIG_HOTPLUG_CPU
-	/* It must still be possible to apply SMP alternatives. */
-	if (num_possible_cpus() <= 1)
-#endif
-	{
-		change_page_attr(virt_to_page(start),
-		                 size >> PAGE_SHIFT, PAGE_KERNEL_RX);
-		printk("Write protecting the kernel text: %luk\n", size >> 10);
-	}
-#endif
+	change_page_attr(virt_to_page(start),
+		size >> PAGE_SHIFT, PAGE_KERNEL_RX);
+	printk(KERN_INFO "Write protecting the kernel text: %luk\n",
+		size >> 10);
+
 	start += size;
 	size = (unsigned long)__end_rodata - start;
 	change_page_attr(virt_to_page(start),
 	                 size >> PAGE_SHIFT, PAGE_KERNEL_RO);
-	printk("Write protecting the kernel read-only data: %luk\n",
+	printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
 	       size >> 10);
 
 	/*
@@ -839,6 +832,7 @@ void mark_rodata_ro(void)
 	 */
 	global_flush_tlb();
 }
+
 #endif
 
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#34 lore raw

PATCH 13/26 Text Edit Lock - x86_64 standardize debug rodata

In reply to Mathieu Desnoyers

Standardize DEBUG_RODATA, removing special cases for hotplug and kprobes.

CC Andi Kleen <[email protected]>
CC Thomas Gleixner <[email protected]>
CC Ingo Molnar <[email protected]>
CC H. Peter Anvin <[email protected]>
---
 arch/x86/mm/init_64.c |   23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)
arch/x86/mm/init_64.c +5 -18
Index: linux-2.6-lttng.mm/arch/x86/mm/init_64.c
===================================================================
@@ -714,25 +714,11 @@ void free_initmem(void)
 
 void mark_rodata_ro(void)
 {
-	unsigned long start = (unsigned long)_stext, end;
+	unsigned long start = PFN_ALIGN(_stext);
+	unsigned long end = PFN_ALIGN(__end_rodata);
 
-#ifdef CONFIG_HOTPLUG_CPU
-	/* It must still be possible to apply SMP alternatives. */
-	if (num_possible_cpus() > 1)
-		start = (unsigned long)_etext;
-#endif
-
-#ifdef CONFIG_KPROBES
-	start = (unsigned long)__start_rodata;
-#endif
-	
-	end = (unsigned long)__end_rodata;
-	start = (start + PAGE_SIZE - 1) & PAGE_MASK;
-	end &= PAGE_MASK;
-	if (end <= start)
-		return;
-
-	change_page_attr_addr(start, (end - start) >> PAGE_SHIFT, PAGE_KERNEL_RO);
+	change_page_attr_addr(start, (end - start) >> PAGE_SHIFT,
+				PAGE_KERNEL_RO);
 
 	printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
 	       (end - start) >> 10);
@@ -745,6 +731,7 @@ void mark_rodata_ro(void)
 	 */
 	global_flush_tlb();
 }
+
 #endif
 
 #ifdef CONFIG_BLK_DEV_INITRD
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#35 lore raw

PATCH 14/26 Immediate Values - Architecture Independent Code

In reply to Mathieu Desnoyers

Immediate values are used as read mostly variables that are rarely updated. They use code patching to modify the values inscribed in the instruction stream. It provides a way to save precious cache lines that would otherwise have to be used by these variables.

There is a generic _imv_read() version, which uses standard global variables, and optimized per architecture imv_read() implementations, which use a load immediate to remove a data cache hit. When the immediate values functionnality is disabled in the kernel, it falls back to global variables.

It adds a new rodata section "__imv" to place the pointers to the enable value. Immediate values activation functions sits in kernel/immediate.c.

Immediate values refer to the memory address of a previously declared integer. This integer holds the information about the state of the immediate values associated, and must be accessed through the API found in linux/immediate.h.

At module load time, each immediate value is checked to see if it must be enabled. It would be the case if the variable they refer to is exported from another module and already enabled.

In the early stages of start_kernel(), the immediate values are updated to reflect the state of the variable they refer to.

* Why should this be merged *

It improves performances on heavy memory I/O workloads.

An interesting result shows the potential this infrastructure has by showing the slowdown a simple system call such as getppid() suffers when it is used under heavy user-space cache trashing:

Random walk L1 and L2 trashing surrounding a getppid() call:
(note: in this test, do_syscal_trace was taken at each system call, see Documentation/immediate.txt in these patches for details)
- No memory pressure : getppid() takes 1573 cycles
- With memory pressure : getppid() takes 15589 cycles

We therefore have a slowdown of 10 times just to get the kernel variables from memory. Another test on the same architecture (Intel P4) measured the memory latency to be 559 cycles. Therefore, each cache line removed from the hot path would improve the syscall time of 3.5% in these conditions.

Changelog:

- section __imv is already SHF_ALLOC
- Because of the wonders of ELF, section 0 has sh_addr and sh_size 0. So the if (immediateindex) is unnecessary here.
- Remove module_mutex usage: depend on functions implemented in module.c for that.
- Does not update tainted module's immediate values.
- remove imv_*_t types, add DECLARE_IMV() and DEFINE_IMV().
- imv_read(&var) becomes imv_read(var) because of this.
- Adding a new EXPORT_IMV_SYMBOL(_GPL).
- remove imv_if(). Should use if (unlikely(imv_read(var))) instead.
- Wait until we have gcc support before we add the imv_if macro, since its form may have to change.
- Dont't declare the __imv section in vmlinux.lds.h, just put the content in the rodata section.
- Simplify interface : remove imv_set_early, keep track of kernel boot status internally.
- Remove the ALIGN(8) before the __imv section. It is packed now.
- Uses an IPI busy-loop on each CPU with interrupts disabled as a simple, architecture agnostic, update mechanism.
- Use imv_* instead of immediate_*.

---
 include/asm-generic/vmlinux.lds.h |    3 
 include/linux/immediate.h         |   94 +++++++++++++++++++
 include/linux/module.h            |   16 +++
 init/main.c                       |    8 +
 kernel/Makefile                   |    1 
 kernel/immediate.c                |  187 ++++++++++++++++++++++++++++++++++++++
 kernel/module.c                   |   50 +++++++++-
 7 files changed, 358 insertions(+), 1 deletion(-)
include/linux/immediate.h new file +94 -0
Index: linux-2.6-lttng.mm/include/linux/immediate.h
===================================================================
@@ -0,0 +1,94 @@
+#ifndef _LINUX_IMMEDIATE_H
+#define _LINUX_IMMEDIATE_H
+
+/*
+ * Immediate values, can be updated at runtime and save cache lines.
+ *
+ * (C) Copyright 2007 Mathieu Desnoyers <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifdef CONFIG_IMMEDIATE
+
+struct __imv {
+	unsigned long var;	/* Pointer to the identifier variable of the
+				 * immediate value
+				 */
+	unsigned long imv;	/*
+				 * Pointer to the memory location of the
+				 * immediate value within the instruction.
+				 */
+	unsigned char size;	/* Type size. */
+} __attribute__ ((packed));
+
+#include <asm/immediate.h>
+
+/**
+ * imv_set - set immediate variable (with locking)
+ * @name: immediate value name
+ * @i: required value
+ *
+ * Sets the value of @name, taking the module_mutex if required by
+ * the architecture.
+ */
+#define imv_set(name, i)						\
+	do {								\
+		name##__imv = (i);					\
+		core_imv_update();					\
+		module_imv_update();					\
+	} while (0)
+
+/*
+ * Internal update functions.
+ */
+extern void core_imv_update(void);
+extern void imv_update_range(const struct __imv *begin,
+	const struct __imv *end);
+
+#else
+
+/*
+ * Generic immediate values: a simple, standard, memory load.
+ */
+
+/**
+ * imv_read - read immediate variable
+ * @name: immediate value name
+ *
+ * Reads the value of @name.
+ */
+#define imv_read(name)			_imv_read(name)
+
+/**
+ * imv_set - set immediate variable (with locking)
+ * @name: immediate value name
+ * @i: required value
+ *
+ * Sets the value of @name, taking the module_mutex if required by
+ * the architecture.
+ */
+#define imv_set(name, i)		(name##__imv = (i))
+
+static inline void core_imv_update(void) { }
+static inline void module_imv_update(void) { }
+
+#endif
+
+#define DECLARE_IMV(type, name) extern __typeof__(type) name##__imv
+#define DEFINE_IMV(type, name)  __typeof__(type) name##__imv
+
+#define EXPORT_IMV_SYMBOL(name) EXPORT_SYMBOL(name##__imv)
+#define EXPORT_IMV_SYMBOL_GPL(name) EXPORT_SYMBOL_GPL(name##__imv)
+
+/**
+ * _imv_read - Read immediate value with standard memory load.
+ * @name: immediate value name
+ *
+ * Force a data read of the immediate value instead of the immediate value
+ * based mechanism. Useful for __init and __exit section data read.
+ */
+#define _imv_read(name)		(name##__imv)
+
+#endif
include/linux/module.h +16 -0
Index: linux-2.6-lttng.mm/include/linux/module.h
===================================================================
@@ -15,6 +15,7 @@
 #include <linux/stringify.h>
 #include <linux/kobject.h>
 #include <linux/moduleparam.h>
+#include <linux/immediate.h>
 #include <linux/marker.h>
 #include <asm/local.h>
 
@@ -355,6 +356,10 @@ struct module
 	/* The command line arguments (may be mangled).  People like
 	   keeping pointers to this stuff */
 	char *args;
+#ifdef CONFIG_IMMEDIATE
+	const struct __imv *immediate;
+	unsigned int num_immediate;
+#endif
 #ifdef CONFIG_MARKERS
 	struct marker *markers;
 	unsigned int num_markers;
@@ -467,6 +472,9 @@ extern void print_modules(void);
 
 extern void module_update_markers(void);
 
+extern void _module_imv_update(void);
+extern void module_imv_update(void);
+
 #else /* !CONFIG_MODULES... */
 #define EXPORT_SYMBOL(sym)
 #define EXPORT_SYMBOL_GPL(sym)
@@ -572,6 +580,14 @@ static inline void module_update_markers
 {
 }
 
+static inline void _module_imv_update(void)
+{
+}
+
+static inline void module_imv_update(void)
+{
+}
+
 #endif /* CONFIG_MODULES */
 
 struct device_driver;
kernel/module.c +49 -1

87 lines not on this page · raw message

kernel/immediate.c new file +187 -0

187 lines not on this page · raw message

init/main.c +8 -0
Index: linux-2.6-lttng.mm/init/main.c
===================================================================
@@ -58,6 +58,7 @@
 #include <linux/kthread.h>
 #include <linux/sched.h>
 #include <linux/signal.h>
+#include <linux/immediate.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -101,6 +102,11 @@ static inline void mark_rodata_ro(void) 
 #ifdef CONFIG_TC
 extern void tc_init(void);
 #endif
+#ifdef CONFIG_IMMEDIATE
+extern void imv_init_complete(void);
+#else
+static inline void imv_init_complete(void) { }
+#endif
 
 enum system_states system_state;
 EXPORT_SYMBOL(system_state);
@@ -518,6 +524,7 @@ asmlinkage void __init start_kernel(void
 	unwind_init();
 	lockdep_init();
 	cgroup_init_early();
+	core_imv_update();
 
 	local_irq_disable();
 	early_boot_irqs_off();
@@ -640,6 +647,7 @@ asmlinkage void __init start_kernel(void
 	cpuset_init();
 	taskstats_init_early();
 	delayacct_init();
+	imv_init_complete();
 
 	check_bugs();
 
kernel/Makefile +1 -0
Index: linux-2.6-lttng.mm/kernel/Makefile
===================================================================
@@ -69,6 +69,7 @@ obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_SYSCTL) += utsname_sysctl.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
+obj-$(CONFIG_IMMEDIATE) += immediate.o
 obj-$(CONFIG_MARKERS) += marker.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
include/asm-generic/vmlinux.lds.h +3 -0
Index: linux-2.6-lttng.mm/include/asm-generic/vmlinux.lds.h
===================================================================
@@ -25,6 +25,9 @@
 		*(.rodata) *(.rodata.*)					\
 		*(__vermagic)		/* Kernel version magic */	\
 		*(__markers_strings)	/* Markers: strings */		\
+		VMLINUX_SYMBOL(__start___imv) = .;			\
+		*(__imv)		/* Immediate values: pointers */ \
+		VMLINUX_SYMBOL(__stop___imv) = .;			\
 	}								\
 									\
 	.rodata1          : AT(ADDR(.rodata1) - LOAD_OFFSET) {		\
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#36 lore raw

PATCH 15/26 Immediate Values - Kconfig menu in EMBEDDED

In reply to Mathieu Desnoyers

Immediate values provide a way to use dynamic code patching to update variables sitting within the instruction stream. It saves caches lines normally used by static read mostly variables. Enable it by default, but let users disable it through the EMBEDDED menu with the "Disable immediate values" submenu entry.

Note: Since I think that I really should let embedded systems developers using RO memory the option to disable the immediate values, I choose to leave this menu option there, in the EMBEDDED menu. Also, the "CONFIG_IMMEDIATE" makes sense because we want to compile out all the immediate code when we decide not to use optimized immediate values at all (it removes otherwise unused code).

Changelog:
- Change ARCH_SUPPORTS_IMMEDIATE for ARCH_HAS_IMMEDIATE

CC Rusty Russell <[email protected]>
CC Adrian Bunk <[email protected]>
CC Andi Kleen <[email protected]>
CC Alexey Dobriyan <[email protected]>
CC Christoph Hellwig <[email protected]>
---
 init/Kconfig |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
init/Kconfig +24 -0
Index: linux-2.6-lttng.mm/init/Kconfig
===================================================================
@@ -472,6 +472,20 @@ config CC_OPTIMIZE_FOR_SIZE
 config SYSCTL
 	bool
 
+config IMMEDIATE
+	default y if !DISABLE_IMMEDIATE
+	depends on HAVE_IMMEDIATE
+	bool
+	help
+	  Immediate values are used as read-mostly variables that are rarely
+	  updated. They use code patching to modify the values inscribed in the
+	  instruction stream. It provides a way to save precious cache lines
+	  that would otherwise have to be used by these variables. They can be
+	  disabled through the EMBEDDED menu.
+
+config HAVE_IMMEDIATE
+	def_bool n
+
 menuconfig EMBEDDED
 	bool "Configure standard kernel features (for small systems)"
 	help
@@ -716,6 +730,16 @@ config MARKERS
 
 source "arch/Kconfig"
 
+config DISABLE_IMMEDIATE
+	default y if EMBEDDED
+	bool "Disable immediate values" if EMBEDDED
+	depends on HAVE_IMMEDIATE
+	help
+	  Disable code patching based immediate values for embedded systems. It
+	  consumes slightly more memory and requires to modify the instruction
+	  stream each time a variable is updated. Should really be disabled for
+	  embedded systems with read-only text.
+
 endmenu		# General setup
 
 config SLABINFO
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#37 lore raw

PATCH 16/26 Immediate Values - x86 Optimization

In reply to Mathieu Desnoyers

x86 optimization of the immediate values which uses a movl with code patching to set/unset the value used to populate the register used as variable source.

Changelog:
- Use text_poke_early with cr0 WP save/restore to patch the bypass. We are doing non atomic writes to a code region only touched by us (nobody can execute it since we are protected by the imv_mutex).
- Put imv_set and _imv_set in the architecture independent header.
- Use $0 instead of %2 with (0) operand.
- Add x86_64 support, ready for i386+x86_64 -> x86 merge.
- Use asm-x86/asm.h.

Ok, so the most flexible solution that I see, that should fit for both x86 and x86_64 would be :
1 byte : "=q" : "a", "b", "c", or "d" register for the i386. For
x86-64 it is equivalent to "r" class (for 8-bit instructions that do not use upper halves).
2, 4, 8 bytes : "=r" : A register operand is allowed provided that it is in a
general register.

- "Redux" immediate values : no need to put a breakpoint, therefore, no need to know where the instruction starts. It's therefore OK to have a REX prefix.

- Bugfix : 8 bytes 64 bits immediate value was declared as "4 bytes" in the immediate structure.
- Change the immediate.c update code to support variable length opcodes.
- Vastly simplified, using a busy looping IPI with interrupts disabled. Does not protect against NMI nor MCE.
- Pack the __imv section. Use smallest types required for size (char).
- Use imv_* instead of immediate_*.

CC Andi Kleen <[email protected]>
CC "H. Peter Anvin" <[email protected]>
CC Chuck Ebbert <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Jeremy Fitzhardinge <[email protected]>
CC Thomas Gleixner <[email protected]>
CC Ingo Molnar <[email protected]>
CC Rusty Russell <[email protected]>
---
 arch/x86/Kconfig            |    1 
 include/asm-x86/immediate.h |   77 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
include/asm-x86/immediate.h new file +77 -0
Index: linux-2.6-lttng.mm/include/asm-x86/immediate.h
===================================================================
@@ -0,0 +1,77 @@
+#ifndef _ASM_X86_IMMEDIATE_H
+#define _ASM_X86_IMMEDIATE_H
+
+/*
+ * Immediate values. x86 architecture optimizations.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm/asm.h>
+
+/**
+ * imv_read - read immediate variable
+ * @name: immediate value name
+ *
+ * Reads the value of @name.
+ * Optimized version of the immediate.
+ * Do not use in __init and __exit functions. Use _imv_read() instead.
+ * If size is bigger than the architecture long size, fall back on a memory
+ * read.
+ *
+ * Make sure to populate the initial static 64 bits opcode with a value
+ * what will generate an instruction with 8 bytes immediate value (not the REX.W
+ * prefixed one that loads a sign extended 32 bits immediate value in a r64
+ * register).
+ */
+#define imv_read(name)							\
+	({								\
+		__typeof__(name##__imv) value;				\
+		BUILD_BUG_ON(sizeof(value) > 8);			\
+		switch (sizeof(value)) {				\
+		case 1:							\
+			asm(".section __imv,\"a\",@progbits\n\t"	\
+				_ASM_PTR "%c1, (3f)-%c2\n\t"		\
+				".byte %c2\n\t"				\
+				".previous\n\t"				\
+				"mov $0,%0\n\t"				\
+				"3:\n\t"				\
+				: "=q" (value)				\
+				: "i" (&name##__imv),			\
+				  "i" (sizeof(value)));			\
+			break;						\
+		case 2:							\
+		case 4:							\
+			asm(".section __imv,\"a\",@progbits\n\t"	\
+				_ASM_PTR "%c1, (3f)-%c2\n\t"		\
+				".byte %c2\n\t"				\
+				".previous\n\t"				\
+				"mov $0,%0\n\t"				\
+				"3:\n\t"				\
+				: "=r" (value)				\
+				: "i" (&name##__imv),			\
+				  "i" (sizeof(value)));			\
+			break;						\
+		case 8:							\
+			if (sizeof(long) < 8) {				\
+				value = name##__imv;			\
+				break;					\
+			}						\
+			asm(".section __imv,\"a\",@progbits\n\t"	\
+				_ASM_PTR "%c1, (3f)-%c2\n\t"		\
+				".byte %c2\n\t"				\
+				".previous\n\t"				\
+				"mov $0xFEFEFEFE01010101,%0\n\t" 	\
+				"3:\n\t"				\
+				: "=r" (value)				\
+				: "i" (&name##__imv),			\
+				  "i" (sizeof(value)));			\
+			break;						\
+		};							\
+		value;							\
+	})
+
+#endif /* _ASM_X86_IMMEDIATE_H */
arch/x86/Kconfig +1 -0
Index: linux-2.6-lttng.mm/arch/x86/Kconfig
===================================================================
@@ -20,6 +20,7 @@ config X86
 	def_bool y
 	select HAVE_OPROFILE
 	select HAVE_KPROBES
+	select HAVE_IMMEDIATE
 
 config GENERIC_LOCKBREAK
 	def_bool n
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#38 lore raw

PATCH 17/26 Add text_poke and sync_core to powerpc

In reply to Mathieu Desnoyers

- Needed on architectures where we must surround live instruction modification with "WP flag disable".
- Turns into a memcpy on powerpc since there is no WP flag activated for instruction pages (yet..).
- Add empty sync_core to powerpc so it can be used in architecture independent code.

CC Rusty Russell <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Paul Mackerras <[email protected]>
---
 include/asm-powerpc/cacheflush.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
include/asm-powerpc/cacheflush.h +3 -1
Index: linux-2.6-lttng/include/asm-powerpc/cacheflush.h
===================================================================
@@ -63,7 +63,9 @@ extern void flush_dcache_phys_range(unsi
 #define copy_from_user_page(vma, page, vaddr, dst, src, len) \
 	memcpy(dst, src, len)
 
-
+#define text_poke	memcpy
+#define text_poke_early	text_poke
+#define sync_core()
 
 #ifdef CONFIG_DEBUG_PAGEALLOC
 /* internal debugging function */
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#39 lore raw

PATCH 18/26 Immediate Values - Powerpc Optimization

In reply to Mathieu Desnoyers

PowerPC optimization of the immediate values which uses a li instruction, patched with an immediate value.

Changelog:
- Put imv_set and _imv_set in the architecture independent header.
- Pack the __imv section. Use smallest types required for size (char).
- Remove architecture specific update code : now handled by architecture agnostic code.
- Use imv_* instead of immediate_*.

CC Rusty Russell <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Paul Mackerras <[email protected]>
---
 arch/powerpc/Kconfig            |    1 
 include/asm-powerpc/immediate.h |   55 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
include/asm-powerpc/immediate.h new file +55 -0
Index: linux-2.6-lttng.mm/include/asm-powerpc/immediate.h
===================================================================
@@ -0,0 +1,55 @@
+#ifndef _ASM_POWERPC_IMMEDIATE_H
+#define _ASM_POWERPC_IMMEDIATE_H
+
+/*
+ * Immediate values. PowerPC architecture optimizations.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm/asm-compat.h>
+
+/**
+ * imv_read - read immediate variable
+ * @name: immediate value name
+ *
+ * Reads the value of @name.
+ * Optimized version of the immediate.
+ * Do not use in __init and __exit functions. Use _imv_read() instead.
+ */
+#define imv_read(name)							\
+	({								\
+		__typeof__(name##__imv) value;				\
+		BUILD_BUG_ON(sizeof(value) > 8);			\
+		switch (sizeof(value)) {				\
+		case 1:							\
+			asm(".section __imv,\"a\",@progbits\n\t"	\
+					PPC_LONG "%c1, ((1f)-1)\n\t"	\
+					".byte 1\n\t"			\
+					".previous\n\t"			\
+					"li %0,0\n\t"			\
+					"1:\n\t"			\
+				: "=r" (value)				\
+				: "i" (&name##__imv));			\
+			break;						\
+		case 2:							\
+			asm(".section __imv,\"a\",@progbits\n\t"	\
+					PPC_LONG "%c1, ((1f)-2)\n\t"	\
+					".byte 2\n\t"			\
+					".previous\n\t"			\
+					"li %0,0\n\t"			\
+					"1:\n\t"			\
+				: "=r" (value)				\
+				: "i" (&name##__imv));			\
+			break;						\
+		case 4:							\
+		case 8:	value = name##__imv;				\
+			break;						\
+		};							\
+		value;							\
+	})
+
+#endif /* _ASM_POWERPC_IMMEDIATE_H */
arch/powerpc/Kconfig +1 -0
Index: linux-2.6-lttng.mm/arch/powerpc/Kconfig
===================================================================
@@ -89,6 +89,7 @@ config PPC
 	default y
 	select HAVE_OPROFILE
 	select HAVE_KPROBES
+	select HAVE_IMMEDIATE
 
 config EARLY_PRINTK
 	bool
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#40 lore raw

PATCH 19/26 Immediate Values - Documentation

In reply to Mathieu Desnoyers

Changelog:
- Remove imv_set_early (removed from API).
- Use imv_* instead of immediate_*.

---
 Documentation/immediate.txt |  221 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 221 insertions(+)
Documentation/immediate.txt new file +221 -0
Index: linux-2.6-lttng/Documentation/immediate.txt
===================================================================
@@ -0,0 +1,221 @@
+		        Using the Immediate Values
+
+			    Mathieu Desnoyers
+
+
+This document introduces Immediate Values and their use.
+
+
+* Purpose of immediate values
+
+An immediate value is used to compile into the kernel variables that sit within
+the instruction stream. They are meant to be rarely updated but read often.
+Using immediate values for these variables will save cache lines.
+
+This infrastructure is specialized in supporting dynamic patching of the values
+in the instruction stream when multiple CPUs are running without disturbing the
+normal system behavior.
+
+Compiling code meant to be rarely enabled at runtime can be done using
+if (unlikely(imv_read(var))) as condition surrounding the code. The
+smallest data type required for the test (an 8 bits char) is preferred, since
+some architectures, such as powerpc, only allow up to 16 bits immediate values.
+
+
+* Usage
+
+In order to use the "immediate" macros, you should include linux/immediate.h.
+
+#include <linux/immediate.h>
+
+DEFINE_IMV(char, this_immediate);
+EXPORT_IMV_SYMBOL(this_immediate);
+
+
+And use, in the body of a function:
+
+Use imv_set(this_immediate) to set the immediate value.
+
+Use imv_read(this_immediate) to read the immediate value.
+
+The immediate mechanism supports inserting multiple instances of the same
+immediate. Immediate values can be put in inline functions, inlined static
+functions, and unrolled loops.
+
+If you have to read the immediate values from a function declared as __init or
+__exit, you should explicitly use _imv_read(), which will fall back on a
+global variable read. Failing to do so will leave a reference to the __init
+section after it is freed (it would generate a modpost warning).
+
+You can choose to set an initial static value to the immediate by using, for
+instance:
+
+DEFINE_IMV(long, myptr) = 10;
+
+
+* Optimization for a given architecture
+
+One can implement optimized immediate values for a given architecture by
+replacing asm-$ARCH/immediate.h.
+
+
+* Performance improvement
+
+
+  * Memory hit for a data-based branch
+
+Here are the results on a 3GHz Pentium 4:
+
+number of tests: 100
+number of branches per test: 100000
+memory hit cycles per iteration (mean): 636.611
+L1 cache hit cycles per iteration (mean): 89.6413
+instruction stream based test, cycles per iteration (mean): 85.3438
+Just getting the pointer from a modulo on a pseudo-random value, doing
+  nothing with it, cycles per iteration (mean): 77.5044
+
+So:
+Base case:                      77.50 cycles
+instruction stream based test:  +7.8394 cycles
+L1 cache hit based test:        +12.1369 cycles
+Memory load based test:         +559.1066 cycles
+
+So let's say we have a ping flood coming at
+(14014 packets transmitted, 14014 received, 0% packet loss, time 1826ms)
+7674 packets per second. If we put 2 markers for irq entry/exit, it
+brings us to 15348 markers sites executed per second.
+
+(15348 exec/s) * (559 cycles/exec) / (3G cycles/s) = 0.0029
+We therefore have a 0.29% slowdown just on this case.
+
+Compared to this, the instruction stream based test will cause a
+slowdown of:
+
+(15348 exec/s) * (7.84 cycles/exec) / (3G cycles/s) = 0.00004
+For a 0.004% slowdown.
+
+If we plan to use this for memory allocation, spinlock, and all sorts of
+very high event rate tracing, we can assume it will execute 10 to 100
+times more sites per second, which brings us to 0.4% slowdown with the
+instruction stream based test compared to 29% slowdown with the memory
+load based test on a system with high memory pressure.
+
+
+
+  * Markers impact under heavy memory load
+
+Running a kernel with my LTTng instrumentation set, in a test that
+generates memory pressure (from userspace) by trashing L1 and L2 caches
+between calls to getppid() (note: syscall_trace is active and calls
+a marker upon syscall entry and syscall exit; markers are disarmed).
+This test is done in user-space, so there are some delays due to IRQs
+coming and to the scheduler. (UP 2.6.22-rc6-mm1 kernel, task with -20
+nice level)
+
+My first set of results: Linear cache trashing, turned out not to be
+very interesting, because it seems like the linearity of the memset on a
+full array is somehow detected and it does not "really" trash the
+caches.
+
+Now the most interesting result: Random walk L1 and L2 trashing
+surrounding a getppid() call.
+
+- Markers compiled out (but syscall_trace execution forced)
+number of tests: 10000
+No memory pressure
+Reading timestamps takes 108.033 cycles
+getppid: 1681.4 cycles
+With memory pressure
+Reading timestamps takes 102.938 cycles
+getppid: 15691.6 cycles
+
+
+- With the immediate values based markers:
+number of tests: 10000
+No memory pressure
+Reading timestamps takes 108.006 cycles
+getppid: 1681.84 cycles
+With memory pressure
+Reading timestamps takes 100.291 cycles
+getppid: 11793 cycles
+
+
+- With global variables based markers:
+number of tests: 10000
+No memory pressure
+Reading timestamps takes 107.999 cycles
+getppid: 1669.06 cycles
+With memory pressure
+Reading timestamps takes 102.839 cycles
+getppid: 12535 cycles
+
+The result is quite interesting in that the kernel is slower without
+markers than with markers. I explain it by the fact that the data
+accessed is not laid out in the same manner in the cache lines when the
+markers are compiled in or out. It seems that it aligns the function's
+data better to compile-in the markers in this case.
+
+But since the interesting comparison is between the immediate values and
+global variables based markers, and because they share the same memory
+layout, except for the movl being replaced by a movz, we see that the
+global variable based markers (2 markers) adds 742 cycles to each system
+call (syscall entry and exit are traced and memory locations for both
+global variables lie on the same cache line).
+
+
+- Test redone with less iterations, but with error estimates
+
+10 runs of 100 iterations each: Tests done on a 3GHz P4. Here I run getppid with
+syscall trace inactive, comparing the case with memory pressure and without
+memory pressure. (sorry, my system is not setup to execute syscall_trace this
+time, but it will make the point anyway).
+
+No memory pressure
+Reading timestamps:     150.92 cycles,     std dev.    1.01 cycles
+getppid:               1462.09 cycles,     std dev.   18.87 cycles
+
+With memory pressure
+Reading timestamps:     578.22 cycles,     std dev.  269.51 cycles
+getppid:              17113.33 cycles,     std dev. 1655.92 cycles
+
+
+Now for memory read timing: (10 runs, branches per test: 100000)
+Memory read based branch:
+                       644.09 cycles,      std dev.   11.39 cycles
+L1 cache hit based branch:
+                        88.16 cycles,      std dev.    1.35 cycles
+
+
+So, now that we have the raw results, let's calculate:
+
+Memory read:
+644.09±11.39 - 88.16±1.35 = 555.93±11.46 cycles
+
+Getppid without memory pressure:
+1462.09±18.87 - 150.92±1.01 = 1311.17±18.90 cycles
+
+Getppid with memory pressure:
+17113.33±1655.92 - 578.22±269.51 = 16535.11±1677.71 cycles
+
+Therefore, if we add 2 markers not based on immediate values to the getppid
+code, which would add 2 memory reads, we would add
+2 * 555.93±12.74 = 1111.86±25.48 cycles
+
+Therefore,
+
+1111.86±25.48 / 16535.11±1677.71 = 0.0672
+ relative error: sqrt(((25.48/1111.86)^2)+((1677.71/16535.11)^2))
+                     = 0.1040
+ absolute error: 0.1040 * 0.0672 = 0.0070
+
+Therefore: 0.0672±0.0070 * 100% = 6.72±0.70 %
+
+We can therefore affirm that adding 2 markers to getppid, on a system with high
+memory pressure, would have a performance hit of at least 6.0% on the system
+call time, all within the uncertainty limits of these tests. The same applies to
+other kernel code paths. The smaller those code paths are, the highest the
+impact ratio will be.
+
+Therefore, not only is it interesting to use the immediate values to dynamically
+activate dormant code such as the markers, but I think it should also be
+considered as a replacement for many of the "read-mostly" static variables.
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#41 lore raw

PATCH 20/26 Scheduler Profiling - Use Immediate Values

In reply to Mathieu Desnoyers

Use immediate values with lower d-cache hit in optimized version as a condition for scheduler profiling call.

Changelog :
- Use imv_* instead of immediate_*.

---
 arch/x86/kvm/x86.c      |    2 +-
 include/linux/profile.h |    5 +++--
 kernel/profile.c        |   22 +++++++++++-----------
 kernel/sched_fair.c     |    6 +-----
 4 files changed, 16 insertions(+), 19 deletions(-)
kernel/profile.c +11 -11
Index: linux-2.6-lttng.mm/kernel/profile.c
===================================================================
@@ -41,8 +41,8 @@ static int (*timer_hook)(struct pt_regs 
 static atomic_t *prof_buffer;
 static unsigned long prof_len, prof_shift;
 
-int prof_on __read_mostly;
-EXPORT_SYMBOL_GPL(prof_on);
+DEFINE_IMV(char, prof_on) __read_mostly;
+EXPORT_IMV_SYMBOL_GPL(prof_on);
 
 static cpumask_t prof_cpu_mask = CPU_MASK_ALL;
 #ifdef CONFIG_SMP
@@ -60,7 +60,7 @@ static int __init profile_setup(char *st
 
 	if (!strncmp(str, sleepstr, strlen(sleepstr))) {
 #ifdef CONFIG_SCHEDSTATS
-		prof_on = SLEEP_PROFILING;
+		imv_set(prof_on, SLEEP_PROFILING);
 		if (str[strlen(sleepstr)] == ',')
 			str += strlen(sleepstr) + 1;
 		if (get_option(&str, &par))
@@ -73,7 +73,7 @@ static int __init profile_setup(char *st
 			"kernel sleep profiling requires CONFIG_SCHEDSTATS\n");
 #endif /* CONFIG_SCHEDSTATS */
 	} else if (!strncmp(str, schedstr, strlen(schedstr))) {
-		prof_on = SCHED_PROFILING;
+		imv_set(prof_on, SCHED_PROFILING);
 		if (str[strlen(schedstr)] == ',')
 			str += strlen(schedstr) + 1;
 		if (get_option(&str, &par))
@@ -82,7 +82,7 @@ static int __init profile_setup(char *st
 			"kernel schedule profiling enabled (shift: %ld)\n",
 			prof_shift);
 	} else if (!strncmp(str, kvmstr, strlen(kvmstr))) {
-		prof_on = KVM_PROFILING;
+		imv_set(prof_on, KVM_PROFILING);
 		if (str[strlen(kvmstr)] == ',')
 			str += strlen(kvmstr) + 1;
 		if (get_option(&str, &par))
@@ -92,7 +92,7 @@ static int __init profile_setup(char *st
 			prof_shift);
 	} else if (get_option(&str, &par)) {
 		prof_shift = par;
-		prof_on = CPU_PROFILING;
+		imv_set(prof_on, CPU_PROFILING);
 		printk(KERN_INFO "kernel profiling enabled (shift: %ld)\n",
 			prof_shift);
 	}
@@ -103,7 +103,7 @@ __setup("profile=", profile_setup);
 
 void __init profile_init(void)
 {
-	if (!prof_on)
+	if (!_imv_read(prof_on))
 		return;
 
 	/* only text is profiled */
@@ -290,7 +290,7 @@ void profile_hits(int type, void *__pc, 
 	int i, j, cpu;
 	struct profile_hit *hits;
 
-	if (prof_on != type || !prof_buffer)
+	if (!prof_buffer)
 		return;
 	pc = min((pc - (unsigned long)_stext) >> prof_shift, prof_len - 1);
 	i = primary = (pc & (NR_PROFILE_GRP - 1)) << PROFILE_GRPSHIFT;
@@ -400,7 +400,7 @@ void profile_hits(int type, void *__pc, 
 {
 	unsigned long pc;
 
-	if (prof_on != type || !prof_buffer)
+	if (!prof_buffer)
 		return;
 	pc = ((unsigned long)__pc - (unsigned long)_stext) >> prof_shift;
 	atomic_add(nr_hits, &prof_buffer[min(pc, prof_len - 1)]);
@@ -557,7 +557,7 @@ static int __init create_hash_tables(voi
 	}
 	return 0;
 out_cleanup:
-	prof_on = 0;
+	imv_set(prof_on, 0);
 	smp_mb();
 	on_each_cpu(profile_nop, NULL, 0, 1);
 	for_each_online_cpu(cpu) {
@@ -584,7 +584,7 @@ static int __init create_proc_profile(vo
 {
 	struct proc_dir_entry *entry;
 
-	if (!prof_on)
+	if (!_imv_read(prof_on))
 		return 0;
 	if (create_hash_tables())
 		return -1;
include/linux/profile.h +3 -2
Index: linux-2.6-lttng.mm/include/linux/profile.h
===================================================================
@@ -7,10 +7,11 @@
 #include <linux/init.h>
 #include <linux/cpumask.h>
 #include <linux/cache.h>
+#include <linux/immediate.h>
 
 #include <asm/errno.h>
 
-extern int prof_on __read_mostly;
+DECLARE_IMV(char, prof_on) __read_mostly;
 
 #define CPU_PROFILING	1
 #define SCHED_PROFILING	2
@@ -38,7 +39,7 @@ static inline void profile_hit(int type,
 	/*
 	 * Speedup for the common (no profiling enabled) case:
 	 */
-	if (unlikely(prof_on == type))
+	if (unlikely(imv_read(prof_on) == type))
 		profile_hits(type, ip, 1);
 }
 
kernel/sched_fair.c +1 -5
Index: linux-2.6-lttng.mm/kernel/sched_fair.c
===================================================================
@@ -461,12 +461,8 @@ static void enqueue_sleeper(struct cfs_r
 		 * get a milliseconds-range estimation of the amount of
 		 * time that the task spent sleeping:
 		 */
-		if (unlikely(prof_on == SLEEP_PROFILING)) {
-			struct task_struct *tsk = task_of(se);
-
-			profile_hits(SLEEP_PROFILING, (void *)get_wchan(tsk),
+		profile_hits(SLEEP_PROFILING, (void *)get_wchan(task_of(se)),
 				     delta >> 20);
-		}
 	}
 #endif
 }
arch/x86/kvm/x86.c +1 -1
Index: linux-2.6-lttng.mm/arch/x86/kvm/x86.c
===================================================================
@@ -2592,7 +2592,7 @@ again:
 	/*
 	 * Profile KVM exit RIPs:
 	 */
-	if (unlikely(prof_on == KVM_PROFILING)) {
+	if (unlikely(imv_read(prof_on) == KVM_PROFILING)) {
 		kvm_x86_ops->cache_regs(vcpu);
 		profile_hit(KVM_PROFILING, (void *)vcpu->arch.rip);
 	}
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#42 lore raw

PATCH 21/26 Immediate Values - Move Kprobes x86 restore_interrupt to kdebug.h

In reply to Mathieu Desnoyers

Since the breakpoint handler is useful both to kprobes and immediate values, it makes sense to make the required restore_interrupt() available through asm-i386/kdebug.h.

---
 include/asm-x86/kdebug.h  |   13 +++++++++++++
 include/asm-x86/kprobes.h |    9 ---------
 2 files changed, 13 insertions(+), 9 deletions(-)
include/asm-x86/kdebug.h +13 -0
Index: linux-2.6-lttng.mm/include/asm-x86/kdebug.h
===================================================================
@@ -3,6 +3,9 @@
 
 #include <linux/notifier.h>
 
+#include <linux/ptrace.h>
+#include <asm/system.h>
+
 struct pt_regs;
 
 /* Grossly misnamed. */
@@ -36,4 +39,14 @@ extern void dump_pagetable(unsigned long
 extern unsigned long oops_begin(void);
 extern void oops_end(unsigned long, struct pt_regs *, int signr);
 
+/*
+ * trap3/1 are intr gates for kprobes.  So, restore the status of IF,
+ * if necessary, before executing the original int3/1 (trap) handler.
+ */
+static inline void restore_interrupts(struct pt_regs *regs)
+{
+	if (regs->flags & X86_EFLAGS_IF)
+		local_irq_enable();
+}
+
 #endif
include/asm-x86/kprobes.h +0 -9
Index: linux-2.6-lttng.mm/include/asm-x86/kprobes.h
===================================================================
@@ -83,15 +83,6 @@ struct kprobe_ctlblk {
 	struct prev_kprobe prev_kprobe;
 };
 
-/* trap3/1 are intr gates for kprobes.  So, restore the status of IF,
- * if necessary, before executing the original int3/1 (trap) handler.
- */
-static inline void restore_interrupts(struct pt_regs *regs)
-{
-	if (regs->flags & X86_EFLAGS_IF)
-		local_irq_enable();
-}
-
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#43 lore raw

PATCH 22/26 Add __discard section to x86

In reply to Mathieu Desnoyers

Add a __discard sectionto the linker script. Code produced in this section will not be put in the vmlinux file. This is useful when we have to calculate the size of an instruction before actually declaring it (for alignment purposes for instance). This is used by the immediate values.

CC Andi Kleen <[email protected]>
CC Chuck Ebbert <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Jeremy Fitzhardinge <[email protected]>
CC Thomas Gleixner <[email protected]>
CC Ingo Molnar <[email protected]>
---
 arch/x86/kernel/vmlinux_32.lds.S |    1 +
 arch/x86/kernel/vmlinux_64.lds.S |    1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/vmlinux_32.lds.S +1 -0
Index: linux-2.6-lttng.mm/arch/x86/kernel/vmlinux_32.lds.S
===================================================================
@@ -201,6 +201,7 @@ SECTIONS
   /* Sections to be discarded */
   /DISCARD/ : {
 	*(.exitcall.exit)
+	*(__discard)
 	}
 
   STABS_DEBUG
arch/x86/kernel/vmlinux_64.lds.S +1 -0
Index: linux-2.6-lttng.mm/arch/x86/kernel/vmlinux_64.lds.S
===================================================================
@@ -226,6 +226,7 @@ SECTIONS
   /DISCARD/ : {
 	*(.exitcall.exit)
 	*(.eh_frame)
+	*(__discard)
 	}
 
   STABS_DEBUG
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#44 lore raw

PATCH 23/26 Immediate Values - x86 Optimization NMI and MCE support

In reply to Mathieu Desnoyers

x86 optimization of the immediate values which uses a movl with code patching to set/unset the value used to populate the register used as variable source. It uses a breakpoint to bypass the instruction being changed, which lessens the interrupt latency of the operation and protects against NMIs and MCE.

- More reentrant immediate value : uses a breakpoint. Needs to know the instruction's first byte. This is why we keep the "instruction size" variable, so we can support the REX prefixed instructions too.

Changelog:
- Use text_poke_early with cr0 WP save/restore to patch the bypass. We are doing non atomic writes to a code region only touched by us (nobody can execute it since we are protected by the imv_mutex).
- Add x86_64 support, ready for i386+x86_64 -> x86 merge.
- Use asm-x86/asm.h.
- Change the immediate.c update code to support variable length opcodes.
- Use imv_* instead of immediate_*.
- Use kernel_wp_disable/enable instead of save/restore.

CC Andi Kleen <[email protected]>
CC "H. Peter Anvin" <[email protected]>
CC Chuck Ebbert <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Jeremy Fitzhardinge <[email protected]>
CC Thomas Gleixner <[email protected]>
CC Ingo Molnar <[email protected]>
---
 arch/x86/kernel/Makefile    |    1 
 arch/x86/kernel/immediate.c |  281 ++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/traps_32.c  |   10 -
 include/asm-x86/immediate.h |   42 +++++-
 4 files changed, 325 insertions(+), 9 deletions(-)
include/asm-x86/immediate.h +37 -5
Index: linux-2.6-lttng.mm/include/asm-x86/immediate.h
===================================================================
@@ -12,6 +12,18 @@
 
 #include <asm/asm.h>
 
+struct __imv {
+	unsigned long var;	/* Pointer to the identifier variable of the
+				 * immediate value
+				 */
+	unsigned long imv;	/*
+				 * Pointer to the memory location of the
+				 * immediate value within the instruction.
+				 */
+	unsigned char size;	/* Type size. */
+	unsigned char insn_size;/* Instruction size. */
+} __attribute__ ((packed));
+
 /**
  * imv_read - read immediate variable
  * @name: immediate value name
@@ -26,6 +38,11 @@
  * what will generate an instruction with 8 bytes immediate value (not the REX.W
  * prefixed one that loads a sign extended 32 bits immediate value in a r64
  * register).
+ *
+ * Create the instruction in a discarded section to calculate its size. This is
+ * how we can align the beginning of the instruction on an address that will
+ * permit atomic modification of the immediate value without knowing the size of
+ * the opcode used by the compiler. The operand size is known in advance.
  */
 #define imv_read(name)							\
 	({								\
@@ -35,8 +52,9 @@
 		case 1:							\
 			asm(".section __imv,\"a\",@progbits\n\t"	\
 				_ASM_PTR "%c1, (3f)-%c2\n\t"		\
-				".byte %c2\n\t"				\
+				".byte %c2, (3f-2f)\n\t"		\
 				".previous\n\t"				\
+				"2:\n\t"				\
 				"mov $0,%0\n\t"				\
 				"3:\n\t"				\
 				: "=q" (value)				\
@@ -45,10 +63,16 @@
 			break;						\
 		case 2:							\
 		case 4:							\
-			asm(".section __imv,\"a\",@progbits\n\t"	\
+			asm(".section __discard,\"\",@progbits\n\t"	\
+				"1:\n\t"				\
+				"mov $0,%0\n\t"				\
+				"2:\n\t"				\
+				".previous\n\t"				\
+				".section __imv,\"a\",@progbits\n\t"	\
 				_ASM_PTR "%c1, (3f)-%c2\n\t"		\
-				".byte %c2\n\t"				\
+				".byte %c2, (2b-1b)\n\t"		\
 				".previous\n\t"				\
+				".org . + ((-.-(2b-1b)) & (%c2-1)), 0x90\n\t" \
 				"mov $0,%0\n\t"				\
 				"3:\n\t"				\
 				: "=r" (value)				\
@@ -60,10 +84,16 @@
 				value = name##__imv;			\
 				break;					\
 			}						\
-			asm(".section __imv,\"a\",@progbits\n\t"	\
+			asm(".section __discard,\"\",@progbits\n\t"	\
+				"1:\n\t"				\
+				"mov $0xFEFEFEFE01010101,%0\n\t" 	\
+				"2:\n\t"				\
+				".previous\n\t"				\
+				".section __imv,\"a\",@progbits\n\t"	\
 				_ASM_PTR "%c1, (3f)-%c2\n\t"		\
-				".byte %c2\n\t"				\
+				".byte %c2, (2b-1b)\n\t"		\
 				".previous\n\t"				\
+				".org . + ((-.-(2b-1b)) & (%c2-1)), 0x90\n\t" \
 				"mov $0xFEFEFEFE01010101,%0\n\t" 	\
 				"3:\n\t"				\
 				: "=r" (value)				\
@@ -74,4 +104,6 @@
 		value;							\
 	})
 
+extern int arch_imv_update(const struct __imv *imv, int early);
+
 #endif /* _ASM_X86_IMMEDIATE_H */
arch/x86/kernel/traps_32.c +6 -4
Index: linux-2.6-lttng.mm/arch/x86/kernel/traps_32.c
===================================================================
@@ -552,7 +552,7 @@ void do_##name(struct pt_regs * regs, lo
 }
 
 DO_VM86_ERROR_INFO( 0, SIGFPE,  "divide error", divide_error, FPE_INTDIV, regs->ip)
-#ifndef CONFIG_KPROBES
+#if !defined(CONFIG_KPROBES) && !defined(CONFIG_IMMEDIATE)
 DO_VM86_ERROR( 3, SIGTRAP, "int3", int3)
 #endif
 DO_VM86_ERROR( 4, SIGSEGV, "overflow", overflow)
@@ -794,7 +794,7 @@ void restart_nmi(void)
 	acpi_nmi_enable();
 }
 
-#ifdef CONFIG_KPROBES
+#if defined(CONFIG_KPROBES) || defined(CONFIG_IMMEDIATE)
 void __kprobes do_int3(struct pt_regs *regs, long error_code)
 {
 	trace_hardirqs_fixup();
@@ -802,8 +802,10 @@ void __kprobes do_int3(struct pt_regs *r
 	if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
 			== NOTIFY_STOP)
 		return;
-	/* This is an interrupt gate, because kprobes wants interrupts
-	disabled.  Normal trap handlers don't. */
+	/*
+	 * This is an interrupt gate, because kprobes and immediate values wants
+	 * interrupts disabled.  Normal trap handlers don't.
+	 */
 	restore_interrupts(regs);
 	do_trap(3, SIGTRAP, "int3", 1, regs, error_code, NULL);
 }
arch/x86/kernel/immediate.c new file +281 -0

281 lines not on this page · raw message

arch/x86/kernel/Makefile +1 -0
Index: linux-2.6-lttng.mm/arch/x86/kernel/Makefile
===================================================================
@@ -70,6 +70,7 @@ obj-$(CONFIG_MGEODE_LX)		+= geode_32.o m
 obj-$(CONFIG_VMI)		+= vmi_32.o vmiclock_32.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o paravirt_patch_$(BITS).o
 obj-y				+= pcspeaker.o
+obj-$(CONFIG_IMMEDIATE)		+= immediate.o
 
 obj-$(CONFIG_SCx200)		+= scx200_32.o
 
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#45 lore raw

PATCH 24/26 Immediate Values - Powerpc Optimization NMI MCE support

In reply to Mathieu Desnoyers

Use an atomic update for immediate values.

CC Rusty Russell <[email protected]>
CC Christoph Hellwig <[email protected]>
CC Paul Mackerras <[email protected]>
---
 arch/powerpc/kernel/Makefile    |    1 
 arch/powerpc/kernel/immediate.c |   73 ++++++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/immediate.h |   18 +++++++++
 3 files changed, 92 insertions(+)
arch/powerpc/kernel/immediate.c new file +73 -0
Index: linux-2.6-lttng/arch/powerpc/kernel/immediate.c
===================================================================
@@ -0,0 +1,73 @@
+/*
+ * Powerpc optimized immediate values enabling/disabling.
+ *
+ * Mathieu Desnoyers <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/immediate.h>
+#include <linux/string.h>
+#include <linux/kprobes.h>
+#include <asm/cacheflush.h>
+#include <asm/page.h>
+
+#define LI_OPCODE_LEN	2
+
+/**
+ * arch_imv_update - update one immediate value
+ * @imv: pointer of type const struct __imv to update
+ * @early: early boot (1), normal (0)
+ *
+ * Update one immediate value. Must be called with imv_mutex held.
+ */
+int arch_imv_update(const struct __imv *imv, int early)
+{
+#ifdef CONFIG_KPROBES
+	kprobe_opcode_t *insn;
+	/*
+	 * Fail if a kprobe has been set on this instruction.
+	 * (TODO: we could eventually do better and modify all the (possibly
+	 * nested) kprobes for this site if kprobes had an API for this.
+	 */
+	switch (imv->size) {
+	case 1:	/* The uint8_t points to the 3rd byte of the
+		 * instruction */
+		insn = (void *)(imv->imv - 1 - LI_OPCODE_LEN);
+		break;
+	case 2:	insn = (void *)(imv->imv - LI_OPCODE_LEN);
+		break;
+	default:
+	return -EINVAL;
+	}
+
+	if (unlikely(!early && *insn == BREAKPOINT_INSTRUCTION)) {
+		printk(KERN_WARNING "Immediate value in conflict with kprobe. "
+				    "Variable at %p, "
+				    "instruction at %p, size %lu\n",
+				    (void *)imv->imv,
+				    (void *)imv->var, imv->size);
+		return -EBUSY;
+	}
+#endif
+
+	/*
+	 * If the variable and the instruction have the same value, there is
+	 * nothing to do.
+	 */
+	switch (imv->size) {
+	case 1:	if (*(uint8_t *)imv->imv
+				== *(uint8_t *)imv->var)
+			return 0;
+		break;
+	case 2:	if (*(uint16_t *)imv->imv
+				== *(uint16_t *)imv->var)
+			return 0;
+		break;
+	default:return -EINVAL;
+	}
+	memcpy((void *)imv->imv, (void *)imv->var,
+			imv->size);
+	flush_icache_range(imv->imv,
+		imv->imv + imv->size);
+	return 0;
+}
include/asm-powerpc/immediate.h +18 -0
Index: linux-2.6-lttng/include/asm-powerpc/immediate.h
===================================================================
@@ -12,6 +12,16 @@
 
 #include <asm/asm-compat.h>
 
+struct __imv {
+	unsigned long var;	/* Identifier variable of the immediate value */
+	unsigned long imv;	/*
+				 * Pointer to the memory location that holds
+				 * the immediate value within the load immediate
+				 * instruction.
+				 */
+	unsigned char size;	/* Type size. */
+} __attribute__ ((packed));
+
 /**
  * imv_read - read immediate variable
  * @name: immediate value name
@@ -19,6 +29,11 @@
  * Reads the value of @name.
  * Optimized version of the immediate.
  * Do not use in __init and __exit functions. Use _imv_read() instead.
+ * Makes sure the 2 bytes update will be atomic by aligning the immediate
+ * value. Use a normal memory read for the 4 bytes immediate because there is no
+ * way to atomically update it without using a seqlock read side, which would
+ * cost more in term of total i-cache and d-cache space than a simple memory
+ * read.
  */
 #define imv_read(name)							\
 	({								\
@@ -40,6 +55,7 @@
 					PPC_LONG "%c1, ((1f)-2)\n\t"	\
 					".byte 2\n\t"			\
 					".previous\n\t"			\
+					".align 2\n\t"			\
 					"li %0,0\n\t"			\
 					"1:\n\t"			\
 				: "=r" (value)				\
@@ -52,4 +68,6 @@
 		value;							\
 	})
 
+extern int arch_imv_update(const struct __imv *imv, int early);
+
 #endif /* _ASM_POWERPC_IMMEDIATE_H */
arch/powerpc/kernel/Makefile +1 -0
Index: linux-2.6-lttng/arch/powerpc/kernel/Makefile
===================================================================
@@ -91,3 +91,4 @@ obj-$(CONFIG_PPC64)		+= $(obj64-y)
 
 extra-$(CONFIG_PPC_FPU)		+= fpu.o
 extra-$(CONFIG_PPC64)		+= entry_64.o
+obj-$(CONFIG_IMMEDIATE)		+= immediate.o
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#46 lore raw

PATCH 25/26 Immediate Values Use Arch NMI and MCE Support

In reply to Mathieu Desnoyers

Remove the architecture agnostic code now replaced by architecture specific, atomic instruction updates.

---
 include/linux/immediate.h |   11 ----
 kernel/immediate.c        |  113 +---------------------------------------------
 2 files changed, 4 insertions(+), 120 deletions(-)
kernel/immediate.c +4 -109
Index: linux-2.6-lttng/kernel/immediate.c
===================================================================
@@ -19,9 +19,6 @@
 #include <linux/mutex.h>
 #include <linux/immediate.h>
 #include <linux/memory.h>
-#include <linux/cpu.h>
-
-#include <asm/cacheflush.h>
 
 /*
  * Kernel ready to execute the SMP update that may depend on trap and ipi.
@@ -37,111 +34,6 @@ extern const struct __imv __stop___imv[]
  */
 static DEFINE_MUTEX(imv_mutex);
 
-static atomic_t wait_sync;
-
-struct ipi_loop_data {
-	long value;
-	const struct __imv *imv;
-} loop_data;
-
-static void ipi_busy_loop(void *arg)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	atomic_dec(&wait_sync);
-	do {
-		/* Make sure the wait_sync gets re-read */
-		smp_mb();
-	} while (atomic_read(&wait_sync) > loop_data.value);
-	atomic_dec(&wait_sync);
-	do {
-		/* Make sure the wait_sync gets re-read */
-		smp_mb();
-	} while (atomic_read(&wait_sync) > 0);
-	/*
-	 * Issuing a synchronizing instruction must be done on each CPU before
-	 * reenabling interrupts after modifying an instruction. Required by
-	 * Intel's errata.
-	 */
-	sync_core();
-	flush_icache_range(loop_data.imv->imv,
-		loop_data.imv->imv + loop_data.imv->size);
-	local_irq_restore(flags);
-}
-
-/**
- * apply_imv_update - update one immediate value
- * @imv: pointer of type const struct __imv to update
- *
- * Update one immediate value. Must be called with imv_mutex held.
- * It makes sure all CPUs are not executing the modified code by having them
- * busy looping with interrupts disabled.
- * It does _not_ protect against NMI and MCE (could be a problem with Intel's
- * errata if we use immediate values in their code path).
- */
-static int apply_imv_update(const struct __imv *imv)
-{
-	unsigned long flags;
-	long online_cpus;
-
-	/*
-	 * If the variable and the instruction have the same value, there is
-	 * nothing to do.
-	 */
-	switch (imv->size) {
-	case 1:	if (*(uint8_t *)imv->imv
-				== *(uint8_t *)imv->var)
-			return 0;
-		break;
-	case 2:	if (*(uint16_t *)imv->imv
-				== *(uint16_t *)imv->var)
-			return 0;
-		break;
-	case 4:	if (*(uint32_t *)imv->imv
-				== *(uint32_t *)imv->var)
-			return 0;
-		break;
-	case 8:	if (*(uint64_t *)imv->imv
-				== *(uint64_t *)imv->var)
-			return 0;
-		break;
-	default:return -EINVAL;
-	}
-
-	if (imv_early_boot_complete) {
-		kernel_text_lock();
-		lock_cpu_hotplug();
-		online_cpus = num_online_cpus();
-		atomic_set(&wait_sync, 2 * online_cpus);
-		loop_data.value = online_cpus;
-		loop_data.imv = imv;
-		smp_call_function(ipi_busy_loop, NULL, 1, 0);
-		local_irq_save(flags);
-		atomic_dec(&wait_sync);
-		do {
-			/* Make sure the wait_sync gets re-read */
-			smp_mb();
-		} while (atomic_read(&wait_sync) > online_cpus);
-		text_poke((void *)imv->imv, (void *)imv->var,
-				imv->size);
-		/*
-		 * Make sure the modified instruction is seen by all CPUs before
-		 * we continue (visible to other CPUs and local interrupts).
-		 */
-		wmb();
-		atomic_dec(&wait_sync);
-		flush_icache_range(imv->imv,
-				imv->imv + imv->size);
-		local_irq_restore(flags);
-		unlock_cpu_hotplug();
-		kernel_text_unlock();
-	} else
-		text_poke_early((void *)imv->imv, (void *)imv->var,
-				imv->size);
-	return 0;
-}
-
 /**
  * imv_update_range - Update immediate values in a range
  * @begin: pointer to the beginning of the range
@@ -154,9 +46,12 @@ void imv_update_range(const struct __imv
 {
 	const struct __imv *iter;
 	int ret;
+
 	for (iter = begin; iter < end; iter++) {
 		mutex_lock(&imv_mutex);
-		ret = apply_imv_update(iter);
+		kernel_text_lock();
+		ret = arch_imv_update(iter, !imv_early_boot_complete);
+		kernel_text_unlock();
 		if (imv_early_boot_complete && ret)
 			printk(KERN_WARNING
 				"Invalid immediate value. "
include/linux/immediate.h +0 -11
Index: linux-2.6-lttng/include/linux/immediate.h
===================================================================
@@ -12,17 +12,6 @@
 
 #ifdef CONFIG_IMMEDIATE
 
-struct __imv {
-	unsigned long var;	/* Pointer to the identifier variable of the
-				 * immediate value
-				 */
-	unsigned long imv;	/*
-				 * Pointer to the memory location of the
-				 * immediate value within the instruction.
-				 */
-	unsigned char size;	/* Type size. */
-} __attribute__ ((packed));
-
 #include <asm/immediate.h>
 
 /**
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#47 lore raw

PATCH 26/26 Linux Kernel Markers - Use Immediate Values

In reply to Mathieu Desnoyers

Make markers use immediate values.

Changelog :
- Use imv_* instead of immediate_*.

---
 Documentation/markers.txt |   17 +++++++++++++----
 include/linux/marker.h    |   42 ++++++++++++++++++++++++++++++++----------
 kernel/marker.c           |    8 ++++++--
 kernel/module.c           |    1 +
 4 files changed, 52 insertions(+), 16 deletions(-)
include/linux/marker.h +32 -10
Index: linux-2.6-lttng.mm/include/linux/marker.h
===================================================================
@@ -12,6 +12,7 @@
  * See the file COPYING for more details.
  */
 
+#include <linux/immediate.h>
 #include <linux/types.h>
 
 struct module;
@@ -42,7 +43,7 @@ struct marker {
 	const char *format;	/* Marker format string, describing the
 				 * variable argument list.
 				 */
-	char state;		/* Marker state. */
+	DEFINE_IMV(char, state);/* Immediate value state. */
 	char ptype;		/* probe type : 0 : single, 1 : multi */
 	void (*call)(const struct marker *mdata,	/* Probe wrapper */
 		void *call_private, const char *fmt, ...);
@@ -53,13 +54,14 @@ struct marker {
 #ifdef CONFIG_MARKERS
 
 /*
+ * Generic marker flavor always available.
  * Note : the empty asm volatile with read constraint is used here instead of a
  * "used" attribute to fix a gcc 4.1.x bug.
  * Make sure the alignment of the structure in the __markers section will
  * not add unwanted padding between the beginning of the section and the
  * structure. Force alignment to the same alignment as the section start.
  */
-#define __trace_mark(name, call_private, format, args...)		\
+#define __trace_mark(generic, name, call_private, format, args...)	\
 	do {								\
 		static const char __mstrtab_##name[]			\
 		__attribute__((section("__markers_strings")))		\
@@ -70,17 +72,23 @@ struct marker {
 		0, 0, marker_probe_cb,					\
 		{ __mark_empty_function, NULL}, NULL };			\
 		__mark_check_format(format, ## args);			\
-		if (unlikely(__mark_##name.state)) {			\
-			(*__mark_##name.call)				\
-				(&__mark_##name, call_private,		\
-				format, ## args);			\
+		if (!generic) {						\
+			if (unlikely(imv_read(__mark_##name.state)))	\
+				(*__mark_##name.call)			\
+					(&__mark_##name, call_private,	\
+					format, ## args);		\
+		} else {						\
+			if (unlikely(_imv_read(__mark_##name.state)))	\
+				(*__mark_##name.call)			\
+					(&__mark_##name, call_private,	\
+					format, ## args);		\
 		}							\
 	} while (0)
 
 extern void marker_update_probe_range(struct marker *begin,
 	struct marker *end);
 #else /* !CONFIG_MARKERS */
-#define __trace_mark(name, call_private, format, args...) \
+#define __trace_mark(generic, name, call_private, format, args...) \
 		__mark_check_format(format, ## args)
 static inline void marker_update_probe_range(struct marker *begin,
 	struct marker *end)
@@ -88,15 +96,29 @@ static inline void marker_update_probe_r
 #endif /* CONFIG_MARKERS */
 
 /**
- * trace_mark - Marker
+ * trace_mark - Marker using code patching
  * @name: marker name, not quoted.
  * @format: format string
  * @args...: variable argument list
  *
- * Places a marker.
+ * Places a marker using optimized code patching technique (imv_read())
+ * to be enabled.
  */
 #define trace_mark(name, format, args...) \
-	__trace_mark(name, NULL, format, ## args)
+	__trace_mark(0, name, NULL, format, ## args)
+
+/**
+ * _trace_mark - Marker using variable read
+ * @name: marker name, not quoted.
+ * @format: format string
+ * @args...: variable argument list
+ *
+ * Places a marker using a standard memory read (_imv_read()) to be
+ * enabled. Should be used for markers in __init and __exit functions and in
+ * lockdep code.
+ */
+#define _trace_mark(name, format, args...) \
+	__trace_mark(1, name, NULL, format, ## args)
 
 /**
  * MARK_NOARGS - Format string for a marker with no argument.
kernel/marker.c +6 -2
Index: linux-2.6-lttng.mm/kernel/marker.c
===================================================================
@@ -23,6 +23,7 @@
 #include <linux/rcupdate.h>
 #include <linux/marker.h>
 #include <linux/err.h>
+#include <linux/immediate.h>
 
 extern struct marker __start___markers[];
 extern struct marker __stop___markers[];
@@ -544,7 +545,7 @@ static int set_marker(struct marker_entr
 	 */
 	smp_wmb();
 	elem->ptype = (*entry)->ptype;
-	elem->state = active;
+	elem->state__imv = active;
 
 	return 0;
 }
@@ -558,7 +559,7 @@ static int set_marker(struct marker_entr
 static void disable_marker(struct marker *elem)
 {
 	/* leave "call" as is. It is known statically. */
-	elem->state = 0;
+	elem->state__imv = 0;
 	elem->single.func = __mark_empty_function;
 	/* Update the function before setting the ptype */
 	smp_wmb();
@@ -625,6 +626,9 @@ static void marker_update_probes(void)
 	marker_update_probe_range(__start___markers, __stop___markers);
 	/* Markers in modules. */
 	module_update_markers();
+	/* Update immediate values */
+	core_imv_update();
+	module_imv_update();
 }
 
 /**
Documentation/markers.txt +13 -4
Index: linux-2.6-lttng.mm/Documentation/markers.txt
===================================================================
@@ -15,10 +15,12 @@ provide at runtime. A marker can be "on"
 (no probe is attached). When a marker is "off" it has no effect, except for
 adding a tiny time penalty (checking a condition for a branch) and space
 penalty (adding a few bytes for the function call at the end of the
-instrumented function and adds a data structure in a separate section).  When a
-marker is "on", the function you provide is called each time the marker is
-executed, in the execution context of the caller. When the function provided
-ends its execution, it returns to the caller (continuing from the marker site).
+instrumented function and adds a data structure in a separate section). The
+immediate values are used to minimize the impact on data cache, encoding the
+condition in the instruction stream. When a marker is "on", the function you
+provide is called each time the marker is executed, in the execution context of
+the caller. When the function provided ends its execution, it returns to the
+caller (continuing from the marker site).
 
 You can put markers at important locations in the code. Markers are
 lightweight hooks that can pass an arbitrary number of parameters,
@@ -69,6 +71,13 @@ a printk warning which identifies the in
 "Format mismatch for probe probe_name (format), marker (format)"
 
 
+* Optimization for a given architecture
+
+To force use of a non-optimized version of the markers, _trace_mark() should be
+used. It takes the same parameters as the normal markers, but it does not use
+the immediate values based on code patching.
+
+
 * Probe / marker example
 
 See the example provided in samples/markers/src
kernel/module.c +1 -0
Index: linux-2.6-lttng.mm/kernel/module.c
===================================================================
@@ -2002,6 +2002,7 @@ static struct module *load_module(void _
 			mod->markers + mod->num_markers);
 #endif
 #ifdef CONFIG_IMMEDIATE
+		/* Immediate values must be updated after markers */
 		imv_update_range(mod->immediate,
 			mod->immediate + mod->num_immediate);
 #endif
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68