* Early crash (was: Re: module: show version information for built-in modules in sysfs) @ 2011-02-01 20:33 Geert Uytterhoeven 2011-02-01 21:09 ` Dmitry Torokhov 0 siblings, 1 reply; 26+ messages in thread From: Geert Uytterhoeven @ 2011-02-01 20:33 UTC (permalink / raw) To: Dmitry Torokhov, Rusty Russell; +Cc: linux-kernel, Linux/m68k On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> wrote: > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de > module: show version information for built-in modules in sysfs > > Currently only drivers that are built as modules have their versions > shown in /sys/module/<module_name>/version, but this information might > also be useful for built-in drivers as well. This especially important > for drivers that do not define any parameters - such drivers, if > built-in, are completely invisible from userspace. > > This patch changes MODULE_VERSION() macro so that in case when we are > compiling built-in module, version information is stored in a separate > section. Kernel then uses this data to create 'version' sysfs attribute > in the same fashion it creates attributes for module parameters. This commit causes the crash below on m68k (ARAnyM). Reverting this commit and its dependency 3b90a5b292321b2acac3921f77046ae195aef53f ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") makes it boot again. NET: Registered protocol family 16 Unable to handle kernel NULL pointer dereference at virtual address 0000002b Oops: 00000000 Modules linked in: PC: [<00141164>] kset_find_obj_hinted+0x5a/0xb6 SR: 2300 SP: 00c09ec8 a2: 00c07900 d0: 00000078 d1: 00c2ee90 d2: 00000000 d3: 00000000 d4: 00000000 d5: 00000000 a0: 00c2f700 a1: 00c2f701 Process swapper (pid: 1, task=00c07900) Frame format=7 eff addr=0000002b ssw=0525 faddr=0000002b wb 1 stat/addr/data: 0000 00000000 00000000 wb 2 stat/addr/data: 0000 00000000 00000000 wb 3 stat/addr/data: 0000 0000002b 00000000 push data: 00000000 00000000 00000000 00000000 Stack from 00c09f30: 0000002b 002d853a 00310dac 0031214a 002dbff8 001411d0 00c2ee90 0000002b 00000000 00310dc2 00c2ee90 0000002b 00c2ee50 00000000 002d853a 00310eb8 0000002b 00000000 00000000 003291a8 00310e6c 00c1d8b8 002e6872 00170e00 00c1d8b0 002e6872 003031f8 00000000 00000000 00303038 0031bec2 0031214a 0031bf40 00303038 00000000 003291a4 000020f8 00000000 00000000 00000000 00000000 00000000 00000000 003291a8 00002008 0031214a 0030baee 00310e6c Call Trace: [<00310dac>] locate_module_kobject+0x0/0xc0 [<0031214a>] __alloc_bootmem+0x0/0x1a [<001411d0>] kset_find_obj+0x10/0x18 [<00310dc2>] locate_module_kobject+0x16/0xc0 [<00310eb8>] param_sysfs_init+0x4c/0x1be [<00310e6c>] param_sysfs_init+0x0/0x1be [<00170e00>] vtconsole_init_device+0x3c/0x90 [<0031bec2>] vtconsole_class_init+0x0/0xda [<0031214a>] __alloc_bootmem+0x0/0x1a [<0031bf40>] vtconsole_class_init+0x7e/0xda [<000020f8>] do_one_initcall+0xf0/0x186 [<00002008>] do_one_initcall+0x0/0x186 [<0031214a>] __alloc_bootmem+0x0/0x1a [<0030baee>] kernel_init+0x96/0x136 [<00310e6c>] param_sysfs_init+0x0/0x1be [<0030ba58>] kernel_init+0x0/0x136 [<0002794e>] printk+0x0/0x1a [<00002b52>] kernel_thread+0x3a/0x4e Code: b288 672c 2053 4a88 6716 2248 2c4c 1019 <b01e> 6606 4a00 66f6 6002 9026 4a00 6722 47ed fffc 2a6b 0004 41eb 0004 b288 66d4 Disabling lock debugging due to kernel taint Kernel panic - not syncing: Attempted to kill init! Call Trace: [<0002745c>] panic+0x5a/0x1ba [<00028bd4>] exit_mm+0x0/0x102 [<0011dfd6>] exit_sem+0x0/0x162 [<0002f5f2>] exit_ptrace+0x0/0x106 [<0002a274>] do_exit+0x6cc/0x6ce [<0002794e>] printk+0x0/0x1a [<000031de>] die_if_kernel+0x4e/0x52 [<00006e8c>] send_fault_sig+0xd4/0x11e [<00006f80>] do_page_fault+0xaa/0x1c2 [<000038b6>] buserr_c+0x192/0x6d8 [<00100100>] mnt_xdr_dec_mountres+0xa2/0xb4 [<00002542>] buserr+0x1e/0x24 [<00310dac>] locate_module_kobject+0x0/0xc0 [<0031214a>] __alloc_bootmem+0x0/0x1a [<001411d0>] kset_find_obj+0x10/0x18 [<00310dc2>] locate_module_kobject+0x16/0xc0 [<00310eb8>] param_sysfs_init+0x4c/0x1be [<00310e6c>] param_sysfs_init+0x0/0x1be [<00170e00>] vtconsole_init_device+0x3c/0x90 [<0031bec2>] vtconsole_class_init+0x0/0xda [<0031214a>] __alloc_bootmem+0x0/0x1a [<0031bf40>] vtconsole_class_init+0x7e/0xda [<000020f8>] do_one_initcall+0xf0/0x186 [<00002008>] do_one_initcall+0x0/0x186 [<0031214a>] __alloc_bootmem+0x0/0x1a [<0030baee>] kernel_init+0x96/0x136 [<00310e6c>] param_sysfs_init+0x0/0x1be [<0030ba58>] kernel_init+0x0/0x136 [<0002794e>] printk+0x0/0x1a [<00002b52>] kernel_thread+0x3a/0x4e Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-01 20:33 Early crash (was: Re: module: show version information for built-in modules in sysfs) Geert Uytterhoeven @ 2011-02-01 21:09 ` Dmitry Torokhov 0 siblings, 0 replies; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-01 21:09 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Rusty Russell, linux-kernel, Linux/m68k On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote: > On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List > <linux-kernel@vger.kernel.org> wrote: > > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de > > > module: show version information for built-in modules in sysfs > > > > Currently only drivers that are built as modules have their versions > > shown in /sys/module/<module_name>/version, but this information might > > also be useful for built-in drivers as well. This especially important > > for drivers that do not define any parameters - such drivers, if > > built-in, are completely invisible from userspace. > > > > This patch changes MODULE_VERSION() macro so that in case when we are > > compiling built-in module, version information is stored in a separate > > section. Kernel then uses this data to create 'version' sysfs attribute > > in the same fashion it creates attributes for module parameters. > > This commit causes the crash below on m68k (ARAnyM). > Reverting this commit and its dependency > 3b90a5b292321b2acac3921f77046ae195aef53f > ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") > makes it boot again. > Hi Geert, Does the follwing help by any chance? >From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001 diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S index ef33213..47e15eb 100644 --- a/arch/m68knommu/kernel/vmlinux.lds.S +++ b/arch/m68knommu/kernel/vmlinux.lds.S @@ -141,6 +141,12 @@ SECTIONS { *(__param) __stop___param = .; + /* Built-in module versions */ + . = ALIGN(4) ; + __start___modver = .; + *(__modver) + __stop___modver = .; + . = ALIGN(4) ; _etext = . ; } > TEXT Thanks, Dmitry ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) @ 2011-02-01 21:09 ` Dmitry Torokhov 0 siblings, 0 replies; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-01 21:09 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Rusty Russell, linux-kernel, Linux/m68k On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote: > On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List > <linux-kernel@vger.kernel.org> wrote: > > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de > > > module: show version information for built-in modules in sysfs > > > > Currently only drivers that are built as modules have their versions > > shown in /sys/module/<module_name>/version, but this information might > > also be useful for built-in drivers as well. This especially important > > for drivers that do not define any parameters - such drivers, if > > built-in, are completely invisible from userspace. > > > > This patch changes MODULE_VERSION() macro so that in case when we are > > compiling built-in module, version information is stored in a separate > > section. Kernel then uses this data to create 'version' sysfs attribute > > in the same fashion it creates attributes for module parameters. > > This commit causes the crash below on m68k (ARAnyM). > Reverting this commit and its dependency > 3b90a5b292321b2acac3921f77046ae195aef53f > ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") > makes it boot again. > Hi Geert, Does the follwing help by any chance? From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001 diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S index ef33213..47e15eb 100644 --- a/arch/m68knommu/kernel/vmlinux.lds.S +++ b/arch/m68knommu/kernel/vmlinux.lds.S @@ -141,6 +141,12 @@ SECTIONS { *(__param) __stop___param = .; + /* Built-in module versions */ + . = ALIGN(4) ; + __start___modver = .; + *(__modver) + __stop___modver = .; + . = ALIGN(4) ; _etext = . ; } > TEXT Thanks, Dmitry ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-01 21:09 ` Dmitry Torokhov (?) @ 2011-02-01 22:03 ` Geert Uytterhoeven 2011-02-01 22:26 ` Dmitry Torokhov -1 siblings, 1 reply; 26+ messages in thread From: Geert Uytterhoeven @ 2011-02-01 22:03 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Rusty Russell, linux-kernel, Linux/m68k On Tue, Feb 1, 2011 at 22:09, Dmitry Torokhov <dtor@vmware.com> wrote: > On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote: >> On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List >> <linux-kernel@vger.kernel.org> wrote: >> > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de >> >> > module: show version information for built-in modules in sysfs >> > >> > Currently only drivers that are built as modules have their versions >> > shown in /sys/module/<module_name>/version, but this information might >> > also be useful for built-in drivers as well. This especially important >> > for drivers that do not define any parameters - such drivers, if >> > built-in, are completely invisible from userspace. >> > >> > This patch changes MODULE_VERSION() macro so that in case when we are >> > compiling built-in module, version information is stored in a separate >> > section. Kernel then uses this data to create 'version' sysfs attribute >> > in the same fashion it creates attributes for module parameters. >> >> This commit causes the crash below on m68k (ARAnyM). >> Reverting this commit and its dependency >> 3b90a5b292321b2acac3921f77046ae195aef53f >> ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") >> makes it boot again. >> > > Hi Geert, > > Does the follwing help by any chance? > > From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001 > diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S > index ef33213..47e15eb 100644 > --- a/arch/m68knommu/kernel/vmlinux.lds.S > +++ b/arch/m68knommu/kernel/vmlinux.lds.S The crash happened on m68k with MMU, not m68knommu. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-01 22:03 ` Geert Uytterhoeven @ 2011-02-01 22:26 ` Dmitry Torokhov 2011-02-02 14:48 ` Geert Uytterhoeven 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-01 22:26 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Rusty Russell, linux-kernel, Linux/m68k On Tue, Feb 01, 2011 at 02:03:23PM -0800, Geert Uytterhoeven wrote: > On Tue, Feb 1, 2011 at 22:09, Dmitry Torokhov <dtor@vmware.com> wrote: > > On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote: > >> On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List > >> <linux-kernel@vger.kernel.org> wrote: > >> > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de > >> > >> > module: show version information for built-in modules in sysfs > >> > > >> > Currently only drivers that are built as modules have their versions > >> > shown in /sys/module/<module_name>/version, but this information might > >> > also be useful for built-in drivers as well. This especially important > >> > for drivers that do not define any parameters - such drivers, if > >> > built-in, are completely invisible from userspace. > >> > > >> > This patch changes MODULE_VERSION() macro so that in case when we are > >> > compiling built-in module, version information is stored in a separate > >> > section. Kernel then uses this data to create 'version' sysfs attribute > >> > in the same fashion it creates attributes for module parameters. > >> > >> This commit causes the crash below on m68k (ARAnyM). > >> Reverting this commit and its dependency > >> 3b90a5b292321b2acac3921f77046ae195aef53f > >> ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") > >> makes it boot again. > >> > > > > Hi Geert, > > > > Does the follwing help by any chance? > > > > From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001 > > diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S > > index ef33213..47e15eb 100644 > > --- a/arch/m68knommu/kernel/vmlinux.lds.S > > +++ b/arch/m68knommu/kernel/vmlinux.lds.S > > The crash happened on m68k with MMU, not m68knommu. > Hmm, OK then. Could you please see if the crash happens if you return early in kernel/params.c::version_sysfs_builtin() ? Also, do you see anything in __modev section of your build? Thanks, Dmitry ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-01 22:26 ` Dmitry Torokhov @ 2011-02-02 14:48 ` Geert Uytterhoeven 2011-02-02 19:42 ` Dmitry Torokhov 0 siblings, 1 reply; 26+ messages in thread From: Geert Uytterhoeven @ 2011-02-02 14:48 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch On Tue, Feb 1, 2011 at 23:26, Dmitry Torokhov <dtor@vmware.com> wrote: > On Tue, Feb 01, 2011 at 02:03:23PM -0800, Geert Uytterhoeven wrote: >> On Tue, Feb 1, 2011 at 22:09, Dmitry Torokhov <dtor@vmware.com> wrote: >> > On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote: >> >> On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List >> >> <linux-kernel@vger.kernel.org> wrote: >> >> > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de >> >> >> >> > module: show version information for built-in modules in sysfs >> >> > >> >> > Currently only drivers that are built as modules have their versions >> >> > shown in /sys/module/<module_name>/version, but this information might >> >> > also be useful for built-in drivers as well. This especially important >> >> > for drivers that do not define any parameters - such drivers, if >> >> > built-in, are completely invisible from userspace. >> >> > >> >> > This patch changes MODULE_VERSION() macro so that in case when we are >> >> > compiling built-in module, version information is stored in a separate >> >> > section. Kernel then uses this data to create 'version' sysfs attribute >> >> > in the same fashion it creates attributes for module parameters. >> >> >> >> This commit causes the crash below on m68k (ARAnyM). >> >> Reverting this commit and its dependency >> >> 3b90a5b292321b2acac3921f77046ae195aef53f >> >> ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") >> >> makes it boot again. >> >> >> > >> > Hi Geert, >> > >> > Does the follwing help by any chance? >> > >> > From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001 >> > diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S >> > index ef33213..47e15eb 100644 >> > --- a/arch/m68knommu/kernel/vmlinux.lds.S >> > +++ b/arch/m68knommu/kernel/vmlinux.lds.S >> >> The crash happened on m68k with MMU, not m68knommu. >> > > Hmm, OK then. Could you please see if the crash happens if you return > early in kernel/params.c::version_sysfs_builtin() ? Also, do you see It does not crash if version_sysfs_builtin() returns early. > anything in __modev section of your build? "objdump -h" says: | Sections: | Idx Name Size VMA LMA File off Algn | 7 __modver 0000007c 002e0f84 002e0f84 002e0f84 2**2 | CONTENTS, ALLOC, LOAD, DATA "nm vmlinux | grep __modver" says: | 002e0f84 d __modver_version_attr | 002e0fa8 d __modver_version_attr | 00039026 T __modver_version_show | 002e0f84 D __start___modver | 002e0fca D __stop___modver The section size (0x7c) is larger than __stop___modver - __start___modver (0x46)? Adding some debugging code (which increases the section size even more?) shows: vattr = 002e1004 vattr->module_name = xz_dec mk = 00c2ee50 err = 0 kobject_uevent done kobject_put done vattr = 002e1026 vattr->module_name = (null) Unable to handle kernel NULL pointer dereference at virtual address 0000002c So the second module in the list has no name. Why? Aha, it's not NULL, but just < PAGE_SIZE (0x2c). sizeof(struct module_version_attribute) = 34, which you can see from the 2 consecutive vattr pointers above. But the "aligned(sizeof(void *))" in the definition of MODULE_VERSION() puts the next module_version_attribute struct in the array at offset 36, not offset 34! On m68k, the alignment of 32-bit integrals is 2 bytes, not 4. Removing the alignment, cfr. this (gmail-pasted hence whitespace-corrupted) patch fixes it: --- a/include/linux/module.h +++ b/include/linux/module.h @@ -176,7 +176,7 @@ extern struct module __this_module; struct module *, char *); \ static struct module_version_attribute __modver_version_attr \ __used \ - __attribute__ ((__section__ ("__modver"),aligned(sizeof(void *)))) \ + __attribute__ ((__section__ ("__modver"))) \ = { \ .mattr = { \ .attr = { \ Why is the explicit alignment there? Can it be removed? If not, you can move the alignment into the definition of struct module_version_attribute, so it will be honoured everywhere. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-02 14:48 ` Geert Uytterhoeven @ 2011-02-02 19:42 ` Dmitry Torokhov 2011-02-02 22:52 ` Andreas Schwab 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-02 19:42 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch On Wed, Feb 02, 2011 at 06:48:50AM -0800, Geert Uytterhoeven wrote: > On Tue, Feb 1, 2011 at 23:26, Dmitry Torokhov <dtor@vmware.com> wrote: > > On Tue, Feb 01, 2011 at 02:03:23PM -0800, Geert Uytterhoeven wrote: > >> On Tue, Feb 1, 2011 at 22:09, Dmitry Torokhov <dtor@vmware.com> wrote: > >> > On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote: > >> >> On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List > >> >> <linux-kernel@vger.kernel.org> wrote: > >> >> > Gitweb: http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de > >> >> > >> >> > module: show version information for built-in modules in sysfs > >> >> > > >> >> > Currently only drivers that are built as modules have their versions > >> >> > shown in /sys/module/<module_name>/version, but this information might > >> >> > also be useful for built-in drivers as well. This especially important > >> >> > for drivers that do not define any parameters - such drivers, if > >> >> > built-in, are completely invisible from userspace. > >> >> > > >> >> > This patch changes MODULE_VERSION() macro so that in case when we are > >> >> > compiling built-in module, version information is stored in a separate > >> >> > section. Kernel then uses this data to create 'version' sysfs attribute > >> >> > in the same fashion it creates attributes for module parameters. > >> >> > >> >> This commit causes the crash below on m68k (ARAnyM). > >> >> Reverting this commit and its dependency > >> >> 3b90a5b292321b2acac3921f77046ae195aef53f > >> >> ("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n") > >> >> makes it boot again. > >> >> > >> > > >> > Hi Geert, > >> > > >> > Does the follwing help by any chance? > >> > > >> > From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001 > >> > diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S > >> > index ef33213..47e15eb 100644 > >> > --- a/arch/m68knommu/kernel/vmlinux.lds.S > >> > +++ b/arch/m68knommu/kernel/vmlinux.lds.S > >> > >> The crash happened on m68k with MMU, not m68knommu. > >> > > > > Hmm, OK then. Could you please see if the crash happens if you return > > early in kernel/params.c::version_sysfs_builtin() ? Also, do you see > > It does not crash if version_sysfs_builtin() returns early. > > > anything in __modev section of your build? > > "objdump -h" says: > > | Sections: > | Idx Name Size VMA LMA File off Algn > | 7 __modver 0000007c 002e0f84 002e0f84 002e0f84 2**2 > | CONTENTS, ALLOC, LOAD, DATA > > "nm vmlinux | grep __modver" says: > > | 002e0f84 d __modver_version_attr > | 002e0fa8 d __modver_version_attr > | 00039026 T __modver_version_show > | 002e0f84 D __start___modver > | 002e0fca D __stop___modver > > The section size (0x7c) is larger than __stop___modver - > __start___modver (0x46)? > > Adding some debugging code (which increases the section size even more?) shows: > > vattr = 002e1004 > vattr->module_name = xz_dec > mk = 00c2ee50 > err = 0 > kobject_uevent done > kobject_put done > vattr = 002e1026 > vattr->module_name = (null) > Unable to handle kernel NULL pointer dereference at virtual address 0000002c > > So the second module in the list has no name. Why? > Aha, it's not NULL, but just < PAGE_SIZE (0x2c). > > sizeof(struct module_version_attribute) = 34, which you can see from > the 2 consecutive vattr > pointers above. But the "aligned(sizeof(void *))" in the definition of > MODULE_VERSION() puts > the next module_version_attribute struct in the array at offset 36, > not offset 34! > On m68k, the alignment of 32-bit integrals is 2 bytes, not 4. But why is it aligned on 2-byte boundary and why m64k is not happy with module_version_attribute but is happy with kernel_param which is also aligned similarly? If we unroll module_version_attribute it woud look like this: struct module_version_attribute { struct module_attribute { struct attribute { const char *name; mode_t mode; } attr; ... } mattr; const char *module_name; const char *version; }; So I would expect it be aligned on (char *) boundary which should be the same as (void *). Will it help if we rearrange module_version_attribute definition to explicitly have first field being a pointer so it is more like kernel_param, like this: struct module_version_attribute { const char *module_name; const char *version; struct module_attribute mattr; }; Thanks, Dmitry ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-02 19:42 ` Dmitry Torokhov @ 2011-02-02 22:52 ` Andreas Schwab 0 siblings, 0 replies; 26+ messages in thread From: Andreas Schwab @ 2011-02-02 22:52 UTC (permalink / raw) To: Dmitry Torokhov Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch Dmitry Torokhov <dtor@vmware.com> writes: > But why is it aligned on 2-byte boundary and why m64k is not happy with > module_version_attribute but is happy with kernel_param which is also > aligned similarly? struct kernel_parm doesn't contain internal padding on 32 bit architectures (it does on 64bit architectures though). > If we unroll module_version_attribute it woud look like this: > > struct module_version_attribute { > > struct module_attribute { > > struct attribute { > const char *name; > mode_t mode; > } attr; > ... > > } mattr; > > const char *module_name; > const char *version; > }; > > So I would expect it be aligned on (char *) boundary which should be the > same as (void *). mode_t is a 16 bit type, thus any following member becomes aligned on an odd 2 byte boundary. On 32bit architectures with 4 byte alignment and 16 bit mode_t struct attribute contains 2 bytes of internal padding. (64bit architectures typically have a 32bit mode_t, and there are 4 bytes of padding.) > Will it help if we rearrange module_version_attribute definition to > explicitly have first field being a pointer so it is more like > kernel_param, like this: > > struct module_version_attribute { > const char *module_name; > const char *version; > struct module_attribute mattr; > }; That won't change the total size of the structure. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) @ 2011-02-02 22:52 ` Andreas Schwab 0 siblings, 0 replies; 26+ messages in thread From: Andreas Schwab @ 2011-02-02 22:52 UTC (permalink / raw) To: Dmitry Torokhov Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch Dmitry Torokhov <dtor@vmware.com> writes: > But why is it aligned on 2-byte boundary and why m64k is not happy with > module_version_attribute but is happy with kernel_param which is also > aligned similarly? struct kernel_parm doesn't contain internal padding on 32 bit architectures (it does on 64bit architectures though). > If we unroll module_version_attribute it woud look like this: > > struct module_version_attribute { > > struct module_attribute { > > struct attribute { > const char *name; > mode_t mode; > } attr; > ... > > } mattr; > > const char *module_name; > const char *version; > }; > > So I would expect it be aligned on (char *) boundary which should be the > same as (void *). mode_t is a 16 bit type, thus any following member becomes aligned on an odd 2 byte boundary. On 32bit architectures with 4 byte alignment and 16 bit mode_t struct attribute contains 2 bytes of internal padding. (64bit architectures typically have a 32bit mode_t, and there are 4 bytes of padding.) > Will it help if we rearrange module_version_attribute definition to > explicitly have first field being a pointer so it is more like > kernel_param, like this: > > struct module_version_attribute { > const char *module_name; > const char *version; > struct module_attribute mattr; > }; That won't change the total size of the structure. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-02 22:52 ` Andreas Schwab (?) @ 2011-02-02 23:59 ` Dmitry Torokhov 2011-02-03 0:10 ` Andreas Schwab -1 siblings, 1 reply; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-02 23:59 UTC (permalink / raw) To: Andreas Schwab Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch On Wed, Feb 02, 2011 at 02:52:04PM -0800, Andreas Schwab wrote: > Dmitry Torokhov <dtor@vmware.com> writes: > > > But why is it aligned on 2-byte boundary and why m64k is not happy with > > module_version_attribute but is happy with kernel_param which is also > > aligned similarly? > > struct kernel_parm doesn't contain internal padding on 32 bit > architectures (it does on 64bit architectures though). > > > If we unroll module_version_attribute it woud look like this: > > > > struct module_version_attribute { > > > > struct module_attribute { > > > > struct attribute { > > const char *name; > > mode_t mode; > > } attr; > > ... > > > > } mattr; > > > > const char *module_name; > > const char *version; > > }; > > > > So I would expect it be aligned on (char *) boundary which should be the > > same as (void *). > > mode_t is a 16 bit type, thus any following member becomes aligned on an > odd 2 byte boundary. Even pointers? I'd expect pointers to be aligned on 4-bytes boundaries? Thanks, Dmitry ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-02 23:59 ` Dmitry Torokhov @ 2011-02-03 0:10 ` Andreas Schwab 0 siblings, 0 replies; 26+ messages in thread From: Andreas Schwab @ 2011-02-03 0:10 UTC (permalink / raw) To: Dmitry Torokhov Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch Dmitry Torokhov <dtor@vmware.com> writes: > Even pointers? I'd expect pointers to be aligned on 4-bytes boundaries? Pointers are not special in any way. Why should they? On the machine level pointers are just numbers. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) @ 2011-02-03 0:10 ` Andreas Schwab 0 siblings, 0 replies; 26+ messages in thread From: Andreas Schwab @ 2011-02-03 0:10 UTC (permalink / raw) To: Dmitry Torokhov Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch Dmitry Torokhov <dtor@vmware.com> writes: > Even pointers? I'd expect pointers to be aligned on 4-bytes boundaries? Pointers are not special in any way. Why should they? On the machine level pointers are just numbers. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-03 0:10 ` Andreas Schwab (?) @ 2011-02-03 0:24 ` Dmitry Torokhov 2011-02-03 17:38 ` Andreas Schwab 2011-02-07 8:19 ` Dmitry Torokhov -1 siblings, 2 replies; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-03 0:24 UTC (permalink / raw) To: Andreas Schwab Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch On Wed, Feb 02, 2011 at 04:10:04PM -0800, Andreas Schwab wrote: > Dmitry Torokhov <dtor@vmware.com> writes: > > > Even pointers? I'd expect pointers to be aligned on 4-bytes boundaries? > > Pointers are not special in any way. Why should they? On the machine > level pointers are just numbers. Are pointers (along with ints/longs) on m68k naturally aligned on word boundary even though they are 32 bit? Anyway, here is the description that introduced alignment statement: commit 02dba5c6439cff34936460b95cd1ba42b370f345 Author: ak <ak> Date: Sat Jun 21 16:18:16 2003 +0000 [PATCH] Fix over-alignment problem on x86-64 Thanks to Jan Hubicka who suggested this fix. The problem seems to be that gcc generates a 32byte alignment for static objects > 32bytes. This causes gas to set a high alignment on the section, which causes the uneven (not multiple of sizeof(struct kernel_param)) section size. The pointer division with a base not being a multiple of sizeof(*ptr) then causes the invalid result. This just forces a small alignment, which makes the section end come out with the correct alignment. The only mystery left is why ld chose a 16 byte padding instead of 32byte. BKrev: 3ef485487jZN-h3PtASDeL2Vs55NIg I guess this does not directly apply to modversions since they are currently under 32 bytes, but I wonder what happen if we decide to extend one of the structures involved... I guess explicitly setting alignment requirement for struct module_version_attribute is the best option. Thanks, Dmitry ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-03 0:24 ` Dmitry Torokhov @ 2011-02-03 17:38 ` Andreas Schwab 2011-02-07 8:19 ` Dmitry Torokhov 1 sibling, 0 replies; 26+ messages in thread From: Andreas Schwab @ 2011-02-03 17:38 UTC (permalink / raw) To: Dmitry Torokhov Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch Dmitry Torokhov <dtor@vmware.com> writes: > Are pointers (along with ints/longs) on m68k naturally aligned on word > boundary even though they are 32 bit? On m68k, every object by default is at most aligned to a 2 byte boundary. That's a heritage of the original Sun m68000 compiler, which served as the template for the gcc configuration for Linux/m68k when the porting started. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) @ 2011-02-03 17:38 ` Andreas Schwab 0 siblings, 0 replies; 26+ messages in thread From: Andreas Schwab @ 2011-02-03 17:38 UTC (permalink / raw) To: Dmitry Torokhov Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch Dmitry Torokhov <dtor@vmware.com> writes: > Are pointers (along with ints/longs) on m68k naturally aligned on word > boundary even though they are 32 bit? On m68k, every object by default is at most aligned to a 2 byte boundary. That's a heritage of the original Sun m68000 compiler, which served as the template for the gcc configuration for Linux/m68k when the porting started. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-03 17:38 ` Andreas Schwab (?) @ 2011-02-05 23:47 ` Thorsten Glaser 2011-02-08 21:31 ` Andreas Schwab -1 siblings, 1 reply; 26+ messages in thread From: Thorsten Glaser @ 2011-02-05 23:47 UTC (permalink / raw) To: linux-m68k Andreas Schwab dixit: >On m68k, every object by default is at most aligned to a 2 byte >boundary. That's a heritage of the original Sun m68000 compiler, which >served as the template for the gcc configuration for Linux/m68k when the >porting started. Ah, from there. I think it might violate the SYSV ABI, funnily enough (which I've peeked into in order to port elfutils when bored again). Oh the history ;) bye, //mirabilos -- 22:20⎜<asarch> The crazy that persists in his craziness becomes a master 22:21⎜<asarch> And the distance between the craziness and geniality is only measured by the success 22:21⎜<mksh> it’s a very thin line anyway… with some, you don’t know which side they’re on ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-05 23:47 ` Thorsten Glaser @ 2011-02-08 21:31 ` Andreas Schwab 2011-02-08 22:46 ` Thorsten Glaser 0 siblings, 1 reply; 26+ messages in thread From: Andreas Schwab @ 2011-02-08 21:31 UTC (permalink / raw) To: Thorsten Glaser; +Cc: linux-m68k Thorsten Glaser <tg@mirbsd.de> writes: > Andreas Schwab dixit: > >>On m68k, every object by default is at most aligned to a 2 byte >>boundary. That's a heritage of the original Sun m68000 compiler, which >>served as the template for the gcc configuration for Linux/m68k when the >>porting started. > > Ah, from there. I think it might violate the SYSV ABI, funnily > enough (which I've peeked into in order to port elfutils when > bored again). Oh the history ;) I looked into fixing that when I ported m68k from a.out to ELF. Unfortunately, that would have broken a big part of the syscall/ioctl ABI, so I had to scratch that. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-08 21:31 ` Andreas Schwab @ 2011-02-08 22:46 ` Thorsten Glaser 0 siblings, 0 replies; 26+ messages in thread From: Thorsten Glaser @ 2011-02-08 22:46 UTC (permalink / raw) To: Andreas Schwab; +Cc: linux-m68k Andreas Schwab dixit: >>>On m68k, every object by default is at most aligned to a 2 byte >>>boundary. That's a heritage of the original Sun m68000 compiler, which >>>served as the template for the gcc configuration for Linux/m68k when the >>>porting started. >> >> Ah, from there. I think it might violate the SYSV ABI, funnily >> enough (which I've peeked into in order to port elfutils when >> bored again). Oh the history ;) > >I looked into fixing that when I ported m68k from a.out to ELF. >Unfortunately, that would have broken a big part of the syscall/ioctl >ABI, so I had to scratch that. Ah, I understand. I had the breakage in MirBSD with time_t… so I don't think anyone could blame you ;) Nice to know, though. I'll update places like the Debian Wiki. bye, //mirabilos -- 22:20⎜<asarch> The crazy that persists in his craziness becomes a master 22:21⎜<asarch> And the distance between the craziness and geniality is only measured by the success 18:35⎜<asarch> "Psychotics are consistently inconsistent. The essence of sanity is to be inconsistently inconsistent ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) 2011-02-03 0:24 ` Dmitry Torokhov @ 2011-02-07 8:19 ` Dmitry Torokhov 2011-02-07 8:19 ` Dmitry Torokhov 1 sibling, 0 replies; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-07 8:19 UTC (permalink / raw) To: Andreas Schwab Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch On Wed, Feb 02, 2011 at 04:24:59PM -0800, Dmitry Torokhov wrote: > On Wed, Feb 02, 2011 at 04:10:04PM -0800, Andreas Schwab wrote: > > Dmitry Torokhov <dtor@vmware.com> writes: > > > > > Even pointers? I'd expect pointers to be aligned on 4-bytes boundaries? > > > > Pointers are not special in any way. Why should they? On the machine > > level pointers are just numbers. > > Are pointers (along with ints/longs) on m68k naturally aligned on word > boundary even though they are 32 bit? > > Anyway, here is the description that introduced alignment statement: > > commit 02dba5c6439cff34936460b95cd1ba42b370f345 > Author: ak <ak> > Date: Sat Jun 21 16:18:16 2003 +0000 > > [PATCH] Fix over-alignment problem on x86-64 > > Thanks to Jan Hubicka who suggested this fix. > > The problem seems to be that gcc generates a 32byte alignment for static > objects > 32bytes. This causes gas to set a high alignment on the > section, which causes the uneven (not multiple of sizeof(struct > kernel_param)) section size. The pointer division with a base not being > a multiple of sizeof(*ptr) then causes the invalid result. > > This just forces a small alignment, which makes the section end come out > with the correct alignment. > > The only mystery left is why ld chose a 16 byte padding instead of > 32byte. > > BKrev: 3ef485487jZN-h3PtASDeL2Vs55NIg > > > I guess this does not directly apply to modversions since they are > currently under 32 bytes, but I wonder what happen if we decide to > extend one of the structures involved... > > I guess explicitly setting alignment requirement for struct > module_version_attribute is the best option. > So here is the patch that explicitly specifies alignment for struct module_version_attribute. I tested it on i386 and x86_64 and I believe it will fix the issue with m68k but I do not have access to such a box. Thanks, Dmitry >From f0e0e10b58b22047e36e21a022abf5e86b5819c2 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov <dtor@vmware.com> Date: Fri, 4 Feb 2011 13:30:10 -0800 Subject: [PATCH] module: explicitly align module_version_attribute structure We force particular alignment when we generate attribute structures when generation MODULE_VERSION() data and we need to make sure that this alignment is followed when we iterate over these structures, otherwise we may crash on platforms whose natural alignment is not sizeof(void *), such as m68k. Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Dmitry Torokhov <dtor@vmware.com> --- include/linux/module.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index e7c6385..de5cd21 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -62,7 +62,7 @@ struct module_version_attribute { struct module_attribute mattr; const char *module_name; const char *version; -}; +} __attribute__ ((__aligned__(sizeof(void *)))); struct module_kobject { -- 1.7.3.2 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Early crash (was: Re: module: show version information for built-in modules in sysfs) @ 2011-02-07 8:19 ` Dmitry Torokhov 0 siblings, 0 replies; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-07 8:19 UTC (permalink / raw) To: Andreas Schwab Cc: Geert Uytterhoeven, Rusty Russell, linux-kernel, Linux/m68k, Linux-Arch On Wed, Feb 02, 2011 at 04:24:59PM -0800, Dmitry Torokhov wrote: > On Wed, Feb 02, 2011 at 04:10:04PM -0800, Andreas Schwab wrote: > > Dmitry Torokhov <dtor@vmware.com> writes: > > > > > Even pointers? I'd expect pointers to be aligned on 4-bytes boundaries? > > > > Pointers are not special in any way. Why should they? On the machine > > level pointers are just numbers. > > Are pointers (along with ints/longs) on m68k naturally aligned on word > boundary even though they are 32 bit? > > Anyway, here is the description that introduced alignment statement: > > commit 02dba5c6439cff34936460b95cd1ba42b370f345 > Author: ak <ak> > Date: Sat Jun 21 16:18:16 2003 +0000 > > [PATCH] Fix over-alignment problem on x86-64 > > Thanks to Jan Hubicka who suggested this fix. > > The problem seems to be that gcc generates a 32byte alignment for static > objects > 32bytes. This causes gas to set a high alignment on the > section, which causes the uneven (not multiple of sizeof(struct > kernel_param)) section size. The pointer division with a base not being > a multiple of sizeof(*ptr) then causes the invalid result. > > This just forces a small alignment, which makes the section end come out > with the correct alignment. > > The only mystery left is why ld chose a 16 byte padding instead of > 32byte. > > BKrev: 3ef485487jZN-h3PtASDeL2Vs55NIg > > > I guess this does not directly apply to modversions since they are > currently under 32 bytes, but I wonder what happen if we decide to > extend one of the structures involved... > > I guess explicitly setting alignment requirement for struct > module_version_attribute is the best option. > So here is the patch that explicitly specifies alignment for struct module_version_attribute. I tested it on i386 and x86_64 and I believe it will fix the issue with m68k but I do not have access to such a box. Thanks, Dmitry From f0e0e10b58b22047e36e21a022abf5e86b5819c2 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov <dtor@vmware.com> Date: Fri, 4 Feb 2011 13:30:10 -0800 Subject: [PATCH] module: explicitly align module_version_attribute structure We force particular alignment when we generate attribute structures when generation MODULE_VERSION() data and we need to make sure that this alignment is followed when we iterate over these structures, otherwise we may crash on platforms whose natural alignment is not sizeof(void *), such as m68k. Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Dmitry Torokhov <dtor@vmware.com> --- include/linux/module.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index e7c6385..de5cd21 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -62,7 +62,7 @@ struct module_version_attribute { struct module_attribute mattr; const char *module_name; const char *version; -}; +} __attribute__ ((__aligned__(sizeof(void *)))); struct module_kobject { -- 1.7.3.2 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Early crash 2011-02-07 8:19 ` Dmitry Torokhov (?) @ 2011-02-07 8:50 ` David Miller 2011-02-07 16:58 ` Dmitry Torokhov -1 siblings, 1 reply; 26+ messages in thread From: David Miller @ 2011-02-07 8:50 UTC (permalink / raw) To: dtor; +Cc: schwab, geert, rusty, linux-kernel, linux-m68k, linux-arch From: Dmitry Torokhov <dtor@vmware.com> Date: Mon, 7 Feb 2011 00:19:33 -0800 > So here is the patch that explicitly specifies alignment for struct > module_version_attribute. I tested it on i386 and x86_64 and I believe > it will fix the issue with m68k but I do not have access to such a box. While this may or may not fix the m68k issue, this isn't really sufficient to make this thing work in all cases. And the older tracepoint commits referenced in this thread are known to cause problems with platforms such as sparc64. You can't reliably put structures into independent objects, put them into a special section, and then expect array access over them (via the section boundaries) after linking the objects together to just "work". Your attribute specification is only a lower-bound. GCC can and does use variable alignment choices in different situations, so the align directive in the various objects can all be different. So the array iteration will assume the iterations should use one object size, but within the linked together section the alignments are all different so the inter-struct gap is different. The only portable mechanism that will work in all cases, as we've found recently for tracepoints and similar, is to make an array of plain pointers to the objects in the special section. See: http://marc.info/?l=linux-kernel&m=129674396021733&w=2 http://marc.info/?l=linux-kernel&m=129674399621795&w=2 http://marc.info/?l=linux-kernel&m=129674396121739&w=2 http://marc.info/?l=linux-kernel&m=129674396021735&w=2 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash 2011-02-07 8:50 ` Early crash David Miller @ 2011-02-07 16:58 ` Dmitry Torokhov 2011-02-07 19:27 ` David Miller 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-07 16:58 UTC (permalink / raw) To: David Miller; +Cc: schwab, geert, rusty, linux-kernel, linux-m68k, linux-arch On Mon, Feb 07, 2011 at 12:50:10AM -0800, David Miller wrote: > From: Dmitry Torokhov <dtor@vmware.com> > Date: Mon, 7 Feb 2011 00:19:33 -0800 > > > So here is the patch that explicitly specifies alignment for struct > > module_version_attribute. I tested it on i386 and x86_64 and I believe > > it will fix the issue with m68k but I do not have access to such a box. > > While this may or may not fix the m68k issue, this isn't really > sufficient to make this thing work in all cases. And the older > tracepoint commits referenced in this thread are known to cause > problems with platforms such as sparc64. > > You can't reliably put structures into independent objects, put them > into a special section, and then expect array access over them (via > the section boundaries) after linking the objects together to just > "work". > > Your attribute specification is only a lower-bound. Right. > > GCC can and does use variable alignment choices in different situations, > so the align directive in the various objects can all be different. So > the array iteration will assume the iterations should use one object > size, but within the linked together section the alignments are all > different so the inter-struct gap is different. > > The only portable mechanism that will work in all cases, as we've > found recently for tracepoints and similar, is to make an array of > plain pointers to the objects in the special section. But, theoretically speaking, nothing stops GCC to align pointers with "gaps" as well? Let's say having everything (or some) aligned on quadword boundary even though arch is 32 bit? Thanks, Dmitry ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash 2011-02-07 16:58 ` Dmitry Torokhov @ 2011-02-07 19:27 ` David Miller 2011-02-07 19:28 ` Dmitry Torokhov 2011-02-08 3:12 ` Rusty Russell 0 siblings, 2 replies; 26+ messages in thread From: David Miller @ 2011-02-07 19:27 UTC (permalink / raw) To: dtor; +Cc: schwab, geert, rusty, linux-kernel, linux-m68k, linux-arch From: Dmitry Torokhov <dtor@vmware.com> Date: Mon, 7 Feb 2011 08:58:29 -0800 > But, theoretically speaking, nothing stops GCC to align pointers with > "gaps" as well? Let's say having everything (or some) aligned on > quadword boundary even though arch is 32 bit? The alignment business only applies to aggregates (ie. structs and unions). This has been confirmed via several weeks of expermentation with different GCC versions on different platforms as well. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash 2011-02-07 19:27 ` David Miller @ 2011-02-07 19:28 ` Dmitry Torokhov 2011-02-08 3:12 ` Rusty Russell 1 sibling, 0 replies; 26+ messages in thread From: Dmitry Torokhov @ 2011-02-07 19:28 UTC (permalink / raw) To: David Miller; +Cc: schwab, geert, rusty, linux-kernel, linux-m68k, linux-arch On Mon, Feb 07, 2011 at 11:27:08AM -0800, David Miller wrote: > From: Dmitry Torokhov <dtor@vmware.com> > Date: Mon, 7 Feb 2011 08:58:29 -0800 > > > But, theoretically speaking, nothing stops GCC to align pointers with > > "gaps" as well? Let's say having everything (or some) aligned on > > quadword boundary even though arch is 32 bit? > > The alignment business only applies to aggregates (ie. structs and > unions). > > This has been confirmed via several weeks of expermentation with > different GCC versions on different platforms as well. I see. OK, then I'll prepare a patch to switch over to struct + pointer scheme. Thanks, Dmitry ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash 2011-02-07 19:27 ` David Miller 2011-02-07 19:28 ` Dmitry Torokhov @ 2011-02-08 3:12 ` Rusty Russell 2011-02-08 3:31 ` David Miller 1 sibling, 1 reply; 26+ messages in thread From: Rusty Russell @ 2011-02-08 3:12 UTC (permalink / raw) To: David Miller; +Cc: dtor, schwab, geert, linux-kernel, linux-m68k, linux-arch On Tue, 8 Feb 2011 05:57:08 am David Miller wrote: > From: Dmitry Torokhov <dtor@vmware.com> > Date: Mon, 7 Feb 2011 08:58:29 -0800 > > > But, theoretically speaking, nothing stops GCC to align pointers with > > "gaps" as well? Let's say having everything (or some) aligned on > > quadword boundary even though arch is 32 bit? > > The alignment business only applies to aggregates (ie. structs and > unions). > > This has been confirmed via several weeks of expermentation with > different GCC versions on different platforms as well. But OTOH, this is an old problem which was faced by module params since pre-git. And we use the-align-to-void*-size method there; I vaguely recall inserting it. You've now got me wondering whether these platforms have broken builtin module parameters, but I think it would crash iterating if you had any boot parameters at all if that were the case. So do we fix that now too, or wait for it to break? Rusty. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Early crash 2011-02-08 3:12 ` Rusty Russell @ 2011-02-08 3:31 ` David Miller 0 siblings, 0 replies; 26+ messages in thread From: David Miller @ 2011-02-08 3:31 UTC (permalink / raw) To: rusty; +Cc: dtor, schwab, geert, linux-kernel, linux-m68k, linux-arch From: Rusty Russell <rusty@rustcorp.com.au> Date: Tue, 8 Feb 2011 13:42:48 +1030 > But OTOH, this is an old problem which was faced by module params since > pre-git. And we use the-align-to-void*-size method there; I vaguely recall > inserting it. > > You've now got me wondering whether these platforms have broken builtin > module parameters, but I think it would crash iterating if you had any > boot parameters at all if that were the case. > > So do we fix that now too, or wait for it to break? Good question. I think the magic align magic on x86_64 in gcc only kicks in if a structure is larger than a certain size. But of course they could change that at some point if they like. So probably better safe than sorry. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2011-02-08 22:48 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-01 20:33 Early crash (was: Re: module: show version information for built-in modules in sysfs) Geert Uytterhoeven 2011-02-01 21:09 ` Dmitry Torokhov 2011-02-01 21:09 ` Dmitry Torokhov 2011-02-01 22:03 ` Geert Uytterhoeven 2011-02-01 22:26 ` Dmitry Torokhov 2011-02-02 14:48 ` Geert Uytterhoeven 2011-02-02 19:42 ` Dmitry Torokhov 2011-02-02 22:52 ` Andreas Schwab 2011-02-02 22:52 ` Andreas Schwab 2011-02-02 23:59 ` Dmitry Torokhov 2011-02-03 0:10 ` Andreas Schwab 2011-02-03 0:10 ` Andreas Schwab 2011-02-03 0:24 ` Dmitry Torokhov 2011-02-03 17:38 ` Andreas Schwab 2011-02-03 17:38 ` Andreas Schwab 2011-02-05 23:47 ` Thorsten Glaser 2011-02-08 21:31 ` Andreas Schwab 2011-02-08 22:46 ` Thorsten Glaser 2011-02-07 8:19 ` Dmitry Torokhov 2011-02-07 8:19 ` Dmitry Torokhov 2011-02-07 8:50 ` Early crash David Miller 2011-02-07 16:58 ` Dmitry Torokhov 2011-02-07 19:27 ` David Miller 2011-02-07 19:28 ` Dmitry Torokhov 2011-02-08 3:12 ` Rusty Russell 2011-02-08 3:31 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.