The first two patches are simply preparation for the third, making it
effectively trivial, even though it's the only one with a concrete
change in behaviour.
The origins of this patchset are the discovery that unwind and kmemleak
don't always cooperate well with each other - any allocation within
an exit or devexit function causes kmemleak to look up symbols that
aren't in any unwind table. This of course means that all WARN_ONs and
BUGs will suffer the same fate.
It could certainly be said that with a typical system the linked list
has grown too large to be practical as a container, and some improvements
could be made in that direction in the future.
Cheers,
Phil
Less to read.
Signed-off-by: Phil Carmody <[email protected]>
---
arch/arm/kernel/module.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index c628bdf..d531a63 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -71,17 +71,19 @@ int module_frob_arch_sections(Elf_Ehdr *hdr,
Elf_Shdr *s, *sechdrs_end = sechdrs + hdr->e_shnum;
for (s = sechdrs; s < sechdrs_end; s++) {
- if (strcmp(".ARM.exidx.init.text", secstrings + s->sh_name) == 0)
+ char const *secname = secstrings + s->sh_name;
+
+ if (strcmp(".ARM.exidx.init.text", secname) == 0)
mod->arch.unw_sec_init = s;
- else if (strcmp(".ARM.exidx.devinit.text", secstrings + s->sh_name) == 0)
+ else if (strcmp(".ARM.exidx.devinit.text", secname) == 0)
mod->arch.unw_sec_devinit = s;
- else if (strcmp(".ARM.exidx", secstrings + s->sh_name) == 0)
+ else if (strcmp(".ARM.exidx", secname) == 0)
mod->arch.unw_sec_core = s;
- else if (strcmp(".init.text", secstrings + s->sh_name) == 0)
+ else if (strcmp(".init.text", secname) == 0)
mod->arch.sec_init_text = s;
- else if (strcmp(".devinit.text", secstrings + s->sh_name) == 0)
+ else if (strcmp(".devinit.text", secname) == 0)
mod->arch.sec_devinit_text = s;
- else if (strcmp(".text", secstrings + s->sh_name) == 0)
+ else if (strcmp(".text", secname) == 0)
mod->arch.sec_core_text = s;
}
#endif
--
1.6.0.4
The various sections are all dealt with similarly, so factor out
that common behaviour.
Signed-off-by: Phil Carmody <[email protected]>
---
arch/arm/include/asm/module.h | 29 +++++++++++++++----------
arch/arm/kernel/module.c | 46 +++++++++++++++++------------------------
2 files changed, 36 insertions(+), 39 deletions(-)
diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index e4dfa69..2164061 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -7,20 +7,25 @@
struct unwind_table;
-struct mod_arch_specific
-{
#ifdef CONFIG_ARM_UNWIND
- Elf_Shdr *unw_sec_init;
- Elf_Shdr *unw_sec_devinit;
- Elf_Shdr *unw_sec_core;
- Elf_Shdr *sec_init_text;
- Elf_Shdr *sec_devinit_text;
- Elf_Shdr *sec_core_text;
- struct unwind_table *unwind_init;
- struct unwind_table *unwind_devinit;
- struct unwind_table *unwind_core;
-#endif
+struct arm_unwind_mapping {
+ Elf_Shdr *unw_sec;
+ Elf_Shdr *sec_text;
+ struct unwind_table *unwind;
+};
+enum {
+ ARM_SEC_INIT,
+ ARM_SEC_DEVINIT,
+ ARM_SEC_CORE,
+ ARM_SEC_MAX,
};
+struct mod_arch_specific {
+ struct arm_unwind_mapping map[ARM_SEC_MAX];
+};
+#else
+struct mod_arch_specific {
+}
+#endif
/*
* Include the ARM architecture version.
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index d531a63..bcf928e 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -69,22 +69,23 @@ int module_frob_arch_sections(Elf_Ehdr *hdr,
{
#ifdef CONFIG_ARM_UNWIND
Elf_Shdr *s, *sechdrs_end = sechdrs + hdr->e_shnum;
+ struct arm_unwind_mapping *maps = mod->arch.map;
for (s = sechdrs; s < sechdrs_end; s++) {
char const *secname = secstrings + s->sh_name;
if (strcmp(".ARM.exidx.init.text", secname) == 0)
- mod->arch.unw_sec_init = s;
+ maps[ARM_SEC_INIT].unw_sec = s;
else if (strcmp(".ARM.exidx.devinit.text", secname) == 0)
- mod->arch.unw_sec_devinit = s;
+ maps[ARM_SEC_DEVINIT].unw_sec = s;
else if (strcmp(".ARM.exidx", secname) == 0)
- mod->arch.unw_sec_core = s;
+ maps[ARM_SEC_CORE].unw_sec = s;
else if (strcmp(".init.text", secname) == 0)
- mod->arch.sec_init_text = s;
+ maps[ARM_SEC_INIT].sec_text = s;
else if (strcmp(".devinit.text", secname) == 0)
- mod->arch.sec_devinit_text = s;
+ maps[ARM_SEC_DEVINIT].sec_text = s;
else if (strcmp(".text", secname) == 0)
- mod->arch.sec_core_text = s;
+ maps[ARM_SEC_CORE].sec_text = s;
}
#endif
return 0;
@@ -260,31 +261,22 @@ apply_relocate_add(Elf32_Shdr *sechdrs, const char *strtab,
#ifdef CONFIG_ARM_UNWIND
static void register_unwind_tables(struct module *mod)
{
- if (mod->arch.unw_sec_init && mod->arch.sec_init_text)
- mod->arch.unwind_init =
- unwind_table_add(mod->arch.unw_sec_init->sh_addr,
- mod->arch.unw_sec_init->sh_size,
- mod->arch.sec_init_text->sh_addr,
- mod->arch.sec_init_text->sh_size);
- if (mod->arch.unw_sec_devinit && mod->arch.sec_devinit_text)
- mod->arch.unwind_devinit =
- unwind_table_add(mod->arch.unw_sec_devinit->sh_addr,
- mod->arch.unw_sec_devinit->sh_size,
- mod->arch.sec_devinit_text->sh_addr,
- mod->arch.sec_devinit_text->sh_size);
- if (mod->arch.unw_sec_core && mod->arch.sec_core_text)
- mod->arch.unwind_core =
- unwind_table_add(mod->arch.unw_sec_core->sh_addr,
- mod->arch.unw_sec_core->sh_size,
- mod->arch.sec_core_text->sh_addr,
- mod->arch.sec_core_text->sh_size);
+ int i;
+ for (i = 0; i < ARM_SEC_MAX; ++i) {
+ struct arm_unwind_mapping *map = &mod->arch.map[i];
+ if (map->unw_sec && map->sec_text)
+ map->unwind = unwind_table_add(map->unw_sec->sh_addr,
+ map->unw_sec->sh_size,
+ map->sec_text->sh_addr,
+ map->sec_text->sh_size);
+ }
}
static void unregister_unwind_tables(struct module *mod)
{
- unwind_table_del(mod->arch.unwind_init);
- unwind_table_del(mod->arch.unwind_devinit);
- unwind_table_del(mod->arch.unwind_core);
+ int i = ARM_SEC_MAX;
+ while (--i >= 0)
+ unwind_table_del(mod->arch.map[i].unwind);
}
#else
static inline void register_unwind_tables(struct module *mod) { }
--
1.6.0.4
Without these, exit functions cannot be stack-traced, so to speak.
This implies that module unloads that perform allocations (don't
laugh) will cause noisy warnings on the console when kmemleak is
enabled, as it presumes that all code's call chains are traceable.
Similarly, BUGs and WARN_ONs will give additional console spam.
Signed-off-by: Phil Carmody <[email protected]>
---
arch/arm/include/asm/module.h | 2 ++
arch/arm/kernel/module.c | 8 ++++++++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index 2164061..fcff0d2 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -17,6 +17,8 @@ enum {
ARM_SEC_INIT,
ARM_SEC_DEVINIT,
ARM_SEC_CORE,
+ ARM_SEC_EXIT,
+ ARM_SEC_DEVEXIT,
ARM_SEC_MAX,
};
struct mod_arch_specific {
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index bcf928e..8eaff1c 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -80,12 +80,20 @@ int module_frob_arch_sections(Elf_Ehdr *hdr,
maps[ARM_SEC_DEVINIT].unw_sec = s;
else if (strcmp(".ARM.exidx", secname) == 0)
maps[ARM_SEC_CORE].unw_sec = s;
+ else if (strcmp(".ARM.exidx.exit.text", secname) == 0)
+ maps[ARM_SEC_EXIT].unw_sec = s;
+ else if (strcmp(".ARM.exidx.devexit.text", secname) == 0)
+ maps[ARM_SEC_DEVEXIT].unw_sec = s;
else if (strcmp(".init.text", secname) == 0)
maps[ARM_SEC_INIT].sec_text = s;
else if (strcmp(".devinit.text", secname) == 0)
maps[ARM_SEC_DEVINIT].sec_text = s;
else if (strcmp(".text", secname) == 0)
maps[ARM_SEC_CORE].sec_text = s;
+ else if (strcmp(".exit.text", secname) == 0)
+ maps[ARM_SEC_EXIT].sec_text = s;
+ else if (strcmp(".devexit.text", secname) == 0)
+ maps[ARM_SEC_DEVEXIT].sec_text = s;
}
#endif
return 0;
--
1.6.0.4
On 03/06/10 14:17 +0200, Carmody Phil.2 (EXT-Ixonos/Helsinki) wrote:
>
> The first two patches are simply preparation for the third, making it
> effectively trivial, even though it's the only one with a concrete
> change in behaviour.
>
> The origins of this patchset are the discovery that unwind and kmemleak
> don't always cooperate well with each other - any allocation within
> an exit or devexit function causes kmemleak to look up symbols that
> aren't in any unwind table. This of course means that all WARN_ONs and
> BUGs will suffer the same fate.
>
> It could certainly be said that with a typical system the linked list
> has grown too large to be practical as a container, and some improvements
> could be made in that direction in the future.
Catalin,
Have you had a chance to look at these yet? The linked-list efficiency
issue I mention in the final paragraph above is a no-brainer; I have a
1-line tweak that improves the real-world efficiency so much that on
average there are only 2 linked list operations rather than (on a 50+
module system) 70. However, that patch is orthogonal to the above set,
so I'll not mix the two.
Phil
On Thu, 2010-06-03 at 13:17 +0100, Phil Carmody wrote:
> Less to read.
>
> Signed-off-by: Phil Carmody <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
--
Catalin
On Thu, 2010-06-03 at 13:17 +0100, Phil Carmody wrote:
> The various sections are all dealt with similarly, so factor out
> that common behaviour.
>
> Signed-off-by: Phil Carmody <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
--
Catalin
On Thu, 2010-06-03 at 13:17 +0100, Phil Carmody wrote:
> Without these, exit functions cannot be stack-traced, so to speak.
> This implies that module unloads that perform allocations (don't
> laugh) will cause noisy warnings on the console when kmemleak is
> enabled, as it presumes that all code's call chains are traceable.
> Similarly, BUGs and WARN_ONs will give additional console spam.
>
> Signed-off-by: Phil Carmody <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
--
Catalin
On Thu, Jun 03, 2010 at 03:17:21PM +0300, Phil Carmody wrote:
> #ifdef CONFIG_ARM_UNWIND
...
> +#else
> +struct mod_arch_specific {
> +}
> +#endif
I assume no one tried building these with UNWIND disabled? linux-next
reports most ARM builds are broken by this.
Dropped all three patches out of my git tree until it can be fixed.
On Thu, 2010-06-24 at 10:05 +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 03, 2010 at 03:17:21PM +0300, Phil Carmody wrote:
> > #ifdef CONFIG_ARM_UNWIND
> ...
> > +#else
> > +struct mod_arch_specific {
> > +}
> > +#endif
>
> I assume no one tried building these with UNWIND disabled? linux-next
> reports most ARM builds are broken by this.
>
> Dropped all three patches out of my git tree until it can be fixed.
It looks like a patch for fixing the build was posted on the 16th of
June, though not sure whether it went to your patch system or not.
--
Catalin