Markers in (non-module) kernel code?

I've been playing with adding some markers into ext4 to see if they could be useful in solving some problems along with Systemtap. It appears, though, that as of 2.6.27-rc8, markers defined in code which is compiled directly into the kernel (i.e., not as modules) don't show up in Module.markers:

kvm_trace_entryexit arch/x86/kvm/kvm-intel %u %p %u %u %u %u %u %u
kvm_trace_handler arch/x86/kvm/kvm-intel %u %p %u %u %u %u %u %u
kvm_trace_entryexit arch/x86/kvm/kvm-amd %u %p %u %u %u %u %u %u
kvm_trace_handler arch/x86/kvm/kvm-amd %u %p %u %u %u %u %u %u

(Note the lack of any of the kernel_sched_* markers, and the markers I added for ext4_* and jbd2_* are missing as wel.)

Systemtap apparently depends on in-kernel trace_mark being recorded in Module.markers, and apparently it's been claimed that it used to be there. Is this a bug in systemtap, or in how Module.markers is getting
built? And is there a file that contains the equivalent information
for markers located in non-modules code?

Thanks, regards,

- Ted

I think the problem comes from this patch :
commit d35cb360c29956510b2fe1a953bd4968536f7216
"markers: fix duplicate modpost entry"

Especially :

index a07f91a..8f038e6 100644
@@ -1992,7 +1992,8 @@ static void read_markers(const char *fname)
      mod->skip = 1;
    }
 
-   add_marker(mod, marker, fmt);
+   if (!mod->skip)
+     add_marker(mod, marker, fmt);
  }
  return;
 fail:

Here is a fix that should take care if this problem. Given I am not the modpost expert, let's see if I can get an ACK from Sam.

Thanks for the bug report!

---
 scripts/mod/modpost.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-2.6-lttng/scripts/mod/modpost.c
===================================================================
@@ -1986,11 +1986,13 @@ static void read_markers(const char *fna
 
 		mod = find_module(modname);
 		if (!mod) {
-			if (is_vmlinux(modname))
-				have_vmlinux = 1;
 			mod = new_module(NOFAIL(strdup(modname)));
 			mod->skip = 1;
 		}
+		if (is_vmlinux(modname)) {
+			have_vmlinux = 1;
+			mod->skip = 0;
+		}
 
 		if (!mod->skip)
 			add_marker(mod, marker, fmt);

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

It works, thanks!! Can we get this pushed to Linus before 2.6.28 opens? This is technically a regression since it was broken around 2.6.27-rc1.

Also, something we need to consider is getting distributions to ship Modules.markers, and where it should be installed. I would argue that it belongs in /lib/modules/uname -r, so maybe "make modules_install" should put it there? This attention to deployability is going to be important if markers are to be successfully used by applications such as Systemtap.

- Ted

If it misses .27, can someone forward it to [email protected] when it goes into Linus's tree?

thanks,

greg k-h

the patch went upstream without a Cc: <[email protected]> tag, so i'm sending it here as a backport request. The upstream commit is below.

Ingo

--------------->

From: Mathieu Desnoyers <[email protected]>
Date: Mon, 6 Oct 2008 09:30:12 -0400
Subject: [PATCH] Marker depmod fix core kernel list

I think the problem comes from "markers: fix duplicate modpost entry" (commit d35cb360c29956510b2fe1a953bd4968536f7216)

Especially :

- add_marker(mod, marker, fmt);
+ if (!mod->skip)

  +     add_marker(mod, marker, fmt);
    }
    return;

fail:

Here is a fix that should take care if this problem.

Thanks for the bug report!

---
 scripts/mod/modpost.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

index 418cd7d..8e0de6a 100644
@@ -1986,11 +1986,13 @@ static void read_markers(const char *fname)
 
 		mod = find_module(modname);
 		if (!mod) {
-			if (is_vmlinux(modname))
-				have_vmlinux = 1;
 			mod = new_module(NOFAIL(strdup(modname)));
 			mod->skip = 1;
 		}
+		if (is_vmlinux(modname)) {
+			have_vmlinux = 1;
+			mod->skip = 0;
+		}
 
 		if (!mod->skip)
 			add_marker(mod, marker, fmt);

ah, ignore me - we broke it in .27-rc1 so .26 and earlier is not affected.

Ingo

Gladly ignored :)

thanks,

greg k-h

Can we get this into 2.6.27.x at some point as its a regression and a crash on boot for some users moving from 2.6.26

--

early_ioremap: fix fencepost error

From: Alan Cox <[email protected]>

The x86 implementation of early_ioremap has an off by one error. If we get an object which ends on the first byte of a page we undermap by one page and this causes a crash on boot with the ASUS P5QL whose DMI table happens to fit this alignment.

The size computation is currently

	last_addr = phys_addr + size - 1;
	npages = (PAGE_ALIGN(last_addr) - phys_addr)

(Consider a request for 1 byte at alignment 0...)

Closes #11693

Debugging work by Ian Campbell/Felix Geyer

---

 arch/x86/mm/ioremap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index d4b6e6a..d0975fc 100644
@@ -595,7 +595,7 @@ void __init *early_ioremap(unsigned long phys_addr, unsigned long size)
 	 */
 	offset = phys_addr & ~PAGE_MASK;
 	phys_addr &= PAGE_MASK;
-	size = PAGE_ALIGN(last_addr) - phys_addr;
+	size = PAGE_ALIGN(last_addr + 1) - phys_addr;
 
 	/*
 	 * Mappings have to fit in the FIX_BTMAP area.

yes, have your fix patch from yesterday in tip/x86/urgent already, with a [email protected] tag included as well - see below. Thanks Alan!

Ingo

-------------->

From: Alan Cox <[email protected]>
Date: Fri, 10 Oct 2008 10:46:45 +0100
Subject: [PATCH] x86, early_ioremap: fix fencepost error

The x86 implementation of early_ioremap has an off by one error. If we get an object which ends on the first byte of a page we undermap by one page and this causes a crash on boot with the ASUS P5QL whose DMI table happens to fit this alignment.

The size computation is currently

	last_addr = phys_addr + size - 1;
	npages = (PAGE_ALIGN(last_addr) - phys_addr)

(Consider a request for 1 byte at alignment 0...)

Closes #11693

Debugging work by Ian Campbell/Felix Geyer

---
 arch/x86/mm/ioremap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index d4b6e6a..d0975fc 100644
@@ -595,7 +595,7 @@ void __init *early_ioremap(unsigned long phys_addr, unsigned long size)
 	 */
 	offset = phys_addr & ~PAGE_MASK;
 	phys_addr &= PAGE_MASK;
-	size = PAGE_ALIGN(last_addr) - phys_addr;
+	size = PAGE_ALIGN(last_addr + 1) - phys_addr;
 
 	/*
 	 * Mappings have to fit in the FIX_BTMAP area.

Fedora kernel RPMs put it in /lib/modules/uname -r/build/, which is where Module.symvers is installed. This lands it in the kernel-devel RPM, which is needed to build modules (and hence by Systemtap).

Since new uses of markers won't necessarily require building modules, your suggestion is probably better.

Thanks,
Roland