Skip to contentSkip to conversation

scripts/mod/modpost.c: New option -E to load extra symbols

PATCH In review Richard Hacker 2 participants 4 messages Feb 28, 2008 – Apr 25, 2008 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 scripts/mod/modpost.c: New option -E to load extra symbols

This patch adds a new command line option -E to modpost, expecting a symbol file as an argument which is read prior to symbol processing. -E can be supplied multiple times for as many files as is needed.

---
When building kernel modules that depend on other modules not in the main kernel tree, modpost complains about undefined symbols:

# make -C /path/to/linux/kernel M=/path/to/my/module
...
Building modules, stage 2.
....
WARNING: "rt_copy_buf" [/home/rich/osc_etl_rtw/osc_kmod.ko] undefined!
...etc

This situation occurs when modpost processes the new module's symbols. When it finds symbols not exported by the mainline kernel, it issues this warning.

The patch adds a new command line option -E to modpost which expects a symbol file as an argument. The symbols listed in this file are added to modpost's symbol tables during startup. -E can be supplied as often as required.

This patch works together with the second patch. It introduces a new make variable, EXTRA_SYMBOLS, which is used when calling modpost.

I hope this patch is useful and finds its way into the kernel.

o Changes from the previous attempt:
- patches kernel version 2.6.24
- patches for scripts/mod/modpost.c and scripts/Makefile.modpost are separated
- the argument to -E is now a single file, not a comma separated list any more. To pass more than one file, -E is used as often as required
- patch 002/002 in this series introduces the Kbuild variable EXTRA_SYMBOLS to scripts/Makefile.modpost that in turn calls modpost with the correct -E command line arguments.

--- linux-2.6.24-vanilla/scripts/mod/modpost.c 2008-02-27 21:10:24.000000000
+0100
+++ linux-2.6.24/scripts/mod/modpost.c 2008-02-27 21:14:36.000000000 +0100
@@ -1658,8 +1658,12 @@ int main(int argc, char **argv)
char *dump_write = NULL;
int opt;
int err;
+ struct ext_sym_list {
+ struct ext_sym_list *next;
+ const char *file;
+ } *extsym_start = NULL, *extsym_iter;

