Re: [PATCH 1/3] module: deal with alignment issues in built-in module versions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 21, 2011 at 05:00, Rusty Russell <rusty@xxxxxxxxxxxxxxx> 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?

We can discuss about that until someone gets bitten by that.

But please fix the "aligned(sizeof(void *))"-in-one-place-only issue.

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 <dtor@xxxxxxxxxx>

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 <geert@xxxxxxxxxxxxxx>
Signed-off-by: Dmitry Torokhov <dtor@xxxxxxxxxx>
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
---
Â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);

Gr{oetje,eeting}s,

            Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux