On Mon, 2011-02-21 at 14:30 +1030, Rusty Russell wrote: > > Except that .long is 32-bit on ppc64 :-( You need .llong for 64-bit. > > OK, all options suck. Do we want the workaround or not?
The only sane thing I can see is make sure that such structures that we put into sections "arrays" like that are naturally aligned with padding. Cheers, Ben. > Cheers, > Rusty. > > Subject: module: deal with alignment issues in built-in module versions > Date: Mon, 7 Feb 2011 16:02:25 -0800 > From: Dmitry Torokhov <[email protected]> > > On m68k natural alignment is 2-byte boundary but we are trying to > align structures in __modver section on sizeof(void *) boundary. > This causes trouble when we try to access elements in this section > in array-like fashion when create "version" attributes for built-in > modules. > > Moreover, as DaveM said, we 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" due to variable alignment choices in > different situations. The only solution that seems to work reliably > is to make an array of plain pointers to the objects in question and > put those pointers in the special section. > > Reported-by: Geert Uytterhoeven <[email protected]> > Signed-off-by: Dmitry Torokhov <[email protected]> > Signed-off-by: Rusty Russell <[email protected]> > --- > include/linux/module.h | 10 +++++----- > kernel/params.c | 9 ++++++--- > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 9bdf27c..9b7081a 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -174,10 +174,7 @@ extern struct module __this_module; > #define MODULE_VERSION(_version) \ > extern ssize_t __modver_version_show(struct module_attribute *, \ > struct module *, char *); \ > - static struct module_version_attribute __modver_version_attr \ > - __used \ > - __attribute__ ((__section__ ("__modver"),aligned(sizeof(void *)))) \ > - = { \ > + static struct module_version_attribute ___modver_attr = { \ > .mattr = { \ > .attr = { \ > .name = "version", \ > @@ -187,7 +184,10 @@ extern struct module __this_module; > }, \ > .module_name = KBUILD_MODNAME, \ > .version = _version, \ > - } > + }; \ > + static const struct module_version_attribute \ > + __used __attribute__ ((__section__ ("__modver"))) \ > + * __moduleparam_const __modver_attr = &___modver_attr > #endif > > /* Optional firmware file (or files) needed by the module > diff --git a/kernel/params.c b/kernel/params.c > index 0da1411..09a08cb 100644 > --- a/kernel/params.c > +++ b/kernel/params.c > @@ -821,15 +821,18 @@ ssize_t __modver_version_show(struct module_attribute > *mattr, > return sprintf(buf, "%s\n", vattr->version); > } > > -extern struct module_version_attribute __start___modver[], __stop___modver[]; > +extern const struct module_version_attribute *__start___modver[]; > +extern const struct module_version_attribute *__stop___modver[]; > > static void __init version_sysfs_builtin(void) > { > - const struct module_version_attribute *vattr; > + const struct module_version_attribute **p; > struct module_kobject *mk; > int err; > > - for (vattr = __start___modver; vattr < __stop___modver; vattr++) { > + for (p = __start___modver; p < __stop___modver; p++) { > + const struct module_version_attribute *vattr = *p; > + > mk = locate_module_kobject(vattr->module_name); > if (mk) { > err = sysfs_create_file(&mk->kobj, &vattr->mattr.attr); > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
