Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
the modaliases being exposed.
This fixes the problem by including the name of the device_id table in the
__mod_*_device_table alias, allowing us to export several device_id tables
per module.
Suggested-by: Kay Sievers <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rusty Russell <[email protected]>
---
include/linux/module.h | 2 +-
scripts/mod/file2alias.c | 14 +++++++++-----
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 15cd6b1..7732d76 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -143,7 +143,7 @@ extern const struct gtype##_id __mod_##gtype##_table \
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
#define MODULE_DEVICE_TABLE(type,name) \
- MODULE_GENERIC_TABLE(type##_device,name)
+ MODULE_GENERIC_TABLE(type##__##name##_device,name)
/* Version of form [<epoch>:]<version>[-<extra-version>].
Or for CVS/RCS ID version, everything but the number is stripped.
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 2370863..6778381 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -42,7 +42,7 @@ typedef unsigned char __u8;
/* This array collects all instances that use the generic do_table */
struct devtable {
- const char *device_id; /* name of table, __mod_<name>_device_table. */
+ const char *device_id; /* name of table, __mod_<name>__*_device_table. */
unsigned long id_size;
void *function;
};
@@ -146,7 +146,8 @@ static void device_id_check(const char *modname, const char *device_id,
if (size % id_size || size < id_size) {
fatal("%s: sizeof(struct %s_device_id)=%lu is not a modulo "
- "of the size of section __mod_%s_device_table=%lu.\n"
+ "of the size of "
+ "section __mod_%s__<identifier>_device_table=%lu.\n"
"Fix definition of struct %s_device_id "
"in mod_devicetable.h\n",
modname, device_id, id_size, device_id, size, device_id);
@@ -1206,7 +1207,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
{
void *symval;
char *zeros = NULL;
- const char *name;
+ const char *name, *identifier;
unsigned int namelen;
/* We're looking for a section relative symbol */
@@ -1217,7 +1218,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form <prefix>__mod_XXX_device_table. */
+ /* All our symbols are of form <prefix>__mod_<name>__<identifier>_device_table. */
name = strstr(symname, "__mod_");
if (!name)
return;
@@ -1227,7 +1228,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
return;
if (strcmp(name + namelen - strlen("_device_table"), "_device_table"))
return;
- namelen -= strlen("_device_table");
+ identifier = strstr(name, "__");
+ if (!identifier)
+ return;
+ namelen = identifier - name;
/* Handle all-NULL symbols allocated into .bss */
if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
--
1.8.5.3
On Mon, Jan 27, 2014 at 08:09:55PM +0100, Tom Gundersen wrote:
> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> the modaliases being exposed.
>
> This fixes the problem by including the name of the device_id table in the
> __mod_*_device_table alias, allowing us to export several device_id tables
> per module.
>
> Suggested-by: Kay Sievers <[email protected]>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Rusty Russell <[email protected]>
> ---
> include/linux/module.h | 2 +-
> scripts/mod/file2alias.c | 14 +++++++++-----
> 2 files changed, 10 insertions(+), 6 deletions(-)
Ah, very nice, I've wanted this for a while now, it would make a number
of different drivers much smaller and simpler to add new device ids to
(no multiple lists of ids, one for the module loader and one for the
sub-driver that is in the single file.)
If this doesn't break any userspace tools, I have no objection to it at
all:
Acked-by: Greg Kroah-Hartman <[email protected]>
On Mon, Jan 27, 2014 at 11:54:22AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Jan 27, 2014 at 08:09:55PM +0100, Tom Gundersen wrote:
> > Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> > second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> > the modaliases being exposed.
> >
> > This fixes the problem by including the name of the device_id table in the
> > __mod_*_device_table alias, allowing us to export several device_id tables
> > per module.
> >
> > Suggested-by: Kay Sievers <[email protected]>
> > Cc: Dmitry Torokhov <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Rusty Russell <[email protected]>
> > ---
> > include/linux/module.h | 2 +-
> > scripts/mod/file2alias.c | 14 +++++++++-----
> > 2 files changed, 10 insertions(+), 6 deletions(-)
>
> Ah, very nice, I've wanted this for a while now, it would make a number
> of different drivers much smaller and simpler to add new device ids to
> (no multiple lists of ids, one for the module loader and one for the
> sub-driver that is in the single file.)
>
> If this doesn't break any userspace tools, I have no objection to it at
> all:
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
Looks great to me as long as module tools keep working.
Thanks.
--
Dmitry
Greg Kroah-Hartman <[email protected]> writes:
> On Mon, Jan 27, 2014 at 08:09:55PM +0100, Tom Gundersen wrote:
>> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
>> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
>> the modaliases being exposed.
>>
>> This fixes the problem by including the name of the device_id table in the
>> __mod_*_device_table alias, allowing us to export several device_id tables
>> per module.
>>
>> Suggested-by: Kay Sievers <[email protected]>
>> Cc: Dmitry Torokhov <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Rusty Russell <[email protected]>
>> ---
>> include/linux/module.h | 2 +-
>> scripts/mod/file2alias.c | 14 +++++++++-----
>> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> Ah, very nice, I've wanted this for a while now, it would make a number
> of different drivers much smaller and simpler to add new device ids to
> (no multiple lists of ids, one for the module loader and one for the
> sub-driver that is in the single file.)
You never asked :(
I've applied, this, but I'm actually amazed this patch works. C is
weird sometimes. It changes declarations of the form:
extern const struct pci_device_id __mod_pci_device_id_table
__attribute__ ((unused, alias("e1000_pci_tbl"));
Into:
extern const struct pci__e1000_pci_tbl_device_id __mod_pci__e1000_pci_tbl_device_id_table
__attribute__ ((unused, alias("e1000_pci_tbl"));
Now, that's a completely unknown type, but gcc doesn't care because it's
just an extern declaration. It does insert the alias, which is all we
care about.
We would normally use a special section for this, so it's mainly
historical. Now we have DEFINE_PCI_DEVICE_TABLE etc, we should
use those to put it in a special section (eg. "pci_ids") and
grab that directly.
Volunteers welcome :)
Rusty.
Tom Gundersen <[email protected]> writes:
> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> the modaliases being exposed.
No Signed-off-by?
Thanks,
Rusty.
>
> This fixes the problem by including the name of the device_id table in the
> __mod_*_device_table alias, allowing us to export several device_id tables
> per module.
>
> Suggested-by: Kay Sievers <[email protected]>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Rusty Russell <[email protected]>
> ---
> include/linux/module.h | 2 +-
> scripts/mod/file2alias.c | 14 +++++++++-----
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 15cd6b1..7732d76 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -143,7 +143,7 @@ extern const struct gtype##_id __mod_##gtype##_table \
> #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
>
> #define MODULE_DEVICE_TABLE(type,name) \
> - MODULE_GENERIC_TABLE(type##_device,name)
> + MODULE_GENERIC_TABLE(type##__##name##_device,name)
>
> /* Version of form [<epoch>:]<version>[-<extra-version>].
> Or for CVS/RCS ID version, everything but the number is stripped.
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 2370863..6778381 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -42,7 +42,7 @@ typedef unsigned char __u8;
>
> /* This array collects all instances that use the generic do_table */
> struct devtable {
> - const char *device_id; /* name of table, __mod_<name>_device_table. */
> + const char *device_id; /* name of table, __mod_<name>__*_device_table. */
> unsigned long id_size;
> void *function;
> };
> @@ -146,7 +146,8 @@ static void device_id_check(const char *modname, const char *device_id,
>
> if (size % id_size || size < id_size) {
> fatal("%s: sizeof(struct %s_device_id)=%lu is not a modulo "
> - "of the size of section __mod_%s_device_table=%lu.\n"
> + "of the size of "
> + "section __mod_%s__<identifier>_device_table=%lu.\n"
> "Fix definition of struct %s_device_id "
> "in mod_devicetable.h\n",
> modname, device_id, id_size, device_id, size, device_id);
> @@ -1206,7 +1207,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> {
> void *symval;
> char *zeros = NULL;
> - const char *name;
> + const char *name, *identifier;
> unsigned int namelen;
>
> /* We're looking for a section relative symbol */
> @@ -1217,7 +1218,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
> return;
>
> - /* All our symbols are of form <prefix>__mod_XXX_device_table. */
> + /* All our symbols are of form <prefix>__mod_<name>__<identifier>_device_table. */
> name = strstr(symname, "__mod_");
> if (!name)
> return;
> @@ -1227,7 +1228,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> return;
> if (strcmp(name + namelen - strlen("_device_table"), "_device_table"))
> return;
> - namelen -= strlen("_device_table");
> + identifier = strstr(name, "__");
> + if (!identifier)
> + return;
> + namelen = identifier - name;
>
> /* Handle all-NULL symbols allocated into .bss */
> if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
> --
> 1.8.5.3
Hi Rusty,
On Tue, Jan 28, 2014 at 2:35 AM, Rusty Russell <[email protected]> wrote:
> Tom Gundersen <[email protected]> writes:
>> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
>> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
>> the modaliases being exposed.
>
> No Signed-off-by?
Sorry about that, I'll resend in a minute with the signoff.
Cheers,
Tom
Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
the modaliases being exposed.
This fixes the problem by including the name of the device_id table in the
__mod_*_device_table alias, allowing us to export several device_id tables
per module.
Suggested-by: Kay Sievers <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Rusty Russell <[email protected]>
Signed-off-by: Tom Gundersen <[email protected]>
---
include/linux/module.h | 2 +-
scripts/mod/file2alias.c | 14 +++++++++-----
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 15cd6b1..7732d76 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -143,7 +143,7 @@ extern const struct gtype##_id __mod_##gtype##_table \
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
#define MODULE_DEVICE_TABLE(type,name) \
- MODULE_GENERIC_TABLE(type##_device,name)
+ MODULE_GENERIC_TABLE(type##__##name##_device,name)
/* Version of form [<epoch>:]<version>[-<extra-version>].
Or for CVS/RCS ID version, everything but the number is stripped.
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 2370863..6778381 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -42,7 +42,7 @@ typedef unsigned char __u8;
/* This array collects all instances that use the generic do_table */
struct devtable {
- const char *device_id; /* name of table, __mod_<name>_device_table. */
+ const char *device_id; /* name of table, __mod_<name>__*_device_table. */
unsigned long id_size;
void *function;
};
@@ -146,7 +146,8 @@ static void device_id_check(const char *modname, const char *device_id,
if (size % id_size || size < id_size) {
fatal("%s: sizeof(struct %s_device_id)=%lu is not a modulo "
- "of the size of section __mod_%s_device_table=%lu.\n"
+ "of the size of "
+ "section __mod_%s__<identifier>_device_table=%lu.\n"
"Fix definition of struct %s_device_id "
"in mod_devicetable.h\n",
modname, device_id, id_size, device_id, size, device_id);
@@ -1206,7 +1207,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
{
void *symval;
char *zeros = NULL;
- const char *name;
+ const char *name, *identifier;
unsigned int namelen;
/* We're looking for a section relative symbol */
@@ -1217,7 +1218,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form <prefix>__mod_XXX_device_table. */
+ /* All our symbols are of form <prefix>__mod_<name>__<identifier>_device_table. */
name = strstr(symname, "__mod_");
if (!name)
return;
@@ -1227,7 +1228,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
return;
if (strcmp(name + namelen - strlen("_device_table"), "_device_table"))
return;
- namelen -= strlen("_device_table");
+ identifier = strstr(name, "__");
+ if (!identifier)
+ return;
+ namelen = identifier - name;
/* Handle all-NULL symbols allocated into .bss */
if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
--
1.8.5.3
Tom Gundersen <[email protected]> writes:
> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> the modaliases being exposed.
>
> This fixes the problem by including the name of the device_id table in the
> __mod_*_device_table alias, allowing us to export several device_id tables
> per module.
Thanks, applied.
I also put this on top:
module: remove MODULE_GENERIC_TABLE
MODULE_DEVICE_TABLE() calles MODULE_GENERIC_TABLE(); make it do the
work directly. This also removes a wart introduced in the last patch,
where the alias is defined to be an unknown struct type "struct
type##__##name##_device_id" instead of "struct type##_device_id" (it's
an extern so GCC doesn't care, but it's wrong).
The other user of MODULE_GENERIC_TABLE (ISAPNP_CARD_TABLE) is unused,
so delete it.
Signed-off-by: Rusty Russell <[email protected]>
diff --git a/include/linux/isapnp.h b/include/linux/isapnp.h
index e2d28b026a8c..3c77bf9b1efd 100644
--- a/include/linux/isapnp.h
+++ b/include/linux/isapnp.h
@@ -56,10 +56,6 @@
#define ISAPNP_DEVICE_ID(_va, _vb, _vc, _function) \
{ .vendor = ISAPNP_VENDOR(_va, _vb, _vc), .function = ISAPNP_FUNCTION(_function) }
-/* export used IDs outside module */
-#define ISAPNP_CARD_TABLE(name) \
- MODULE_GENERIC_TABLE(isapnp_card, name)
-
struct isapnp_card_id {
unsigned long driver_data; /* data private to the driver */
unsigned short card_vendor, card_device;
diff --git a/include/linux/module.h b/include/linux/module.h
index ad18f6006f86..5686b37e11d6 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -82,15 +82,6 @@ void sort_extable(struct exception_table_entry *start,
void sort_main_extable(void);
void trim_init_extable(struct module *m);
-#ifdef MODULE
-#define MODULE_GENERIC_TABLE(gtype, name) \
-extern const struct gtype##_id __mod_##gtype##_table \
- __attribute__ ((unused, alias(__stringify(name))))
-
-#else /* !MODULE */
-#define MODULE_GENERIC_TABLE(gtype, name)
-#endif
-
/* Generic info of form tag = "info" */
#define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
@@ -141,8 +132,14 @@ extern const struct gtype##_id __mod_##gtype##_table \
/* What your module does. */
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
-#define MODULE_DEVICE_TABLE(type, name) \
- MODULE_GENERIC_TABLE(type##__##name##_device, name)
+#ifdef MODULE
+/* Creates an alias so file2alias.c can find device table. */
+#define MODULE_DEVICE_TABLE(type, name) \
+ extern const struct type##_device_id __mod_##type##__##name##_device_table \
+ __attribute__ ((unused, alias(__stringify(name))))
+#else /* !MODULE */
+#define MODULE_DEVICE_TABLE(type, name)
+#endif
/* Version of form [<epoch>:]<version>[-<extra-version>].
* Or for CVS/RCS ID version, everything but the number is stripped.