Since these CRCs are really only 32-bit quantities, there's no need to
store them in 64-bit slots. Since, however, gcc doesn't allow
respective initializations, asm() constructs get used to create the CRC
tables (and its for that reason that the patch only makes x86-64 and
ia64 utilize that functionality, as I can't verify this doesn't break
in some subtle way elsewhere).
Observed size reduction (x86-64):
- 16k kernel resident size
- 2k module resident size
- 1M vmlinux image size
If the asm() construct could be extended to all architectures, and if
modpost would guarantee generation of CRCs for all exported, then
the __crc_* symbols wouldn't need to be declared weak anymore, could
hence remain local to the exporting object. By adding
-Wa,-strip-local-absolute to the second compilation step the __crc_*
symbols could even be prevented from making it into any object file at
all.
Signed-off-by: Jan Beulich <[email protected]>
---
arch/ia64/include/asm/module.h | 2 ++
arch/x86/include/asm/module.h | 1 +
include/linux/module.h | 41 ++++++++++++++++++++++++++---------------
include/linux/moduleparam.h | 10 +++++++++-
kernel/module.c | 26 +++++++++++++-------------
scripts/genksyms/genksyms.c | 16 ++++++++--------
6 files changed, 59 insertions(+), 37 deletions(-)
--- linux-2.6.31-rc1/arch/ia64/include/asm/module.h 2009-06-10 05:05:27.000000000 +0200
+++ 2.6.31-rc1-module-crc-int/arch/ia64/include/asm/module.h 2009-04-27 12:12:50.000000000 +0200
@@ -39,4 +39,6 @@ struct mod_arch_specific {
#define ARCH_SHF_SMALL SHF_IA_64_SHORT
+#define ARCH_ASM_KSYM_CRC data4
+
#endif /* _ASM_IA64_MODULE_H */
--- linux-2.6.31-rc1/arch/x86/include/asm/module.h 2008-12-25 00:26:37.000000000 +0100
+++ 2.6.31-rc1-module-crc-int/arch/x86/include/asm/module.h 2009-04-27 12:12:50.000000000 +0200
@@ -15,6 +15,7 @@ struct mod_arch_specific {};
#endif
#ifdef CONFIG_X86_64
+#define ARCH_ASM_KSYM_CRC .long
/* X86_64 does not define MODULE_PROC_FAMILY */
#elif defined CONFIG_M386
#define MODULE_PROC_FAMILY "386 "
--- linux-2.6.31-rc1/include/linux/module.h 2009-06-26 17:50:00.000000000 +0200
+++ 2.6.31-rc1-module-crc-int/include/linux/module.h 2009-04-27 12:15:26.000000000 +0200
@@ -19,8 +19,6 @@
#include <linux/tracepoint.h>
#include <asm/local.h>
-#include <asm/module.h>
-
/* Not Yet Implemented */
#define MODULE_SUPPORTED_DEVICE(name)
@@ -39,7 +37,7 @@ struct kernel_symbol
struct modversion_info
{
- unsigned long crc;
+ ksym_crc_t crc;
char name[MODULE_NAME_LEN];
};
@@ -175,17 +173,30 @@ void *__symbol_get_gpl(const char *symbo
#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x)))
#ifndef __GENKSYMS__
-#ifdef CONFIG_MODVERSIONS
+#define __KSTRTAB(sym) __kstrtab_##sym
+#ifndef CONFIG_MODVERSIONS
+#define __CRC_SYMBOL(sym, sec)
+#elif !defined(ARCH_ASM_KSYM_CRC) || defined(CC_PTR_CAST_INT)
/* Mark the CRC weak since genksyms apparently decides not to
* generate a checksums for some symbols */
#define __CRC_SYMBOL(sym, sec) \
extern void *__crc_##sym __attribute__((weak)); \
- static const unsigned long __kcrctab_##sym \
+ static const ksym_crc_t __kcrctab_##sym \
__used \
__attribute__((section("__kcrctab" sec), unused)) \
- = (unsigned long) &__crc_##sym;
+ = (ksym_crc_t) (unsigned long) &__crc_##sym;
#else
-#define __CRC_SYMBOL(sym, sec)
+#define __CRC_SYMBOL(sym, sec) \
+ extern const char __Kstrtab_##sym[] \
+ __asm__("__kstrtab_" #sym "\n\t" \
+ ".section __kcrctab" sec ",\"a\"\n\t" \
+ ".weak __crc_" #sym "\n\t" \
+ __stringify(ARCH_ASM_KSYM_CRC) " __crc_" #sym "\n\t" \
+ ".previous"); \
+ static const char __kstrtab_##sym[] \
+ __used __attribute__((__unused__));
+#undef __KSTRTAB
+#define __KSTRTAB(sym) __Kstrtab_##sym
#endif
/* For every exported symbol, place a struct in the __ksymtab section */
@@ -198,7 +209,7 @@ void *__symbol_get_gpl(const char *symbo
static const struct kernel_symbol __ksymtab_##sym \
__used \
__attribute__((section("__ksymtab" sec), unused)) \
- = { (unsigned long)&sym, __kstrtab_##sym }
+ = { (unsigned long)&sym, __KSTRTAB(sym) }
#define EXPORT_SYMBOL(sym) \
__EXPORT_SYMBOL(sym, "")
@@ -246,7 +257,7 @@ struct module
/* Exported symbols */
const struct kernel_symbol *syms;
- const unsigned long *crcs;
+ const ksym_crc_t *crcs;
unsigned int num_syms;
/* Kernel parameters. */
@@ -256,23 +267,23 @@ struct module
/* GPL-only exported symbols. */
unsigned int num_gpl_syms;
const struct kernel_symbol *gpl_syms;
- const unsigned long *gpl_crcs;
+ const ksym_crc_t *gpl_crcs;
#ifdef CONFIG_UNUSED_SYMBOLS
/* unused exported symbols. */
const struct kernel_symbol *unused_syms;
- const unsigned long *unused_crcs;
+ const ksym_crc_t *unused_crcs;
unsigned int num_unused_syms;
/* GPL-only, unused exported symbols. */
unsigned int num_unused_gpl_syms;
const struct kernel_symbol *unused_gpl_syms;
- const unsigned long *unused_gpl_crcs;
+ const ksym_crc_t *unused_gpl_crcs;
#endif
/* symbols that will be GPL-only in the near future. */
const struct kernel_symbol *gpl_future_syms;
- const unsigned long *gpl_future_crcs;
+ const ksym_crc_t *gpl_future_crcs;
unsigned int num_gpl_future_syms;
/* Exception table */
@@ -406,7 +417,7 @@ struct module *find_module(const char *n
struct symsearch {
const struct kernel_symbol *start, *stop;
- const unsigned long *crcs;
+ const ksym_crc_t *crcs;
enum {
NOT_GPL_ONLY,
GPL_ONLY,
@@ -418,7 +429,7 @@ struct symsearch {
/* Search for an exported symbol by name. */
const struct kernel_symbol *find_symbol(const char *name,
struct module **owner,
- const unsigned long **crc,
+ const ksym_crc_t **crc,
bool gplok,
bool warn);
--- linux-2.6.31-rc1/include/linux/moduleparam.h 2009-06-26 17:50:00.000000000 +0200
+++ 2.6.31-rc1-module-crc-int/include/linux/moduleparam.h 2009-04-27 12:12:50.000000000 +0200
@@ -5,6 +5,8 @@
#include <linux/stringify.h>
#include <linux/kernel.h>
+#include <asm/module.h>
+
/* You can override this manually, but generally this should match the
module name. */
#ifdef MODULE
@@ -13,8 +15,14 @@
#define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
#endif
+#ifdef ARCH_ASM_KSYM_CRC
+typedef unsigned int ksym_crc_t;
+#else
+typedef unsigned long ksym_crc_t;
+#endif
+
/* Chosen so that structs with an unsigned long line up. */
-#define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
+#define MAX_PARAM_PREFIX_LEN (64 - sizeof(ksym_crc_t))
#ifdef MODULE
#define ___module_cat(a,b) __mod_ ## a ## b
--- linux-2.6.31-rc1/kernel/module.c 2009-06-26 17:50:00.000000000 +0200
+++ 2.6.31-rc1-module-crc-int/kernel/module.c 2009-04-27 12:16:40.000000000 +0200
@@ -174,16 +174,16 @@ extern const struct kernel_symbol __star
extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
extern const struct kernel_symbol __start___ksymtab_gpl_future[];
extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
-extern const unsigned long __start___kcrctab[];
-extern const unsigned long __start___kcrctab_gpl[];
-extern const unsigned long __start___kcrctab_gpl_future[];
+extern const ksym_crc_t __start___kcrctab[];
+extern const ksym_crc_t __start___kcrctab_gpl[];
+extern const ksym_crc_t __start___kcrctab_gpl_future[];
#ifdef CONFIG_UNUSED_SYMBOLS
extern const struct kernel_symbol __start___ksymtab_unused[];
extern const struct kernel_symbol __stop___ksymtab_unused[];
extern const struct kernel_symbol __start___ksymtab_unused_gpl[];
extern const struct kernel_symbol __stop___ksymtab_unused_gpl[];
-extern const unsigned long __start___kcrctab_unused[];
-extern const unsigned long __start___kcrctab_unused_gpl[];
+extern const ksym_crc_t __start___kcrctab_unused[];
+extern const ksym_crc_t __start___kcrctab_unused_gpl[];
#endif
#ifndef CONFIG_MODVERSIONS
@@ -276,7 +276,7 @@ struct find_symbol_arg {
/* Output */
struct module *owner;
- const unsigned long *crc;
+ const ksym_crc_t *crc;
const struct kernel_symbol *sym;
};
@@ -326,7 +326,7 @@ static bool find_symbol_in_section(const
* (optional) module which owns it */
const struct kernel_symbol *find_symbol(const char *name,
struct module **owner,
- const unsigned long **crc,
+ const ksym_crc_t **crc,
bool gplok,
bool warn)
{
@@ -1024,7 +1024,7 @@ static int check_version(Elf_Shdr *sechd
unsigned int versindex,
const char *symname,
struct module *mod,
- const unsigned long *crc)
+ const ksym_crc_t *crc)
{
unsigned int i, num_versions;
struct modversion_info *versions;
@@ -1047,8 +1047,8 @@ static int check_version(Elf_Shdr *sechd
if (versions[i].crc == *crc)
return 1;
- DEBUGP("Found checksum %lX vs module %lX\n",
- *crc, versions[i].crc);
+ DEBUGP("Found checksum %X vs module %X\n",
+ (unsigned int)*crc, (unsigned int)versions[i].crc);
goto bad_version;
}
@@ -1066,7 +1066,7 @@ static inline int check_modstruct_versio
unsigned int versindex,
struct module *mod)
{
- const unsigned long *crc;
+ const ksym_crc_t *crc;
if (!find_symbol("module_layout", NULL, &crc, true, false))
BUG();
@@ -1088,7 +1088,7 @@ static inline int check_version(Elf_Shdr
unsigned int versindex,
const char *symname,
struct module *mod,
- const unsigned long *crc)
+ const ksym_crc_t *crc)
{
return 1;
}
@@ -1116,7 +1116,7 @@ static const struct kernel_symbol *resol
{
struct module *owner;
const struct kernel_symbol *sym;
- const unsigned long *crc;
+ const ksym_crc_t *crc;
sym = find_symbol(name, &owner, &crc,
!(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true);
--- linux-2.6.31-rc1/scripts/genksyms/genksyms.c 2009-03-24 00:12:14.000000000 +0100
+++ 2.6.31-rc1-module-crc-int/scripts/genksyms/genksyms.c 2009-04-27 12:12:50.000000000 +0200
@@ -119,19 +119,19 @@ static const unsigned int crctab32[] = {
0x2d02ef8dU
};
-static unsigned long partial_crc32_one(unsigned char c, unsigned long crc)
+static unsigned int partial_crc32_one(unsigned char c, unsigned int crc)
{
return crctab32[(crc ^ c) & 0xff] ^ (crc >> 8);
}
-static unsigned long partial_crc32(const char *s, unsigned long crc)
+static unsigned int partial_crc32(const char *s, unsigned int crc)
{
while (*s)
crc = partial_crc32_one(*s++, crc);
return crc;
}
-static unsigned long crc32(const char *s)
+static unsigned int crc32(const char *s)
{
return partial_crc32(s, 0xffffffff) ^ 0xffffffff;
}
@@ -149,7 +149,7 @@ static enum symbol_type map_to_ns(enum s
struct symbol *find_symbol(const char *name, enum symbol_type ns)
{
- unsigned long h = crc32(name) % HASH_BUCKETS;
+ unsigned int h = crc32(name) % HASH_BUCKETS;
struct symbol *sym;
for (sym = symtab[h]; sym; sym = sym->hash_next)
@@ -180,7 +180,7 @@ struct symbol *__add_symbol(const char *
struct string_list *defn, int is_extern,
int is_reference)
{
- unsigned long h = crc32(name) % HASH_BUCKETS;
+ unsigned int h = crc32(name) % HASH_BUCKETS;
struct symbol *sym;
enum symbol_status status = STATUS_UNCHANGED;
@@ -433,7 +433,7 @@ static void print_list(FILE * f, struct
}
}
-static unsigned long expand_and_crc_sym(struct symbol *sym, unsigned long crc)
+static unsigned int expand_and_crc_sym(struct symbol *sym, unsigned int crc)
{
struct string_list *list = sym->defn;
struct string_list **e, **b;
@@ -569,7 +569,7 @@ void export_symbol(const char *name)
if (!sym)
error_with_pos("export undefined symbol %s", name);
else {
- unsigned long crc;
+ unsigned int crc;
int has_changed = 0;
if (flag_dump_defs)
@@ -611,7 +611,7 @@ void export_symbol(const char *name)
fputs(">\n", debugfile);
/* Used as a linker script. */
- printf("%s__crc_%s = 0x%08lx ;\n", mod_prefix, name, crc);
+ printf("%s__crc_%s = %#08x ;\n", mod_prefix, name, crc);
}
}
* Jan Beulich <[email protected]> wrote:
> Since these CRCs are really only 32-bit quantities, there's no
> need to store them in 64-bit slots. Since, however, gcc doesn't
> allow respective initializations, asm() constructs get used to
> create the CRC tables (and its for that reason that the patch only
> makes x86-64 and ia64 utilize that functionality, as I can't
> verify this doesn't break in some subtle way elsewhere).
>
> Observed size reduction (x86-64):
> - 16k kernel resident size
> - 2k module resident size
> - 1M vmlinux image size
>
> If the asm() construct could be extended to all architectures, and
> if modpost would guarantee generation of CRCs for all exported,
> then the __crc_* symbols wouldn't need to be declared weak
> anymore, could hence remain local to the exporting object. By
> adding -Wa,-strip-local-absolute to the second compilation step
> the __crc_* symbols could even be prevented from making it into
> any object file at all.
>
> Signed-off-by: Jan Beulich <[email protected]>
>
> ---
> arch/ia64/include/asm/module.h | 2 ++
> arch/x86/include/asm/module.h | 1 +
> include/linux/module.h | 41 ++++++++++++++++++++++++++---------------
> include/linux/moduleparam.h | 10 +++++++++-
> kernel/module.c | 26 +++++++++++++-------------
> scripts/genksyms/genksyms.c | 16 ++++++++--------
> 6 files changed, 59 insertions(+), 37 deletions(-)
No objections to the (minor) x86 bits.
Ingo
On Tue, 30 Jun 2009 09:21:52 pm Jan Beulich wrote:
> Since these CRCs are really only 32-bit quantities, there's no need to
> store them in 64-bit slots. Since, however, gcc doesn't allow
> respective initializations, asm() constructs get used to create the CRC
> tables (and its for that reason that the patch only makes x86-64 and
> ia64 utilize that functionality, as I can't verify this doesn't break
> in some subtle way elsewhere).
Hmm, can we change the build system to just link this in as a normal table,
rather than use linker tricks?
Then genksyms would just spit out a C file we could compile and link into final
vmlinux.
Thanks,
Rusty.
>>> Rusty Russell <[email protected]> 01.07.09 07:00 >>>
>On Tue, 30 Jun 2009 09:21:52 pm Jan Beulich wrote:
>> Since these CRCs are really only 32-bit quantities, there's no need to
>> store them in 64-bit slots. Since, however, gcc doesn't allow
>> respective initializations, asm() constructs get used to create the CRC
>> tables (and its for that reason that the patch only makes x86-64 and
>> ia64 utilize that functionality, as I can't verify this doesn't break
>> in some subtle way elsewhere).
>
>Hmm, can we change the build system to just link this in as a normal table,
>rather than use linker tricks?
>
>Then genksyms would just spit out a C file we could compile and link into final
>vmlinux.
That's going to be non-trivial, because of the ordering that's required to be
identical between the symbol and the CRC tables. Basically that would mean
that genksyms would need to look at either the assembly output or the
object file, which would further slow down the build process. Compared to
that the asm() solution in the patch sent seems more strait forward (despite
it possibly looking convoluted at the first glance).
Jan
Jan Beulich napsal(a):
> Since these CRCs are really only 32-bit quantities, there's no need to
> store them in 64-bit slots. Since, however, gcc doesn't allow
> respective initializations, asm() constructs get used to create the CRC
> tables (and its for that reason that the patch only makes x86-64 and
> ia64 utilize that functionality, as I can't verify this doesn't break
> in some subtle way elsewhere).
>
...
> struct modversion_info
> {
> - unsigned long crc;
> + ksym_crc_t crc;
> char name[MODULE_NAME_LEN];
> };
This change breaks module-init-tools:
Before:
$ /sbin/modprobe --dump-modversions _build/drivers/usb/core/usbcore.ko
| head
0xb49b735a module_layout
0xdb7e6a70 bus_register
0xe3a25bcc device_remove_file
0x1fedf0f4 __request_region
0x37429b3b device_remove_bin_file
0x27ac237c cdev_del
0xf3be63b3 per_cpu__current_task
0x26b76a45 kmalloc_caches
0x5a34a45c __kmalloc
0x1c820358 cdev_init
After:
$ /sbin/modprobe --dump-modversions
_build-crc-int/drivers/usb/core/usbcore.ko | head
0x75646f6d91ea7b5c le_layout
0x5f7375623e215f43 register
0x69766564848c67e2 ce_remove_file
0x65725f5f1fedf0f4 quest_region
0x69766564a0c29c25 ce_remove_bin_file
0x766564633ce98bc2 _del
0x5f72657012c9615b cpu__current_task
0x6c616d6b6ea6c464 loc_caches
0x6d6b5f5f5a34a45c alloc
0x766564631e5d885b _init
It also breaks the newly added depmod -E option (check symbol versions),
which also reads the struct modversion_info array (*). Is it possible
name the section differently (__versions2?) on those architectures where
the size changes, so that it is possible to fix m-i-t in a
backwards-compatible manner?
Michal
(*) dump_modvers() and load_symbols() in
http://git.kernel.org/?p=utils/kernel/module-init-tools/module-init-tools.git;a=blob;f=elfops_core.c;hb=HEAD
>>> Michal Marek <[email protected]> 03.07.09 22:16 >>>
>Jan Beulich napsal(a):
>> Since these CRCs are really only 32-bit quantities, there's no need to
>> store them in 64-bit slots. Since, however, gcc doesn't allow
>> respective initializations, asm() constructs get used to create the CRC
>> tables (and its for that reason that the patch only makes x86-64 and
>> ia64 utilize that functionality, as I can't verify this doesn't break
>> in some subtle way elsewhere).
>>
>...
>> struct modversion_info
>> {
>> - unsigned long crc;
>> + ksym_crc_t crc;
>> char name[MODULE_NAME_LEN];
>> };
>
>This change breaks module-init-tools:
>Before:
>$ /sbin/modprobe --dump-modversions _build/drivers/usb/core/usbcore.ko
>| head
>0xb49b735a module_layout
>0xdb7e6a70 bus_register
>...
>After:
>$ /sbin/modprobe --dump-modversions
>_build-crc-int/drivers/usb/core/usbcore.ko | head
>0x75646f6d91ea7b5c le_layout
>0x5f7375623e215f43 register
>...
>It also breaks the newly added depmod -E option (check symbol versions),
>which also reads the struct modversion_info array (*). Is it possible
>name the section differently (__versions2?) on those architectures where
>the size changes, so that it is possible to fix m-i-t in a
>backwards-compatible manner?
First of all I'd view it as a design bug if user mode code assumptions prevent
changes to the kernel.
But taking this as an uncorrectable fact, I'd think that renaming the section
would certainly be an option (though I'm unsure whether that would have
other consequences - Rusty?), however I could also imagine other means to
communicate to user land the width of a CRC value (e.g. adding an absolute
symbol during the .ko linking stage).
Jan
On Mon, 6 Jul 2009 05:12:51 pm Jan Beulich wrote:
> >Jan Beulich napsal(a):
> >> Since these CRCs are really only 32-bit quantities, there's no need to
> >> store them in 64-bit slots. Since, however, gcc doesn't allow
> >> respective initializations, asm() constructs get used to create the CRC
> >> tables (and its for that reason that the patch only makes x86-64 and
> >> ia64 utilize that functionality, as I can't verify this doesn't break
> >> in some subtle way elsewhere).
> >
> >...
> >
> >> struct modversion_info
> >> {
> >> - unsigned long crc;
> >> + ksym_crc_t crc;
> >> char name[MODULE_NAME_LEN];
> >> };
> >
> >This change breaks module-init-tools:
> >Before:
> >$ /sbin/modprobe --dump-modversions _build/drivers/usb/core/usbcore.ko
> >
> >| head
> >
> >0xb49b735a module_layout
> >0xdb7e6a70 bus_register
> >...
> >After:
> >$ /sbin/modprobe --dump-modversions
> >_build-crc-int/drivers/usb/core/usbcore.ko | head
> >0x75646f6d91ea7b5c le_layout
> >0x5f7375623e215f43 register
> >...
> >It also breaks the newly added depmod -E option (check symbol versions),
> >which also reads the struct modversion_info array (*). Is it possible
> >name the section differently (__versions2?) on those architectures where
> >the size changes, so that it is possible to fix m-i-t in a
> >backwards-compatible manner?
>
> First of all I'd view it as a design bug if user mode code assumptions
> prevent changes to the kernel.
Yes, but unfortunately it happens. We do it much less than we used to, but
there are limits.
> But taking this as an uncorrectable fact, I'd think that renaming the
> section would certainly be an option (though I'm unsure whether that would
> have other consequences - Rusty?), however I could also imagine other means
> to communicate to user land the width of a CRC value (e.g. adding an
> absolute symbol during the .ko linking stage).
No, just break it once. And I still like the idea that we should do something
more radical if we're going to break this anyway, rather than these nasty asm
hacks.
Thanks,
Rusty.
>>> Rusty Russell <[email protected]> 09.07.09 13:14 >>>
>On Mon, 6 Jul 2009 05:12:51 pm Jan Beulich wrote:
>> >Jan Beulich napsal(a):
>> >> Since these CRCs are really only 32-bit quantities, there's no need to
>> >> store them in 64-bit slots. Since, however, gcc doesn't allow
>> >> respective initializations, asm() constructs get used to create the CRC
>> >> tables (and its for that reason that the patch only makes x86-64 and
>> >> ia64 utilize that functionality, as I can't verify this doesn't break
>> >> in some subtle way elsewhere).
>> >
>> >...
>> >
>> >> struct modversion_info
>> >> {
>> >> - unsigned long crc;
>> >> + ksym_crc_t crc;
>> >> char name[MODULE_NAME_LEN];
>> >> };
>> >
>> >This change breaks module-init-tools:
>> >Before:
>> >$ /sbin/modprobe --dump-modversions _build/drivers/usb/core/usbcore.ko
>> >
>> >| head
>> >
>> >0xb49b735a module_layout
>> >0xdb7e6a70 bus_register
>> >...
>> >After:
>> >$ /sbin/modprobe --dump-modversions
>> >_build-crc-int/drivers/usb/core/usbcore.ko | head
>> >0x75646f6d91ea7b5c le_layout
>> >0x5f7375623e215f43 register
>> >...
>> >It also breaks the newly added depmod -E option (check symbol versions),
>> >which also reads the struct modversion_info array (*). Is it possible
>> >name the section differently (__versions2?) on those architectures where
>> >the size changes, so that it is possible to fix m-i-t in a
>> >backwards-compatible manner?
>>
>> First of all I'd view it as a design bug if user mode code assumptions
>> prevent changes to the kernel.
>
>Yes, but unfortunately it happens. We do it much less than we used to, but
>there are limits.
>
>> But taking this as an uncorrectable fact, I'd think that renaming the
>> section would certainly be an option (though I'm unsure whether that would
>> have other consequences - Rusty?), however I could also imagine other means
>> to communicate to user land the width of a CRC value (e.g. adding an
>> absolute symbol during the .ko linking stage).
>
>No, just break it once. And I still like the idea that we should do something
>more radical if we're going to break this anyway, rather than these nasty asm
>hacks.
Actually I meanwhile think that module-init-tools can easily detect the changed
layout without any further kernel side adjustments: Since it is known that a
CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
value has more than 32 significant bits should suffice: If so, the new layout
is being used (with the symbol name starting at offset 4), else the old one is
in effect (name at offset 8). This ought to be a pretty trivial change to that
code.
Jan
Jan Beulich wrote:
>
> Actually I meanwhile think that module-init-tools can easily detect the changed
> layout without any further kernel side adjustments: Since it is known that a
> CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
> value has more than 32 significant bits should suffice: If so, the new layout
> is being used (with the symbol name starting at offset 4), else the old one is
> in effect (name at offset 8). This ought to be a pretty trivial change to that
> code.
>
If we're changing module-init-tools, it seems changing the section name
would be cleaner anyway.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Fri, 2009-07-10 at 08:23 +0100, Jan Beulich wrote:
> Actually I meanwhile think that module-init-tools can easily detect the changed
> layout without any further kernel side adjustments: Since it is known that a
> CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
> value has more than 32 significant bits should suffice: If so, the new layout
> is being used (with the symbol name starting at offset 4), else the old one is
> in effect (name at offset 8). This ought to be a pretty trivial change to that
> code.
Agreed. That can be done. I won't get chance to do that until Sunday
though because I'm about to be on a plane back from Tokyo to Boston and
I expect a day of living hell with jetlag/email craziness.
Jon.
Jan Beulich napsal(a):
> Actually I meanwhile think that module-init-tools can easily detect the changed
> layout without any further kernel side adjustments: Since it is known that a
> CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
> value has more than 32 significant bits should suffice: If so, the new layout
> is being used (with the symbol name starting at offset 4), else the old one is
> in effect (name at offset 8). This ought to be a pretty trivial change to that
> code.
But old module-init-tools will continue reading garbage in this case.
Michal
>>> Michal Marek <[email protected]> 13.07.09 10:11 >>>
>Jan Beulich napsal(a):
>> Actually I meanwhile think that module-init-tools can easily detect the changed
>> layout without any further kernel side adjustments: Since it is known that a
>> CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
>> value has more than 32 significant bits should suffice: If so, the new layout
>> is being used (with the symbol name starting at offset 4), else the old one is
>> in effect (name at offset 8). This ought to be a pretty trivial change to that
>> code.
>
>But old module-init-tools will continue reading garbage in this case.
And would you think returning nothing is significantly better than returning
garbage?
Jan
On Mon, 2009-07-13 at 09:44 +0100, Jan Beulich wrote:
> >>> Michal Marek <[email protected]> 13.07.09 10:11 >>>
> >Jan Beulich napsal(a):
> >> Actually I meanwhile think that module-init-tools can easily detect the changed
> >> layout without any further kernel side adjustments: Since it is known that a
> >> CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
> >> value has more than 32 significant bits should suffice: If so, the new layout
> >> is being used (with the symbol name starting at offset 4), else the old one is
> >> in effect (name at offset 8). This ought to be a pretty trivial change to that
> >> code.
> >
> >But old module-init-tools will continue reading garbage in this case.
Most of the distros can fix that with a dependency on the kernel package
in the absolute worst case, not that I love that idea, but it happens. I
assume for now we are going with detecting the two possibilities because
it doesn't really hurt in any case to have this support.
Jon.
On Jul 14, Jon Masters <[email protected]> wrote:
> Most of the distros can fix that with a dependency on the kernel package
> in the absolute worst case, not that I love that idea, but it happens. I
FYI, Debian cannot do this.
--
ciao,
Marco
Jon Masters wrote:
> On Mon, 2009-07-13 at 09:44 +0100, Jan Beulich wrote:
>>>>> Michal Marek <[email protected]> 13.07.09 10:11 >>>
>>> Jan Beulich napsal(a):
>>>> Actually I meanwhile think that module-init-tools can easily detect the changed
>>>> layout without any further kernel side adjustments: Since it is known that a
>>>> CRC always is a 32-bit value, simply checking whether the so-far-used 64-bit
>>>> value has more than 32 significant bits should suffice: If so, the new layout
>>>> is being used (with the symbol name starting at offset 4), else the old one is
>>>> in effect (name at offset 8). This ought to be a pretty trivial change to that
>>>> code.
>>> But old module-init-tools will continue reading garbage in this case.
>
> Most of the distros can fix that with a dependency on the kernel package
> in the absolute worst case, not that I love that idea, but it happens. I
> assume for now we are going with detecting the two possibilities because
> it doesn't really hurt in any case to have this support.
>
It would seem to me that reading garbage is worse than reading nothing,
unless I'm missing something fundamental.
-hpa