2008-06-28 22:26:26

by Anders Kaseorg

[permalink] [raw]
Subject: [PATCH] x86_64: Patch paravirt inline replacements when loading modules

Paravirt replacements were added to the i386 module loader by commit
139ec7c416248b9ea227d21839235344edfee1e0. This adds the same code to
the x86_64 module loader.

Signed-off-by: Anders Kaseorg <[email protected]>
---
arch/x86/kernel/module_64.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/module_64.c b/arch/x86/kernel/module_64.c
index a888e67..0e86767 100644
--- a/arch/x86/kernel/module_64.c
+++ b/arch/x86/kernel/module_64.c
@@ -150,7 +150,8 @@ int module_finalize(const Elf_Ehdr *hdr,
const Elf_Shdr *sechdrs,
struct module *me)
{
- const Elf_Shdr *s, *text = NULL, *alt = NULL, *locks = NULL;
+ const Elf_Shdr *s, *text = NULL, *alt = NULL, *locks = NULL,
+ *para = NULL;
char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;

for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
@@ -160,6 +161,8 @@ int module_finalize(const Elf_Ehdr *hdr,
alt = s;
if (!strcmp(".smp_locks", secstrings + s->sh_name))
locks= s;
+ if (!strcmp(".parainstructions", secstrings + s->sh_name))
+ para = s;
}

if (alt) {
@@ -175,6 +178,11 @@ int module_finalize(const Elf_Ehdr *hdr,
tseg, tseg + text->sh_size);
}

+ if (para) {
+ void *pseg = (void *)para->sh_addr;
+ apply_paravirt(pseg, pseg + para->sh_size);
+ }
+
return module_bug_finalize(hdr, sechdrs, me);
}

--
1.5.4.3


2008-06-28 22:32:23

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86_64: Patch paravirt inline replacements when loading modules

Anders H Kaseorg wrote:
> Paravirt replacements were added to the i386 module loader by commit
> 139ec7c416248b9ea227d21839235344edfee1e0. This adds the same code to
> the x86_64 module loader.

Not having looked at this in detail, is this a *current* problem or a
*future* problem, i.e. does it fix a bug that's currently in the kernel?

-hpa

2008-06-28 22:46:06

by Anders Kaseorg

[permalink] [raw]
Subject: Re: [PATCH] x86_64: Patch paravirt inline replacements when loading modules

On Sat, 28 Jun 2008, H. Peter Anvin wrote:
> Not having looked at this in detail, is this a *current* problem or a
> *future* problem, i.e. does it fix a bug that's currently in the kernel?

It's a current problem. The current situation is that .parainstructions
are patched into the kernel core and modules on i386, but only the kernel
core on x86_64. This results in a slight performance regression in x86_64
modules, compared to the situation before .parainstructions existed. My
patch restores full performance of x86_64 modules.

Anders

2008-06-28 23:17:04

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86_64: Patch paravirt inline replacements when loading modules

Anders H Kaseorg wrote:
> On Sat, 28 Jun 2008, H. Peter Anvin wrote:
>> Not having looked at this in detail, is this a *current* problem or a
>> *future* problem, i.e. does it fix a bug that's currently in the kernel?
>
> It's a current problem. The current situation is that .parainstructions
> are patched into the kernel core and modules on i386, but only the
> kernel core on x86_64. This results in a slight performance regression
> in x86_64 modules, compared to the situation before .parainstructions
> existed. My patch restores full performance of x86_64 modules.

OK, it's a performance regression. I don't think it makes sense for
2.6.26, but it should definitely go into 2.6.27.

Thanks!

-hpa

2008-06-30 10:15:52

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86_64: Patch paravirt inline replacements when loading modules


* H. Peter Anvin <[email protected]> wrote:

> Anders H Kaseorg wrote:
>> On Sat, 28 Jun 2008, H. Peter Anvin wrote:
>>> Not having looked at this in detail, is this a *current* problem or a
>>> *future* problem, i.e. does it fix a bug that's currently in the
>>> kernel?
>>
>> It's a current problem. The current situation is that
>> .parainstructions are patched into the kernel core and modules on i386,
>> but only the kernel core on x86_64. This results in a slight
>> performance regression in x86_64 modules, compared to the situation
>> before .parainstructions existed. My patch restores full performance
>> of x86_64 modules.
>
> OK, it's a performance regression. I don't think it makes sense for
> 2.6.26, but it should definitely go into 2.6.27.

applied to tip/x86/modules - thanks Anders.

( Note: the patch had diff context tabs converted to spaces and other
whitespace damage, i fixed that up by hand for the patch to apply. See
Documentation/email-clients.txt about how to avoid it with future
patches. See the commit below in its final form. )

Ingo

---------->
commit 5e5a29bf2624a5984e1c36c3a2481ee91249ec9c
Author: Anders H Kaseorg <[email protected]>
Date: Sat Jun 28 18:25:41 2008 -0400

x86, 64-bit: patch paravirt inline replacements when loading modules

small speedup.

Paravirt replacements were added to the i386 module loader by commit
139ec7c416248b9ea227d21839235344edfee1e0. This adds the same code to
the x86_64 module loader.

Signed-off-by: Anders Kaseorg <[email protected]>
Acked-by: "H. Peter Anvin" <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>

diff --git a/arch/x86/kernel/module_64.c b/arch/x86/kernel/module_64.c
index a888e67..0e86767 100644
--- a/arch/x86/kernel/module_64.c
+++ b/arch/x86/kernel/module_64.c
@@ -150,7 +150,8 @@ int module_finalize(const Elf_Ehdr *hdr,
const Elf_Shdr *sechdrs,
struct module *me)
{
- const Elf_Shdr *s, *text = NULL, *alt = NULL, *locks = NULL;
+ const Elf_Shdr *s, *text = NULL, *alt = NULL, *locks = NULL,
+ *para = NULL;
char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;

for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
@@ -160,6 +161,8 @@ int module_finalize(const Elf_Ehdr *hdr,
alt = s;
if (!strcmp(".smp_locks", secstrings + s->sh_name))
locks= s;
+ if (!strcmp(".parainstructions", secstrings + s->sh_name))
+ para = s;
}

if (alt) {
@@ -175,6 +178,11 @@ int module_finalize(const Elf_Ehdr *hdr,
tseg, tseg + text->sh_size);
}

+ if (para) {
+ void *pseg = (void *)para->sh_addr;
+ apply_paravirt(pseg, pseg + para->sh_size);
+ }
+
return module_bug_finalize(hdr, sechdrs, me);
}