2008-10-03 19:16:39

by Theodore Ts'o

[permalink] [raw]
Subject: 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


2008-10-03 19:54:49

by Mathieu Desnoyers

[permalink] [raw]
Subject: [PATCH] Marker depmod fix core kernel list

* Theodore Ts'o ([email protected]) wrote:
>
> 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,
>


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

Especially :

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index a07f91a..8f038e6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -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!

Signed-off-by: Mathieu Desnoyers <[email protected]>
CC: Theodore Ts'o <[email protected]>
CC: David Smith <[email protected]>
CC: Roland McGrath <[email protected]>
CC: Sam Ravnborg <[email protected]>
CC: Wenji Huang <[email protected]>
CC: Takashi Nishiie <[email protected]>
---
scripts/mod/modpost.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-2.6-lttng/scripts/mod/modpost.c
===================================================================
--- linux-2.6-lttng.orig/scripts/mod/modpost.c 2008-10-03 15:42:00.000000000 -0400
+++ linux-2.6-lttng/scripts/mod/modpost.c 2008-10-03 15:42:59.000000000 -0400
@@ -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

2008-10-04 15:25:27

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] Marker depmod fix core kernel list

On Fri, Oct 03, 2008 at 03:54:36PM -0400, Mathieu Desnoyers wrote:
> 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.

Tested-by: "Theodore Ts'o" <[email protected]>

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

2008-10-06 04:29:54

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Marker depmod fix core kernel list

On Sat, Oct 04, 2008 at 11:24:56AM -0400, Theodore Tso wrote:
> On Fri, Oct 03, 2008 at 03:54:36PM -0400, Mathieu Desnoyers wrote:
> > 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.
>
> Tested-by: "Theodore Ts'o" <[email protected]>
>
> 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.

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

thanks,

greg k-h

2008-10-06 20:23:43

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] Marker depmod fix core kernel list

> 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?

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

2008-10-10 09:40:11

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] Marker depmod fix core kernel list


* Greg KH <[email protected]> wrote:

> On Sat, Oct 04, 2008 at 11:24:56AM -0400, Theodore Tso wrote:
> > On Fri, Oct 03, 2008 at 03:54:36PM -0400, Mathieu Desnoyers wrote:
> > > 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.
> >
> > Tested-by: "Theodore Ts'o" <[email protected]>
> >
> > 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.
>
> If it misses .27, can someone forward it to [email protected] when it
> goes into Linus's tree?
>
> thanks,

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 87f3b6b6fbcbfa715f0d0db3e7a63e65716a6d4e Mon Sep 17 00:00:00 2001
From: Mathieu Desnoyers <[email protected]>
Date: Mon, 6 Oct 2008 09:30:12 -0400
Subject: [PATCH] Marker depmod fix core kernel list

* Theodore Ts'o ([email protected]) wrote:
>
> 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?

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!

Signed-off-by: Mathieu Desnoyers <[email protected]>
Tested-by: "Theodore Ts'o" <[email protected]>
CC: Greg KH <[email protected]>
CC: David Smith <[email protected]>
CC: Roland McGrath <[email protected]>
CC: Sam Ravnborg <[email protected]>
CC: Wenji Huang <[email protected]>
CC: Takashi Nishiie <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
---
scripts/mod/modpost.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 418cd7d..8e0de6a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -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);

2008-10-10 09:40:47

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] Marker depmod fix core kernel list


* Ingo Molnar <[email protected]> wrote:

> 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.

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

Ingo

2008-10-10 09:47:24

by Alan

[permalink] [raw]
Subject: [PATCH RESEND] early_ioremap has a fencepost error

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

Signed-off-by: Alan Cox <[email protected]>
---

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


diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index d4b6e6a..d0975fc 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -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.

2008-10-10 10:22:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH RESEND] early_ioremap has a fencepost error


* Alan Cox <[email protected]> wrote:

> 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

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 dd5698f42a5f2b494c3e811598403f105b00f4f2 Mon Sep 17 00:00:00 2001
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

Signed-off-by: Alan Cox <[email protected]>
Cc: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/mm/ioremap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index d4b6e6a..d0975fc 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -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.

2008-10-10 15:53:25

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Marker depmod fix core kernel list

On Fri, Oct 10, 2008 at 11:39:42AM +0200, Ingo Molnar wrote:
>
> * Ingo Molnar <[email protected]> wrote:
>
> > 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.
>
> ah, ignore me - we broke it in .27-rc1 so .26 and earlier is not
> affected.

Gladly ignored :)

thanks,

greg k-h