| Message ID | 20191029123809.29301-3-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | More nsdeps improvements | expand |
+++ Masahiro Yamada [29/10/19 21:38 +0900]: >The modpost, with the -d option given, generates per-module .ns_deps >files. > >Kbuild generates per-module .mod files to carry module information. >This is convenient because Make handles multiple jobs when the -j >option is given. > >On the other hand, the modpost always runs as a single thread. >I do not see a strong reason to produce separate .ns_deps files. > >This commit changes the modpost to generate just one file, >modules.nsdeps, each line of which has the following format: > > <module_name>: <list of missing namespaces> > >Please note it contains *missing* namespaces instead of required ones. >So, modules.nsdeps is empty if the namespace dependency is all good. > >This will work more efficiently because spatch will no longer process >already imported namespaces. I removed the '(if needed)' from the >nsdeps log since spatch is invoked only when needed. This is a nice optimization! :-) >This also solved the stale .ns_deps files problem reported by >Jessica Yu: > > https://lkml.org/lkml/2019/10/28/467 Tested-by: Jessica Yu <jeyu@kernel.org> Acked-by: Jessica Yu <jeyu@kernel.org> Thanks for the fix! >While I was here, I improved the modpost code a little more; >I freed ns_deps_bus.p because buf_write() allocates memory. > >Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >--- > > .gitignore | 2 +- > Documentation/dontdiff | 1 + > Makefile | 4 ++-- > scripts/Makefile.modpost | 2 +- > scripts/mod/modpost.c | 44 +++++++++++++++++----------------------- > scripts/mod/modpost.h | 4 ++-- > scripts/nsdeps | 21 +++++++++---------- > 7 files changed, 36 insertions(+), 42 deletions(-) > >diff --git a/.gitignore b/.gitignore >index 70580bdd352c..72ef86a5570d 100644 >--- a/.gitignore >+++ b/.gitignore >@@ -32,7 +32,6 @@ > *.lzo > *.mod > *.mod.c >-*.ns_deps > *.o > *.o.* > *.patch >@@ -61,6 +60,7 @@ modules.order > /System.map > /Module.markers > /modules.builtin.modinfo >+/modules.nsdeps > > # > # RPM spec file (make rpm-pkg) >diff --git a/Documentation/dontdiff b/Documentation/dontdiff >index 9f4392876099..72fc2e9e2b63 100644 >--- a/Documentation/dontdiff >+++ b/Documentation/dontdiff >@@ -179,6 +179,7 @@ mkutf8data > modpost > modules.builtin > modules.builtin.modinfo >+modules.nsdeps > modules.order > modversions.h* > nconf >diff --git a/Makefile b/Makefile >index 0ef897fd9cfd..1e3f307bd49b 100644 >--- a/Makefile >+++ b/Makefile >@@ -1356,7 +1356,7 @@ endif # CONFIG_MODULES > > # Directories & files removed with 'make clean' > CLEAN_DIRS += include/ksym >-CLEAN_FILES += modules.builtin.modinfo >+CLEAN_FILES += modules.builtin.modinfo modules.nsdeps > > # Directories & files removed with 'make mrproper' > MRPROPER_DIRS += include/config include/generated \ >@@ -1660,7 +1660,7 @@ clean: $(clean-dirs) > -o -name '*.ko.*' \ > -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ > -o -name '*.dwo' -o -name '*.lst' \ >- -o -name '*.su' -o -name '*.mod' -o -name '*.ns_deps' \ >+ -o -name '*.su' -o -name '*.mod' \ > -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ > -o -name '*.lex.c' -o -name '*.tab.[ch]' \ > -o -name '*.asn1.[ch]' \ >diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost >index c9757b20b048..da37128c3f9f 100644 >--- a/scripts/Makefile.modpost >+++ b/scripts/Makefile.modpost >@@ -66,7 +66,7 @@ __modpost: > else > > MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - \ >- $(if $(KBUILD_NSDEPS),-d) >+ $(if $(KBUILD_NSDEPS),-d modules.nsdeps) > > ifeq ($(KBUILD_EXTMOD),) > MODPOST += $(wildcard vmlinux) >diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c >index dcd90d563ce8..f7425f5d4ab0 100644 >--- a/scripts/mod/modpost.c >+++ b/scripts/mod/modpost.c >@@ -38,8 +38,6 @@ static int sec_mismatch_count = 0; > static int sec_mismatch_fatal = 0; > /* ignore missing files */ > static int ignore_missing_files; >-/* write namespace dependencies */ >-static int write_namespace_deps; > > enum export { > export_plain, export_unused, export_gpl, >@@ -2217,14 +2215,11 @@ static int check_exports(struct module *mod) > else > basename = mod->name; > >- if (exp->namespace) { >- add_namespace(&mod->required_namespaces, >- exp->namespace); >- >- if (!module_imports_namespace(mod, exp->namespace)) { >- warn("module %s uses symbol %s from namespace %s, but does not import it.\n", >- basename, exp->name, exp->namespace); >- } >+ if (exp->namespace && >+ !module_imports_namespace(mod, exp->namespace)) { >+ warn("module %s uses symbol %s from namespace %s, but does not import it.\n", >+ basename, exp->name, exp->namespace); >+ add_namespace(&mod->missing_namespaces, exp->namespace); > } > > if (!mod->gpl_compatible) >@@ -2525,29 +2520,27 @@ static void write_dump(const char *fname) > free(buf.p); > } > >-static void write_namespace_deps_files(void) >+static void write_namespace_deps_files(const char *fname) > { > struct module *mod; > struct namespace_list *ns; > struct buffer ns_deps_buf = {}; > > for (mod = modules; mod; mod = mod->next) { >- char fname[PATH_MAX]; > >- if (mod->skip) >+ if (mod->skip || !mod->missing_namespaces) > continue; > >- ns_deps_buf.pos = 0; >+ buf_printf(&ns_deps_buf, "%s.ko:", mod->name); > >- for (ns = mod->required_namespaces; ns; ns = ns->next) >- buf_printf(&ns_deps_buf, "%s\n", ns->namespace); >+ for (ns = mod->missing_namespaces; ns; ns = ns->next) >+ buf_printf(&ns_deps_buf, " %s", ns->namespace); > >- if (ns_deps_buf.pos == 0) >- continue; >- >- sprintf(fname, "%s.ns_deps", mod->name); >- write_if_changed(&ns_deps_buf, fname); >+ buf_printf(&ns_deps_buf, "\n"); > } >+ >+ write_if_changed(&ns_deps_buf, fname); >+ free(ns_deps_buf.p); > } > > struct ext_sym_list { >@@ -2560,6 +2553,7 @@ int main(int argc, char **argv) > struct module *mod; > struct buffer buf = { }; > char *kernel_read = NULL; >+ char *missing_namespace_deps = NULL; > char *dump_write = NULL, *files_source = NULL; > int opt; > int err; >@@ -2567,7 +2561,7 @@ int main(int argc, char **argv) > struct ext_sym_list *extsym_iter; > struct ext_sym_list *extsym_start = NULL; > >- while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd")) != -1) { >+ while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd:")) != -1) { > switch (opt) { > case 'i': > kernel_read = optarg; >@@ -2606,7 +2600,7 @@ int main(int argc, char **argv) > sec_mismatch_fatal = 1; > break; > case 'd': >- write_namespace_deps = 1; >+ missing_namespace_deps = optarg; > break; > default: > exit(1); >@@ -2654,8 +2648,8 @@ int main(int argc, char **argv) > write_if_changed(&buf, fname); > } > >- if (write_namespace_deps) >- write_namespace_deps_files(); >+ if (missing_namespace_deps) >+ write_namespace_deps_files(missing_namespace_deps); > > if (dump_write) > write_dump(dump_write); >diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h >index ad271bc6c313..fe6652535e4b 100644 >--- a/scripts/mod/modpost.h >+++ b/scripts/mod/modpost.h >@@ -126,8 +126,8 @@ struct module { > struct buffer dev_table_buf; > char srcversion[25]; > int is_dot_o; >- // Required namespace dependencies >- struct namespace_list *required_namespaces; >+ // Missing namespace dependencies >+ struct namespace_list *missing_namespaces; > // Actual imported namespaces > struct namespace_list *imported_namespaces; > }; >diff --git a/scripts/nsdeps b/scripts/nsdeps >index dda6fbac016e..08db427a7fe5 100644 >--- a/scripts/nsdeps >+++ b/scripts/nsdeps >@@ -27,15 +27,14 @@ generate_deps_for_ns() { > } > > generate_deps() { >- local mod_name=`basename $@ .ko` >- local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'` >- local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'` >- if [ ! -f "$ns_deps_file" ]; then return; fi >- local mod_source_files=`cat $mod_file | sed -n 1p \ >+ local mod=${1%.ko:} >+ shift >+ local namespaces="$*" >+ local mod_source_files=`cat $mod.mod | sed -n 1p \ > | sed -e 's/\.o/\.c/g' \ > | sed "s|[^ ]* *|${srctree}/&|g"` >- for ns in `cat $ns_deps_file`; do >- echo "Adding namespace $ns to module $mod_name (if needed)." >+ for ns in $namespaces; do >+ echo "Adding namespace $ns to module $mod.ko." > generate_deps_for_ns $ns $mod_source_files > # sort the imports > for source_file in $mod_source_files; do >@@ -52,7 +51,7 @@ generate_deps() { > done > } > >-for f in `cat $objtree/modules.order`; do >- generate_deps $f >-done >- >+while read line >+do >+ generate_deps $line >+done < modules.nsdeps >-- >2.17.1 >
+++ Masahiro Yamada [29/10/19 21:38 +0900]: >The modpost, with the -d option given, generates per-module .ns_deps >files. > >Kbuild generates per-module .mod files to carry module information. >This is convenient because Make handles multiple jobs when the -j >option is given. > >On the other hand, the modpost always runs as a single thread. >I do not see a strong reason to produce separate .ns_deps files. > >This commit changes the modpost to generate just one file, >modules.nsdeps, each line of which has the following format: > > <module_name>: <list of missing namespaces> > >Please note it contains *missing* namespaces instead of required ones. >So, modules.nsdeps is empty if the namespace dependency is all good. > >This will work more efficiently because spatch will no longer process >already imported namespaces. I removed the '(if needed)' from the >nsdeps log since spatch is invoked only when needed. > >This also solved the stale .ns_deps files problem reported by >Jessica Yu: > > https://lkml.org/lkml/2019/10/28/467 > >While I was here, I improved the modpost code a little more; >I freed ns_deps_bus.p because buf_write() allocates memory. > >Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >--- > > .gitignore | 2 +- > Documentation/dontdiff | 1 + > Makefile | 4 ++-- > scripts/Makefile.modpost | 2 +- > scripts/mod/modpost.c | 44 +++++++++++++++++----------------------- > scripts/mod/modpost.h | 4 ++-- > scripts/nsdeps | 21 +++++++++---------- > 7 files changed, 36 insertions(+), 42 deletions(-) > >diff --git a/.gitignore b/.gitignore >index 70580bdd352c..72ef86a5570d 100644 >--- a/.gitignore >+++ b/.gitignore >@@ -32,7 +32,6 @@ > *.lzo > *.mod > *.mod.c >-*.ns_deps > *.o > *.o.* > *.patch >@@ -61,6 +60,7 @@ modules.order > /System.map > /Module.markers > /modules.builtin.modinfo >+/modules.nsdeps > > # > # RPM spec file (make rpm-pkg) >diff --git a/Documentation/dontdiff b/Documentation/dontdiff >index 9f4392876099..72fc2e9e2b63 100644 >--- a/Documentation/dontdiff >+++ b/Documentation/dontdiff >@@ -179,6 +179,7 @@ mkutf8data > modpost > modules.builtin > modules.builtin.modinfo >+modules.nsdeps > modules.order > modversions.h* > nconf >diff --git a/Makefile b/Makefile >index 0ef897fd9cfd..1e3f307bd49b 100644 >--- a/Makefile >+++ b/Makefile >@@ -1356,7 +1356,7 @@ endif # CONFIG_MODULES > > # Directories & files removed with 'make clean' > CLEAN_DIRS += include/ksym >-CLEAN_FILES += modules.builtin.modinfo >+CLEAN_FILES += modules.builtin.modinfo modules.nsdeps Hmm, I tried to run `make -C path/to/kernel/src M=$(PWD) clean` for a test external module, but it didn't remove modules.nsdeps for me. I declared a MODULE namespace for testing purposes. $ make make -C /dev/shm/linux M=/tmp/ppyu/test-module make[1]: Entering directory '/dev/shm/linux' AR /tmp/ppyu/test-module/built-in.a CC [M] /tmp/ppyu/test-module/test1.o CC [M] /tmp/ppyu/test-module/test2.o LD [M] /tmp/ppyu/test-module/test.o Building modules, stage 2. MODPOST 1 modules WARNING: module test uses symbol try_module_get from namespace MODULE, but does not import it. CC [M] /tmp/ppyu/test-module/test.mod.o LD [M] /tmp/ppyu/test-module/test.ko make[1]: Leaving directory '/dev/shm/linux' Then I make nsdeps: make -C /dev/shm/linux M=/tmp/ppyu/test-module nsdeps make[1]: Entering directory '/dev/shm/linux' Building modules, stage 2. MODPOST 1 modules WARNING: module test uses symbol try_module_get from namespace MODULE, but does not import it. Adding namespace MODULE to module /tmp/ppyu/test-module/test.ko. --- /tmp/ppyu/test-module/test1.c +++ /tmp/cocci-output-3696-c1f8b3-test1.c @@ -38,3 +38,4 @@ static void __exit hello_cleanup(void) module_init(hello_init); module_exit(hello_cleanup); MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(MODULE); make[1]: Leaving directory '/dev/shm/linux' $ cat modules.nsdeps /tmp/ppyu/test-module/test.ko: MODULE Looks good so far, then I try make clean: $ make clean make -C /dev/shm/linux M=/tmp/ppyu/test-module clean make[1]: Entering directory '/dev/shm/linux' CLEAN /tmp/ppyu/test-module/Module.symvers make[1]: Leaving directory '/dev/shm/linux' $ ls Makefile modules.nsdeps test1.c test2.c But modules.nsdeps is still there.
On Thu, Oct 31, 2019 at 8:20 PM Jessica Yu <jeyu@kernel.org> wrote: > > +++ Masahiro Yamada [29/10/19 21:38 +0900]: > >The modpost, with the -d option given, generates per-module .ns_deps > >files. > > > >Kbuild generates per-module .mod files to carry module information. > >This is convenient because Make handles multiple jobs when the -j > >option is given. > > > >On the other hand, the modpost always runs as a single thread. > >I do not see a strong reason to produce separate .ns_deps files. > > > >This commit changes the modpost to generate just one file, > >modules.nsdeps, each line of which has the following format: > > > > <module_name>: <list of missing namespaces> > > > >Please note it contains *missing* namespaces instead of required ones. > >So, modules.nsdeps is empty if the namespace dependency is all good. > > > >This will work more efficiently because spatch will no longer process > >already imported namespaces. I removed the '(if needed)' from the > >nsdeps log since spatch is invoked only when needed. > > > >This also solved the stale .ns_deps files problem reported by > >Jessica Yu: > > > > https://lkml.org/lkml/2019/10/28/467 > > > >While I was here, I improved the modpost code a little more; > >I freed ns_deps_bus.p because buf_write() allocates memory. > > > >Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > >--- > > > > .gitignore | 2 +- > > Documentation/dontdiff | 1 + > > Makefile | 4 ++-- > > scripts/Makefile.modpost | 2 +- > > scripts/mod/modpost.c | 44 +++++++++++++++++----------------------- > > scripts/mod/modpost.h | 4 ++-- > > scripts/nsdeps | 21 +++++++++---------- > > 7 files changed, 36 insertions(+), 42 deletions(-) > > > >diff --git a/.gitignore b/.gitignore > >index 70580bdd352c..72ef86a5570d 100644 > >--- a/.gitignore > >+++ b/.gitignore > >@@ -32,7 +32,6 @@ > > *.lzo > > *.mod > > *.mod.c > >-*.ns_deps > > *.o > > *.o.* > > *.patch > >@@ -61,6 +60,7 @@ modules.order > > /System.map > > /Module.markers > > /modules.builtin.modinfo > >+/modules.nsdeps > > > > # > > # RPM spec file (make rpm-pkg) > >diff --git a/Documentation/dontdiff b/Documentation/dontdiff > >index 9f4392876099..72fc2e9e2b63 100644 > >--- a/Documentation/dontdiff > >+++ b/Documentation/dontdiff > >@@ -179,6 +179,7 @@ mkutf8data > > modpost > > modules.builtin > > modules.builtin.modinfo > >+modules.nsdeps > > modules.order > > modversions.h* > > nconf > >diff --git a/Makefile b/Makefile > >index 0ef897fd9cfd..1e3f307bd49b 100644 > >--- a/Makefile > >+++ b/Makefile > >@@ -1356,7 +1356,7 @@ endif # CONFIG_MODULES > > > > # Directories & files removed with 'make clean' > > CLEAN_DIRS += include/ksym > >-CLEAN_FILES += modules.builtin.modinfo > >+CLEAN_FILES += modules.builtin.modinfo modules.nsdeps > > Hmm, I tried to run `make -C path/to/kernel/src M=$(PWD) clean` for a test > external module, but it didn't remove modules.nsdeps for me. I declared a > MODULE namespace for testing purposes. > > $ make > make -C /dev/shm/linux M=/tmp/ppyu/test-module > make[1]: Entering directory '/dev/shm/linux' > AR /tmp/ppyu/test-module/built-in.a > CC [M] /tmp/ppyu/test-module/test1.o > CC [M] /tmp/ppyu/test-module/test2.o > LD [M] /tmp/ppyu/test-module/test.o > Building modules, stage 2. > MODPOST 1 modules > WARNING: module test uses symbol try_module_get from namespace MODULE, but does not import it. > CC [M] /tmp/ppyu/test-module/test.mod.o > LD [M] /tmp/ppyu/test-module/test.ko > make[1]: Leaving directory '/dev/shm/linux' > > Then I make nsdeps: > > make -C /dev/shm/linux M=/tmp/ppyu/test-module nsdeps > make[1]: Entering directory '/dev/shm/linux' > Building modules, stage 2. > MODPOST 1 modules > WARNING: module test uses symbol try_module_get from namespace MODULE, but does not import it. > Adding namespace MODULE to module /tmp/ppyu/test-module/test.ko. > --- /tmp/ppyu/test-module/test1.c > +++ /tmp/cocci-output-3696-c1f8b3-test1.c > @@ -38,3 +38,4 @@ static void __exit hello_cleanup(void) > module_init(hello_init); > module_exit(hello_cleanup); > MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS(MODULE); > make[1]: Leaving directory '/dev/shm/linux' > $ cat modules.nsdeps > /tmp/ppyu/test-module/test.ko: MODULE > > Looks good so far, then I try make clean: > > $ make clean > make -C /dev/shm/linux M=/tmp/ppyu/test-module clean > make[1]: Entering directory '/dev/shm/linux' > CLEAN /tmp/ppyu/test-module/Module.symvers > make[1]: Leaving directory '/dev/shm/linux' > $ ls > Makefile modules.nsdeps test1.c test2.c > > But modules.nsdeps is still there. > Good catch! I forgot to take care of it for external module builds. The following should work. I will fold it in 3/4. diff --git a/Makefile b/Makefile index 79be70bf2899..6035702803eb 100644 --- a/Makefile +++ b/Makefile @@ -1619,7 +1619,7 @@ _emodinst_post: _emodinst_ $(call cmd,depmod) clean-dirs := $(KBUILD_EXTMOD) -clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers +clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers $(KBUILD_EXTMOD)/modules.nsdeps PHONY += / /:
On Thu, Oct 31, 2019 at 08:53:25PM +0900, Masahiro Yamada wrote: >On Thu, Oct 31, 2019 at 8:20 PM Jessica Yu <jeyu@kernel.org> wrote: >> >> +++ Masahiro Yamada [29/10/19 21:38 +0900]: >> >The modpost, with the -d option given, generates per-module .ns_deps >> >files. >> > >> >Kbuild generates per-module .mod files to carry module information. >> >This is convenient because Make handles multiple jobs when the -j >> >option is given. >> > >> >On the other hand, the modpost always runs as a single thread. >> >I do not see a strong reason to produce separate .ns_deps files. >> > >> >This commit changes the modpost to generate just one file, >> >modules.nsdeps, each line of which has the following format: >> > >> > <module_name>: <list of missing namespaces> >> > >> >Please note it contains *missing* namespaces instead of required ones. >> >So, modules.nsdeps is empty if the namespace dependency is all good. >> > >> >This will work more efficiently because spatch will no longer process >> >already imported namespaces. I removed the '(if needed)' from the >> >nsdeps log since spatch is invoked only when needed. >> > >> >This also solved the stale .ns_deps files problem reported by >> >Jessica Yu: >> > >> > https://lkml.org/lkml/2019/10/28/467 >> > >> >While I was here, I improved the modpost code a little more; >> >I freed ns_deps_bus.p because buf_write() allocates memory. >> > >> >Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> >--- >> > >> > .gitignore | 2 +- >> > Documentation/dontdiff | 1 + >> > Makefile | 4 ++-- >> > scripts/Makefile.modpost | 2 +- >> > scripts/mod/modpost.c | 44 +++++++++++++++++----------------------- >> > scripts/mod/modpost.h | 4 ++-- >> > scripts/nsdeps | 21 +++++++++---------- >> > 7 files changed, 36 insertions(+), 42 deletions(-) >> > >> >diff --git a/.gitignore b/.gitignore >> >index 70580bdd352c..72ef86a5570d 100644 >> >--- a/.gitignore >> >+++ b/.gitignore >> >@@ -32,7 +32,6 @@ >> > *.lzo >> > *.mod >> > *.mod.c >> >-*.ns_deps >> > *.o >> > *.o.* >> > *.patch >> >@@ -61,6 +60,7 @@ modules.order >> > /System.map >> > /Module.markers >> > /modules.builtin.modinfo >> >+/modules.nsdeps >> > >> > # >> > # RPM spec file (make rpm-pkg) >> >diff --git a/Documentation/dontdiff b/Documentation/dontdiff >> >index 9f4392876099..72fc2e9e2b63 100644 >> >--- a/Documentation/dontdiff >> >+++ b/Documentation/dontdiff >> >@@ -179,6 +179,7 @@ mkutf8data >> > modpost >> > modules.builtin >> > modules.builtin.modinfo >> >+modules.nsdeps >> > modules.order >> > modversions.h* >> > nconf >> >diff --git a/Makefile b/Makefile >> >index 0ef897fd9cfd..1e3f307bd49b 100644 >> >--- a/Makefile >> >+++ b/Makefile >> >@@ -1356,7 +1356,7 @@ endif # CONFIG_MODULES >> > >> > # Directories & files removed with 'make clean' >> > CLEAN_DIRS += include/ksym >> >-CLEAN_FILES += modules.builtin.modinfo >> >+CLEAN_FILES += modules.builtin.modinfo modules.nsdeps >> >> Hmm, I tried to run `make -C path/to/kernel/src M=$(PWD) clean` for a test >> external module, but it didn't remove modules.nsdeps for me. I declared a >> MODULE namespace for testing purposes. >> >> $ make >> make -C /dev/shm/linux M=/tmp/ppyu/test-module >> make[1]: Entering directory '/dev/shm/linux' >> AR /tmp/ppyu/test-module/built-in.a >> CC [M] /tmp/ppyu/test-module/test1.o >> CC [M] /tmp/ppyu/test-module/test2.o >> LD [M] /tmp/ppyu/test-module/test.o >> Building modules, stage 2. >> MODPOST 1 modules >> WARNING: module test uses symbol try_module_get from namespace MODULE, but does not import it. >> CC [M] /tmp/ppyu/test-module/test.mod.o >> LD [M] /tmp/ppyu/test-module/test.ko >> make[1]: Leaving directory '/dev/shm/linux' >> >> Then I make nsdeps: >> >> make -C /dev/shm/linux M=/tmp/ppyu/test-module nsdeps >> make[1]: Entering directory '/dev/shm/linux' >> Building modules, stage 2. >> MODPOST 1 modules >> WARNING: module test uses symbol try_module_get from namespace MODULE, but does not import it. >> Adding namespace MODULE to module /tmp/ppyu/test-module/test.ko. >> --- /tmp/ppyu/test-module/test1.c >> +++ /tmp/cocci-output-3696-c1f8b3-test1.c >> @@ -38,3 +38,4 @@ static void __exit hello_cleanup(void) >> module_init(hello_init); >> module_exit(hello_cleanup); >> MODULE_LICENSE("GPL"); >> +MODULE_IMPORT_NS(MODULE); >> make[1]: Leaving directory '/dev/shm/linux' >> $ cat modules.nsdeps >> /tmp/ppyu/test-module/test.ko: MODULE >> >> Looks good so far, then I try make clean: >> >> $ make clean >> make -C /dev/shm/linux M=/tmp/ppyu/test-module clean >> make[1]: Entering directory '/dev/shm/linux' >> CLEAN /tmp/ppyu/test-module/Module.symvers >> make[1]: Leaving directory '/dev/shm/linux' >> $ ls >> Makefile modules.nsdeps test1.c test2.c >> >> But modules.nsdeps is still there. >> > >Good catch! > >I forgot to take care of it for external module builds. > >The following should work. I will fold it in 3/4. > > > > >diff --git a/Makefile b/Makefile >index 79be70bf2899..6035702803eb 100644 >--- a/Makefile >+++ b/Makefile >@@ -1619,7 +1619,7 @@ _emodinst_post: _emodinst_ > $(call cmd,depmod) > > clean-dirs := $(KBUILD_EXTMOD) >-clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers >+clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers $(KBUILD_EXTMOD)/modules.nsdeps Reviewed-by: Matthias Maennich <maennich@google.com> Tested-by: Matthias Maennich <maennich@google.com> Thanks for this improvement! Cheers, Matthias > > PHONY += / > /: > > > >-- >Best Regards >Masahiro Yamada
diff --git a/.gitignore b/.gitignore index 70580bdd352c..72ef86a5570d 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,6 @@ *.lzo *.mod *.mod.c -*.ns_deps *.o *.o.* *.patch @@ -61,6 +60,7 @@ modules.order /System.map /Module.markers /modules.builtin.modinfo +/modules.nsdeps # # RPM spec file (make rpm-pkg) diff --git a/Documentation/dontdiff b/Documentation/dontdiff index 9f4392876099..72fc2e9e2b63 100644 --- a/Documentation/dontdiff +++ b/Documentation/dontdiff @@ -179,6 +179,7 @@ mkutf8data modpost modules.builtin modules.builtin.modinfo +modules.nsdeps modules.order modversions.h* nconf diff --git a/Makefile b/Makefile index 0ef897fd9cfd..1e3f307bd49b 100644 --- a/Makefile +++ b/Makefile @@ -1356,7 +1356,7 @@ endif # CONFIG_MODULES # Directories & files removed with 'make clean' CLEAN_DIRS += include/ksym -CLEAN_FILES += modules.builtin.modinfo +CLEAN_FILES += modules.builtin.modinfo modules.nsdeps # Directories & files removed with 'make mrproper' MRPROPER_DIRS += include/config include/generated \ @@ -1660,7 +1660,7 @@ clean: $(clean-dirs) -o -name '*.ko.*' \ -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ -o -name '*.dwo' -o -name '*.lst' \ - -o -name '*.su' -o -name '*.mod' -o -name '*.ns_deps' \ + -o -name '*.su' -o -name '*.mod' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ -o -name '*.lex.c' -o -name '*.tab.[ch]' \ -o -name '*.asn1.[ch]' \ diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index c9757b20b048..da37128c3f9f 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -66,7 +66,7 @@ __modpost: else MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - \ - $(if $(KBUILD_NSDEPS),-d) + $(if $(KBUILD_NSDEPS),-d modules.nsdeps) ifeq ($(KBUILD_EXTMOD),) MODPOST += $(wildcard vmlinux) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index dcd90d563ce8..f7425f5d4ab0 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -38,8 +38,6 @@ static int sec_mismatch_count = 0; static int sec_mismatch_fatal = 0; /* ignore missing files */ static int ignore_missing_files; -/* write namespace dependencies */ -static int write_namespace_deps; enum export { export_plain, export_unused, export_gpl, @@ -2217,14 +2215,11 @@ static int check_exports(struct module *mod) else basename = mod->name; - if (exp->namespace) { - add_namespace(&mod->required_namespaces, - exp->namespace); - - if (!module_imports_namespace(mod, exp->namespace)) { - warn("module %s uses symbol %s from namespace %s, but does not import it.\n", - basename, exp->name, exp->namespace); - } + if (exp->namespace && + !module_imports_namespace(mod, exp->namespace)) { + warn("module %s uses symbol %s from namespace %s, but does not import it.\n", + basename, exp->name, exp->namespace); + add_namespace(&mod->missing_namespaces, exp->namespace); } if (!mod->gpl_compatible) @@ -2525,29 +2520,27 @@ static void write_dump(const char *fname) free(buf.p); } -static void write_namespace_deps_files(void) +static void write_namespace_deps_files(const char *fname) { struct module *mod; struct namespace_list *ns; struct buffer ns_deps_buf = {}; for (mod = modules; mod; mod = mod->next) { - char fname[PATH_MAX]; - if (mod->skip) + if (mod->skip || !mod->missing_namespaces) continue; - ns_deps_buf.pos = 0; + buf_printf(&ns_deps_buf, "%s.ko:", mod->name); - for (ns = mod->required_namespaces; ns; ns = ns->next) - buf_printf(&ns_deps_buf, "%s\n", ns->namespace); + for (ns = mod->missing_namespaces; ns; ns = ns->next) + buf_printf(&ns_deps_buf, " %s", ns->namespace); - if (ns_deps_buf.pos == 0) - continue; - - sprintf(fname, "%s.ns_deps", mod->name); - write_if_changed(&ns_deps_buf, fname); + buf_printf(&ns_deps_buf, "\n"); } + + write_if_changed(&ns_deps_buf, fname); + free(ns_deps_buf.p); } struct ext_sym_list { @@ -2560,6 +2553,7 @@ int main(int argc, char **argv) struct module *mod; struct buffer buf = { }; char *kernel_read = NULL; + char *missing_namespace_deps = NULL; char *dump_write = NULL, *files_source = NULL; int opt; int err; @@ -2567,7 +2561,7 @@ int main(int argc, char **argv) struct ext_sym_list *extsym_iter; struct ext_sym_list *extsym_start = NULL; - while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd")) != -1) { + while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd:")) != -1) { switch (opt) { case 'i': kernel_read = optarg; @@ -2606,7 +2600,7 @@ int main(int argc, char **argv) sec_mismatch_fatal = 1; break; case 'd': - write_namespace_deps = 1; + missing_namespace_deps = optarg; break; default: exit(1); @@ -2654,8 +2648,8 @@ int main(int argc, char **argv) write_if_changed(&buf, fname); } - if (write_namespace_deps) - write_namespace_deps_files(); + if (missing_namespace_deps) + write_namespace_deps_files(missing_namespace_deps); if (dump_write) write_dump(dump_write); diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index ad271bc6c313..fe6652535e4b 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -126,8 +126,8 @@ struct module { struct buffer dev_table_buf; char srcversion[25]; int is_dot_o; - // Required namespace dependencies - struct namespace_list *required_namespaces; + // Missing namespace dependencies + struct namespace_list *missing_namespaces; // Actual imported namespaces struct namespace_list *imported_namespaces; }; diff --git a/scripts/nsdeps b/scripts/nsdeps index dda6fbac016e..08db427a7fe5 100644 --- a/scripts/nsdeps +++ b/scripts/nsdeps @@ -27,15 +27,14 @@ generate_deps_for_ns() { } generate_deps() { - local mod_name=`basename $@ .ko` - local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'` - local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'` - if [ ! -f "$ns_deps_file" ]; then return; fi - local mod_source_files=`cat $mod_file | sed -n 1p \ + local mod=${1%.ko:} + shift + local namespaces="$*" + local mod_source_files=`cat $mod.mod | sed -n 1p \ | sed -e 's/\.o/\.c/g' \ | sed "s|[^ ]* *|${srctree}/&|g"` - for ns in `cat $ns_deps_file`; do - echo "Adding namespace $ns to module $mod_name (if needed)." + for ns in $namespaces; do + echo "Adding namespace $ns to module $mod.ko." generate_deps_for_ns $ns $mod_source_files # sort the imports for source_file in $mod_source_files; do @@ -52,7 +51,7 @@ generate_deps() { done } -for f in `cat $objtree/modules.order`; do - generate_deps $f -done - +while read line +do + generate_deps $line +done < modules.nsdeps
The modpost, with the -d option given, generates per-module .ns_deps files. Kbuild generates per-module .mod files to carry module information. This is convenient because Make handles multiple jobs when the -j option is given. On the other hand, the modpost always runs as a single thread. I do not see a strong reason to produce separate .ns_deps files. This commit changes the modpost to generate just one file, modules.nsdeps, each line of which has the following format: <module_name>: <list of missing namespaces> Please note it contains *missing* namespaces instead of required ones. So, modules.nsdeps is empty if the namespace dependency is all good. This will work more efficiently because spatch will no longer process already imported namespaces. I removed the '(if needed)' from the nsdeps log since spatch is invoked only when needed. This also solved the stale .ns_deps files problem reported by Jessica Yu: https://lkml.org/lkml/2019/10/28/467 While I was here, I improved the modpost code a little more; I freed ns_deps_bus.p because buf_write() allocates memory. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- .gitignore | 2 +- Documentation/dontdiff | 1 + Makefile | 4 ++-- scripts/Makefile.modpost | 2 +- scripts/mod/modpost.c | 44 +++++++++++++++++----------------------- scripts/mod/modpost.h | 4 ++-- scripts/nsdeps | 21 +++++++++---------- 7 files changed, 36 insertions(+), 42 deletions(-)