jump label v8: sort jump table at build-time
in [Kernel]
|
From: David Miller on 19 May 2010 18:40 From: Jason Baron <jbaron(a)redhat.com> Date: Wed, 19 May 2010 12:22:17 -0400 > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 2092361..fe6f8be 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -17,6 +17,8 @@ > #include "modpost.h" > #include "../../include/generated/autoconf.h" > #include "../../include/linux/license.h" > +#include <linux/types.h> > +#include "jump_entry.h" > This breaks the build on non-jump_label-supporting architectures because only they will have the jump_entry.h header file. I'm really getting tired trying to test your changes and every single time as I go through doing very basic build testing I always hit one patch that assumes jump label support exists, or assumes some x86'ism. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jason Baron on 20 May 2010 16:20 On Wed, May 19, 2010 at 03:36:34PM -0700, David Miller wrote: > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index 2092361..fe6f8be 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -17,6 +17,8 @@ > > #include "modpost.h" > > #include "../../include/generated/autoconf.h" > > #include "../../include/linux/license.h" > > +#include <linux/types.h> > > +#include "jump_entry.h" > > > > This breaks the build on non-jump_label-supporting architectures > because only they will have the jump_entry.h header file. > > I'm really getting tired trying to test your changes and every single > time as I go through doing very basic build testing I always hit one > patch that assumes jump label support exists, or assumes some x86'ism. sorry about that. I mis-tested it. anyways here's the interdiff to this patch to fix. btw, how do the sparc bits look? thanks, -Jason diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index fe6f8be..cc87012 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -18,7 +18,9 @@ #include "../../include/generated/autoconf.h" #include "../../include/linux/license.h" #include <linux/types.h> -#include "jump_entry.h" +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL + #include "jump_entry.h" +#endif /* Some toolchains use a `_' prefix for all user symbols. */ #ifdef CONFIG_SYMBOL_PREFIX @@ -377,6 +379,8 @@ void release_file(void *file, unsigned long size) munmap(file, size); } +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL + static void swap_jump_label_entries(struct jump_entry *previous, struct jump_entry *next) { struct jump_entry tmp; @@ -421,6 +425,8 @@ static void sort_jump_label_table(struct elf_info *info, Elf_Ehdr *hdr) } while (swapped == 1); } +#endif + static int parse_elf(struct elf_info *info, const char *filename) { unsigned int i; @@ -539,8 +545,10 @@ static int parse_elf(struct elf_info *info, const char *filename) sym->st_size = TO_NATIVE(sym->st_size); } +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL if (enable_jump_label) sort_jump_label_table(info, hdr); +#endif return 1; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mathieu Desnoyers on 20 May 2010 17:40 * Jason Baron (jbaron(a)redhat.com) wrote: > On Wed, May 19, 2010 at 03:36:34PM -0700, David Miller wrote: > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > > index 2092361..fe6f8be 100644 > > > --- a/scripts/mod/modpost.c > > > +++ b/scripts/mod/modpost.c > > > @@ -17,6 +17,8 @@ > > > #include "modpost.h" > > > #include "../../include/generated/autoconf.h" > > > #include "../../include/linux/license.h" > > > +#include <linux/types.h> > > > +#include "jump_entry.h" > > > > > > > This breaks the build on non-jump_label-supporting architectures > > because only they will have the jump_entry.h header file. > > > > I'm really getting tired trying to test your changes and every single > > time as I go through doing very basic build testing I always hit one > > patch that assumes jump label support exists, or assumes some x86'ism. > > sorry about that. I mis-tested it. anyways here's the interdiff to this > patch to fix. btw, how do the sparc bits look? > > thanks, > > -Jason > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index fe6f8be..cc87012 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -18,7 +18,9 @@ > #include "../../include/generated/autoconf.h" > #include "../../include/linux/license.h" > #include <linux/types.h> > -#include "jump_entry.h" includes with "" is a bit unexpected in kernel code. Any reason for this ? Is jump_entry.h shipped in scripts/mod/ ? > +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL ifdefs everywhere cripples the code and make it unreadable. You might want to try to find another way to deal with the problem. E.g. create linux/jump_entry.h which contains #ifdef CONFIG_HAVE_ARCH_JUMP_LABEL #include <asm/jump_entry.h> #else define some empty static inlines, and use them in the .c files. e.g. static inline void sort_jump_label_table(struct elf_info *info, Elf_Ehdr *hdr) { } #endif > + #include "jump_entry.h" > +#endif > > /* Some toolchains use a `_' prefix for all user symbols. */ > #ifdef CONFIG_SYMBOL_PREFIX > @@ -377,6 +379,8 @@ void release_file(void *file, unsigned long size) > munmap(file, size); > } > > +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL > + > static void swap_jump_label_entries(struct jump_entry *previous, struct jump_entry *next) > { > struct jump_entry tmp; > @@ -421,6 +425,8 @@ static void sort_jump_label_table(struct elf_info *info, Elf_Ehdr *hdr) > } while (swapped == 1); > } > > +#endif > + > static int parse_elf(struct elf_info *info, const char *filename) > { > unsigned int i; > @@ -539,8 +545,10 @@ static int parse_elf(struct elf_info *info, const char *filename) > sym->st_size = TO_NATIVE(sym->st_size); > } > > +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL > if (enable_jump_label) > sort_jump_label_table(info, hdr); > +#endif And remove this ifdef. Thanks, Mathieu > > return 1; > } -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 20 May 2010 18:30 From: Jason Baron <jbaron(a)redhat.com> Date: Thu, 20 May 2010 16:17:29 -0400 > btw, how do the sparc bits look? I have to send you a fix because I use the "call" opcode instead of the one for "ba". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jason Baron on 24 May 2010 16:20 On Thu, May 20, 2010 at 05:32:16PM -0400, Mathieu Desnoyers wrote: > * Jason Baron (jbaron(a)redhat.com) wrote: > > On Wed, May 19, 2010 at 03:36:34PM -0700, David Miller wrote: > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > > > index 2092361..fe6f8be 100644 > > > > --- a/scripts/mod/modpost.c > > > > +++ b/scripts/mod/modpost.c > > > > @@ -17,6 +17,8 @@ > > > > #include "modpost.h" > > > > #include "../../include/generated/autoconf.h" > > > > #include "../../include/linux/license.h" > > > > +#include <linux/types.h> > > > > +#include "jump_entry.h" > > > > > > > > > > This breaks the build on non-jump_label-supporting architectures > > > because only they will have the jump_entry.h header file. > > > > > > I'm really getting tired trying to test your changes and every single > > > time as I go through doing very basic build testing I always hit one > > > patch that assumes jump label support exists, or assumes some x86'ism. > > > > sorry about that. I mis-tested it. anyways here's the interdiff to this > > patch to fix. btw, how do the sparc bits look? > > > > thanks, > > > > -Jason > > > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index fe6f8be..cc87012 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -18,7 +18,9 @@ > > #include "../../include/generated/autoconf.h" > > #include "../../include/linux/license.h" > > #include <linux/types.h> > > -#include "jump_entry.h" > > includes with "" is a bit unexpected in kernel code. Any reason for this ? > Is jump_entry.h shipped in scripts/mod/ ? > The jump_entry.h is per arch found in: arch/<arch>/include/asm/jump_entry. Its being found in modpost.c, via: HOST_EXTRACFLAGS += -iquote "$(srctree)/arch/$(hdr-arch)/include/asm" in scripts/mod/Makefile I used the quotes so that its less likely to conflict with any other include. > > +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL > > ifdefs everywhere cripples the code and make it unreadable. You might > want to try to find another way to deal with the problem. E.g. > > create linux/jump_entry.h > which contains > > #ifdef CONFIG_HAVE_ARCH_JUMP_LABEL > #include <asm/jump_entry.h> > #else > > define some empty static inlines, and use them in the .c files. > > e.g. > > static inline void sort_jump_label_table(struct elf_info *info, Elf_Ehdr *hdr) > { > } > > #endif > right. i have that defined already as include/linux/jump_label.h. Which is the include file that kernel code uses. However, I can't use it in the modpost.c code, since there are a number of kernel specific references. (I probably could simplify things a bit using #ifdef KERNEL). > > + #include "jump_entry.h" > > +#endif > > > > /* Some toolchains use a `_' prefix for all user symbols. */ > > #ifdef CONFIG_SYMBOL_PREFIX > > @@ -377,6 +379,8 @@ void release_file(void *file, unsigned long size) > > munmap(file, size); > > } > > > > +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL > > + > > static void swap_jump_label_entries(struct jump_entry *previous, struct jump_entry *next) > > { > > struct jump_entry tmp; > > @@ -421,6 +425,8 @@ static void sort_jump_label_table(struct elf_info *info, Elf_Ehdr *hdr) > > } while (swapped == 1); > > } > > > > +#endif > > + > > static int parse_elf(struct elf_info *info, const char *filename) > > { > > unsigned int i; > > @@ -539,8 +545,10 @@ static int parse_elf(struct elf_info *info, const char *filename) > > sym->st_size = TO_NATIVE(sym->st_size); > > } > > > > +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL > > if (enable_jump_label) > > sort_jump_label_table(info, hdr); > > +#endif > > And remove this ifdef. > > Thanks, > > Mathieu > agreed. I'll clean this up in the next iteration. thanks, -jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Pages: 1 Prev: drivers/media/video/uvc: Use kmemdup Next: msm_serial: fix serial on trout |