On Wed, Feb 02, 2011 at 06:48:50AM -0800, Geert Uytterhoeven wrote: > On Tue, Feb 1, 2011 at 23:26, Dmitry Torokhov <dtor@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