- while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
+ while ((opt = getopt(argc, argv, "i:I:E:mso:aw")) != -1) {
switch(opt) {
case 'i':
kernel_read = optarg;
@@ -1668,6 +1672,14 @@ int main(int argc, char **argv)
module_read = optarg; external_module = 1; break;
+ case 'E':
+ external_module = 1;
+ extsym_iter =
+ NOFAIL(malloc(sizeof(*extsym_iter)));
+ extsym_iter->next = extsym_start;
+ extsym_iter->file = optarg;
+ extsym_start = extsym_iter;
+ break;
case 'm':
modversions = 1; break;
@@ -1692,6 +1704,12 @@ int main(int argc, char **argv)
read_dump(kernel_read, 1);
if (module_read)
read_dump(module_read, 0);
+ while (extsym_start) {
+ read_dump(extsym_start->file, 0);
+ extsym_iter = extsym_start->next;
+ free(extsym_start);
+ extsym_start = extsym_iter;
+ }

while (optind < argc) {
read_symbols(argv[optind++]);

#1 lore raw

Hi Richard.

I had your first patchqueued up for this and will soon take a look at this update.

At first quick look it looked good and I have only two comments:
1) The variable should be named KBUILD_* to fit the general namespace used
2) The new possibility should be documented in Documentation/kbuild/modules.txt

I will get back when I have given it a closer look.

PS. Please also cc: [email protected] if you submit again.

Sam

Quoted 83 lines · Richard Hacker

This patch adds a new command line option -E to modpost, expecting a symbol file as an argument which is read prior to symbol processing. -E can be supplied multiple times for as many files as is needed.

---
When building kernel modules that depend on other modules not in the main kernel tree, modpost complains about undefined symbols:

# make -C /path/to/linux/kernel M=/path/to/my/module
...
Building modules, stage 2.
....
WARNING: "rt_copy_buf" [/home/rich/osc_etl_rtw/osc_kmod.ko] undefined!
...etc

This situation occurs when modpost processes the new module's symbols. When it finds symbols not exported by the mainline kernel, it issues this warning.

The patch adds a new command line option -E to modpost which expects a symbol file as an argument. The symbols listed in this file are added to modpost's symbol tables during startup. -E can be supplied as often as required.

This patch works together with the second patch. It introduces a new make variable, EXTRA_SYMBOLS, which is used when calling modpost.

I hope this patch is useful and finds its way into the kernel.

o Changes from the previous attempt:
- patches kernel version 2.6.24
- patches for scripts/mod/modpost.c and scripts/Makefile.modpost are separated
- the argument to -E is now a single file, not a comma separated list any more. To pass more than one file, -E is used as often as required
- patch 002/002 in this series introduces the Kbuild variable EXTRA_SYMBOLS to scripts/Makefile.modpost that in turn calls modpost with the correct -E command line arguments.

--- linux-2.6.24-vanilla/scripts/mod/modpost.c 2008-02-27 21:10:24.000000000
+0100
+++ linux-2.6.24/scripts/mod/modpost.c 2008-02-27 21:14:36.000000000 +0100

(quoted diff) +19 -1
@@ -1658,8 +1658,12 @@ int main(int argc, char **argv)
 	char *dump_write = NULL;
 	int opt;
 	int err;
+	struct ext_sym_list {
+		struct ext_sym_list *next;
+		const char *file;
+	} *extsym_start = NULL, *extsym_iter;
 
-	while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
+	while ((opt = getopt(argc, argv, "i:I:E:mso:aw")) != -1) {
 		switch(opt) {
 			case 'i':
 				kernel_read = optarg;
@@ -1668,6 +1672,14 @@ int main(int argc, char **argv)
 				module_read = optarg;
 				external_module = 1;
 				break;
+			case 'E':
+				external_module = 1;
+				extsym_iter =
+					NOFAIL(malloc(sizeof(*extsym_iter)));
+				extsym_iter->next = extsym_start;
+				extsym_iter->file = optarg;
+				extsym_start = extsym_iter;
+				break;
 			case 'm':
 				modversions = 1;
 				break;
@@ -1692,6 +1704,12 @@ int main(int argc, char **argv)
 		read_dump(kernel_read, 1);
 	if (module_read)
 		read_dump(module_read, 0);
+	while (extsym_start) {
+		read_dump(extsym_start->file, 0);
+		extsym_iter = extsym_start->next;
+		free(extsym_start);
+		extsym_start = extsym_iter;
+	}
 
 	while (optind < argc) {
 		read_symbols(argv[optind++]);
#2 lore raw

Hello Sam,

Quoted 2 lines

At first quick look it looked good and I have only two comments:
1) The variable should be named KBUILD_* to fit the general namespace used

I wanted to use the namespace EXTRA_* to continue the line of EXTRA_CFLAGS, etc. that were used when building modules. Is that OK? The other names sound odd to me: KBUILD_SYMBOLS or KBUILD_EXTRA_SYMBOLS. If you really do not like EXTRA_SYMBOLS, which one do you propose?

Quoted 2 lines

2) The new possibility should be documented in
Documentation/kbuild/modules.txt

[PATCH 002/002] in this revised series updated scripts/Makefile.modpost to use this new feature of modpost, and documented the change as well. Please take a look at it: http://lkml.org/lkml/2008/2/28/72

Quoted 1 line

PS. Please also cc: [email protected] if you submit again.

Did it. (For new readers in linux-kbuild: The original patchset can be found at:
http://lkml.org/lkml/2008/2/28/71 for [PATCH 001/002]
http://lkml.org/lkml/2008/2/28/72 for [PATCH 002/002] )

- Richard

#3 lore raw
In reply to Richard Hacker
Quoted 5 lines · Richard Hacker

This patch adds a new command line option -E to modpost, expecting a symbol file as an argument which is read prior to symbol processing. -E can be supplied multiple times for as many files as is needed.

I edited the patches slightly and renamed the variable to KBUILD_EXTRA_SYMBOLS.
Patches has been pushed and will soon appear in mainline.

Thanks,
Sam