[PATCH 0/4] More nsdeps improvements

[PATCH 0/4] More nsdeps improvements

[Masahiro Yamada]

This series improves nsdeps more by addressing the root problem.

Based on linux-next.

This series does NOT apply to the Linus tree because
I updates modpost from my Kbuild tree.

I can pick up this series to kbuild tree if there is no objection.
If it goes to the module tree, we need to discuss how to deal with
the conflicts.

The modpost is actively touched these days
from both kbuild and module trees.



Masahiro Yamada (4):
  modpost: do not invoke extra modpost for nsdeps
  modpost: dump missing namespaces into a single modules.nsdeps file
  scripts/nsdeps: support nsdeps for external module builds
  mospost: remove unneeded local variable in contains_namespace()

 .gitignore                                   |  2 +-
 Documentation/core-api/symbol-namespaces.rst |  3 ++
 Documentation/dontdiff                       |  1 +
 Makefile                                     | 10 ++--
 scripts/Makefile.modpost                     |  8 ++-
 scripts/mod/modpost.c                        | 55 ++++++++------------
 scripts/mod/modpost.h                        |  4 +-
 scripts/nsdeps                               | 29 ++++++-----
 8 files changed, 53 insertions(+), 59 deletions(-)

-- 
2.17.1

[PATCH 1/4] modpost: do not invoke extra modpost for nsdeps

[Masahiro Yamada]

'make nsdeps' invokes the modpost three times at most; before linking
vmlinux, before building modules, and finally for generating .ns_deps
files. Running the modpost again and again is annoying.

The last two can be unified. When the -d option is given, the modpost
still does the usual job, and in addition, generates .ns_deps files.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile                 | 5 ++---
 scripts/Makefile.modpost | 8 +++-----
 scripts/mod/modpost.c    | 9 ++-------
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 4f4d8979bfce..0ef897fd9cfd 100644
--- a/Makefile
+++ b/Makefile
@@ -1682,10 +1682,9 @@ tags TAGS cscope gtags: FORCE
 # ---------------------------------------------------------------------------
 
 PHONY += nsdeps
-
+nsdeps: export KBUILD_NSDEPS=1
 nsdeps: modules
-	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
 
 # Scripts to check various things for consistency
 # ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 01c0a992d293..c9757b20b048 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -53,8 +53,7 @@ MODPOST = scripts/mod/modpost						\
 	$(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS)))	\
 	$(if $(KBUILD_EXTMOD),-o $(modulesymfile))			\
 	$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)			\
-	$(if $(KBUILD_MODPOST_WARN),-w)					\
-	$(if $(filter nsdeps,$(MAKECMDGOALS)),-d)
+	$(if $(KBUILD_MODPOST_WARN),-w)
 
 ifdef MODPOST_VMLINUX
 
@@ -66,7 +65,8 @@ __modpost:
 
 else
 
-MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T -
+MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - \
+	$(if $(KBUILD_NSDEPS),-d)
 
 ifeq ($(KBUILD_EXTMOD),)
 MODPOST += $(wildcard vmlinux)
@@ -97,8 +97,6 @@ ifneq ($(KBUILD_MODPOST_NOFINAL),1)
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
 endif
 
-nsdeps: __modpost
-
 endif
 
 .PHONY: $(PHONY)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 26ba97245576..dcd90d563ce8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2221,8 +2221,7 @@ static int check_exports(struct module *mod)
 			add_namespace(&mod->required_namespaces,
 				      exp->namespace);
 
-			if (!write_namespace_deps &&
-			    !module_imports_namespace(mod, 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);
 			}
@@ -2641,8 +2640,6 @@ int main(int argc, char **argv)
 
 		err |= check_modname_len(mod);
 		err |= check_exports(mod);
-		if (write_namespace_deps)
-			continue;
 
 		add_header(&buf, mod);
 		add_intree_flag(&buf, !external_module);
@@ -2657,10 +2654,8 @@ int main(int argc, char **argv)
 		write_if_changed(&buf, fname);
 	}
 
-	if (write_namespace_deps) {
+	if (write_namespace_deps)
 		write_namespace_deps_files();
-		return 0;
-	}
 
 	if (dump_write)
 		write_dump(dump_write);
-- 
2.17.1

[PATCH 2/4] modpost: dump missing namespaces into a single modules.nsdeps file

[Masahiro Yamada]

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
 
 # 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

[PATCH 3/4] scripts/nsdeps: support nsdeps for external module builds

[Masahiro Yamada]

Apparently, scripts/nsdeps is written to take care of only in-tree
modules. Perhaps, this is not a bug, but just a design. At least,
Documentation/core-api/symbol-namespaces.rst focuses on in-tree modules:

  Again, `make nsdeps` will eventually add the missing namespace imports for
  in-tree modules::
  ^^^^^^^

Having said that, I already saw at least two people trying nsdeps for
external module builds. So, it would be nice to support it.

Reported-by: Steve French <smfrench@gmail.com>
Reported-by: Jessica Yu <jeyu@kernel.org>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Documentation/core-api/symbol-namespaces.rst |  3 +++
 Makefile                                     |  1 +
 scripts/Makefile.modpost                     |  2 +-
 scripts/nsdeps                               | 10 ++++++++--
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index 982ed7b568ac..9b76337f6756 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -152,3 +152,6 @@ in-tree modules::
 	- notice the warning of modpost telling about a missing import
 	- run `make nsdeps` to add the import to the correct code location
 
+You can also run nsdeps for external module builds. A typical usage is::
+
+	$ make -C <path_to_kernel_src> M=$PWD nsdeps
diff --git a/Makefile b/Makefile
index 1e3f307bd49b..780a65493866 100644
--- a/Makefile
+++ b/Makefile
@@ -1007,6 +1007,7 @@ endif
 PHONY += prepare0
 
 export MODORDER := $(extmod-prefix)modules.order
+export MODULES_NSDEPS := $(extmod-prefix)modules.nsdeps
 
 ifeq ($(KBUILD_EXTMOD),)
 core-y		+= kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index da37128c3f9f..8359f8af5ee6 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 modules.nsdeps)
+	$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))
 
 ifeq ($(KBUILD_EXTMOD),)
 MODPOST += $(wildcard vmlinux)
diff --git a/scripts/nsdeps b/scripts/nsdeps
index 08db427a7fe5..3b8a9e173ebf 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -21,6 +21,12 @@ if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then
 	exit 1
 fi
 
+if [ "$KBUILD_EXTMOD" ]; then
+	src_prefix=
+else
+	src_prefix=$srctree/
+fi
+
 generate_deps_for_ns() {
 	$SPATCH --very-quiet --in-place --sp-file \
 		$srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2
@@ -32,7 +38,7 @@ generate_deps() {
 	local namespaces="$*"
 	local mod_source_files=`cat $mod.mod | sed -n 1p                      \
 					      | sed -e 's/\.o/\.c/g'           \
-					      | sed "s|[^ ]* *|${srctree}/&|g"`
+					     | sed "s|[^ ]* *|${src_prefix}&|g"`
 	for ns in $namespaces; do
 		echo "Adding namespace $ns to module $mod.ko."
 		generate_deps_for_ns $ns $mod_source_files
@@ -54,4 +60,4 @@ generate_deps() {
 while read line
 do
 	generate_deps $line
-done < modules.nsdeps
+done < $MODULES_NSDEPS
-- 
2.17.1

[PATCH 4/4] mospost: remove unneeded local variable in contains_namespace()

[Masahiro Yamada]

The local variable, ns_entry, is unneeded.

While I was here, I also cleaned up the comparison with NULL or 0.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 scripts/mod/modpost.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f7425f5d4ab0..f70b924f379f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -239,10 +239,8 @@ static struct symbol *find_symbol(const char *name)
 static bool contains_namespace(struct namespace_list *list,
 			       const char *namespace)
 {
-	struct namespace_list *ns_entry;
-
-	for (ns_entry = list; ns_entry != NULL; ns_entry = ns_entry->next)
-		if (strcmp(ns_entry->namespace, namespace) == 0)
+	for (; list; list = list->next)
+		if (!strcmp(list->namespace, namespace))
 			return true;
 
 	return false;
-- 
2.17.1

Re: [PATCH 3/4] scripts/nsdeps: support nsdeps for external module builds

[Jessica Yu]

+++ Masahiro Yamada [29/10/19 21:38 +0900]:
>Apparently, scripts/nsdeps is written to take care of only in-tree
>modules. Perhaps, this is not a bug, but just a design. At least,
>Documentation/core-api/symbol-namespaces.rst focuses on in-tree modules:
>
>  Again, `make nsdeps` will eventually add the missing namespace imports for
>  in-tree modules::
>  ^^^^^^^
>
>Having said that, I already saw at least two people trying nsdeps for
>external module builds. So, it would be nice to support it.
>
>Reported-by: Steve French <smfrench@gmail.com>
>Reported-by: Jessica Yu <jeyu@kernel.org>
>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

I can confirm that this fixes nsdeps for external modules for me.

Tested-by: Jessica Yu <jeyu@kernel.org>
Acked-by: Jessica Yu <jeyu@kernel.org>

Thanks!

>---
>
> Documentation/core-api/symbol-namespaces.rst |  3 +++
> Makefile                                     |  1 +
> scripts/Makefile.modpost                     |  2 +-
> scripts/nsdeps                               | 10 ++++++++--
> 4 files changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
>index 982ed7b568ac..9b76337f6756 100644
>--- a/Documentation/core-api/symbol-namespaces.rst
>+++ b/Documentation/core-api/symbol-namespaces.rst
>@@ -152,3 +152,6 @@ in-tree modules::
> 	- notice the warning of modpost telling about a missing import
> 	- run `make nsdeps` to add the import to the correct code location
>
>+You can also run nsdeps for external module builds. A typical usage is::
>+
>+	$ make -C <path_to_kernel_src> M=$PWD nsdeps
>diff --git a/Makefile b/Makefile
>index 1e3f307bd49b..780a65493866 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -1007,6 +1007,7 @@ endif
> PHONY += prepare0
>
> export MODORDER := $(extmod-prefix)modules.order
>+export MODULES_NSDEPS := $(extmod-prefix)modules.nsdeps
>
> ifeq ($(KBUILD_EXTMOD),)
> core-y		+= kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
>diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
>index da37128c3f9f..8359f8af5ee6 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 modules.nsdeps)
>+	$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))
>
> ifeq ($(KBUILD_EXTMOD),)
> MODPOST += $(wildcard vmlinux)
>diff --git a/scripts/nsdeps b/scripts/nsdeps
>index 08db427a7fe5..3b8a9e173ebf 100644
>--- a/scripts/nsdeps
>+++ b/scripts/nsdeps
>@@ -21,6 +21,12 @@ if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then
> 	exit 1
> fi
>
>+if [ "$KBUILD_EXTMOD" ]; then
>+	src_prefix=
>+else
>+	src_prefix=$srctree/
>+fi
>+
> generate_deps_for_ns() {
> 	$SPATCH --very-quiet --in-place --sp-file \
> 		$srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2
>@@ -32,7 +38,7 @@ generate_deps() {
> 	local namespaces="$*"
> 	local mod_source_files=`cat $mod.mod | sed -n 1p                      \
> 					      | sed -e 's/\.o/\.c/g'           \
>-					      | sed "s|[^ ]* *|${srctree}/&|g"`
>+					     | sed "s|[^ ]* *|${src_prefix}&|g"`
> 	for ns in $namespaces; do
> 		echo "Adding namespace $ns to module $mod.ko."
> 		generate_deps_for_ns $ns $mod_source_files
>@@ -54,4 +60,4 @@ generate_deps() {
> while read line
> do
> 	generate_deps $line
>-done < modules.nsdeps
>+done < $MODULES_NSDEPS
>-- 
>2.17.1
>

Re: [PATCH 2/4] modpost: dump missing namespaces into a single modules.nsdeps file

[Jessica Yu]

+++ 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
>

Re: [PATCH 0/4] More nsdeps improvements

[Jessica Yu]

+++ Masahiro Yamada [29/10/19 21:38 +0900]:
>This series improves nsdeps more by addressing the root problem.
>
>Based on linux-next.
>
>This series does NOT apply to the Linus tree because
>I updates modpost from my Kbuild tree.
>
>I can pick up this series to kbuild tree if there is no objection.
>If it goes to the module tree, we need to discuss how to deal with
>the conflicts.

Please feel free to take these through the kbuild tree. Thanks!

>The modpost is actively touched these days
>from both kbuild and module trees.
>
>
>
>Masahiro Yamada (4):
>  modpost: do not invoke extra modpost for nsdeps
>  modpost: dump missing namespaces into a single modules.nsdeps file
>  scripts/nsdeps: support nsdeps for external module builds
>  mospost: remove unneeded local variable in contains_namespace()
>
> .gitignore                                   |  2 +-
> Documentation/core-api/symbol-namespaces.rst |  3 ++
> Documentation/dontdiff                       |  1 +
> Makefile                                     | 10 ++--
> scripts/Makefile.modpost                     |  8 ++-
> scripts/mod/modpost.c                        | 55 ++++++++------------
> scripts/mod/modpost.h                        |  4 +-
> scripts/nsdeps                               | 29 ++++++-----
> 8 files changed, 53 insertions(+), 59 deletions(-)
>
>-- 
>2.17.1
>

Re: [PATCH 2/4] modpost: dump missing namespaces into a single modules.nsdeps file

[Jessica Yu]

+++ 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.

Re: [PATCH 2/4] modpost: dump missing namespaces into a single modules.nsdeps file

[Masahiro Yamada]

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 += /
 /:



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 4/4] mospost: remove unneeded local variable in contains_namespace()

[Matthias Maennich]

Hi!

On Tue, Oct 29, 2019 at 09:38:09PM +0900, Masahiro Yamada wrote:
>The local variable, ns_entry, is unneeded.
>
>While I was here, I also cleaned up the comparison with NULL or 0.
>
>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>---

This was not directly sent to me, hence I missed it.
Nevertheless, please feel free to add

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias
>
> scripts/mod/modpost.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index f7425f5d4ab0..f70b924f379f 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -239,10 +239,8 @@ static struct symbol *find_symbol(const char *name)
> static bool contains_namespace(struct namespace_list *list,
> 			       const char *namespace)
> {
>-	struct namespace_list *ns_entry;
>-
>-	for (ns_entry = list; ns_entry != NULL; ns_entry = ns_entry->next)
>-		if (strcmp(ns_entry->namespace, namespace) == 0)
>+	for (; list; list = list->next)
>+		if (!strcmp(list->namespace, namespace))
> 			return true;
>
> 	return false;
>-- 
>2.17.1
>

Re: [PATCH 1/4] modpost: do not invoke extra modpost for nsdeps

[Matthias Maennich]

On Tue, Oct 29, 2019 at 09:38:06PM +0900, Masahiro Yamada wrote:
>'make nsdeps' invokes the modpost three times at most; before linking
>vmlinux, before building modules, and finally for generating .ns_deps
>files. Running the modpost again and again is annoying.
>
>The last two can be unified. When the -d option is given, the modpost
>still does the usual job, and in addition, generates .ns_deps files.
>
>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>---

That is a very nice simplification of `make nsdeps`. Thanks!

Tested-by: Matthias Maennich <maennich@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>
> Makefile                 | 5 ++---
> scripts/Makefile.modpost | 8 +++-----
> scripts/mod/modpost.c    | 9 ++-------
> 3 files changed, 7 insertions(+), 15 deletions(-)
>
>diff --git a/Makefile b/Makefile
>index 4f4d8979bfce..0ef897fd9cfd 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -1682,10 +1682,9 @@ tags TAGS cscope gtags: FORCE
> # ---------------------------------------------------------------------------
>
> PHONY += nsdeps
>-
>+nsdeps: export KBUILD_NSDEPS=1
> nsdeps: modules
>-	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps
>-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@
>+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
>
> # Scripts to check various things for consistency
> # ---------------------------------------------------------------------------
>diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
>index 01c0a992d293..c9757b20b048 100644
>--- a/scripts/Makefile.modpost
>+++ b/scripts/Makefile.modpost
>@@ -53,8 +53,7 @@ MODPOST = scripts/mod/modpost						\
> 	$(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS)))	\
> 	$(if $(KBUILD_EXTMOD),-o $(modulesymfile))			\
> 	$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)			\
>-	$(if $(KBUILD_MODPOST_WARN),-w)					\
>-	$(if $(filter nsdeps,$(MAKECMDGOALS)),-d)
>+	$(if $(KBUILD_MODPOST_WARN),-w)
>
> ifdef MODPOST_VMLINUX
>
>@@ -66,7 +65,8 @@ __modpost:
>
> else
>
>-MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T -
>+MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - \
>+	$(if $(KBUILD_NSDEPS),-d)
>
> ifeq ($(KBUILD_EXTMOD),)
> MODPOST += $(wildcard vmlinux)
>@@ -97,8 +97,6 @@ ifneq ($(KBUILD_MODPOST_NOFINAL),1)
> 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
> endif
>
>-nsdeps: __modpost
>-
> endif
>
> .PHONY: $(PHONY)
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index 26ba97245576..dcd90d563ce8 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -2221,8 +2221,7 @@ static int check_exports(struct module *mod)
> 			add_namespace(&mod->required_namespaces,
> 				      exp->namespace);
>
>-			if (!write_namespace_deps &&
>-			    !module_imports_namespace(mod, 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);
> 			}
>@@ -2641,8 +2640,6 @@ int main(int argc, char **argv)
>
> 		err |= check_modname_len(mod);
> 		err |= check_exports(mod);
>-		if (write_namespace_deps)
>-			continue;
>
> 		add_header(&buf, mod);
> 		add_intree_flag(&buf, !external_module);
>@@ -2657,10 +2654,8 @@ int main(int argc, char **argv)
> 		write_if_changed(&buf, fname);
> 	}
>
>-	if (write_namespace_deps) {
>+	if (write_namespace_deps)
> 		write_namespace_deps_files();
>-		return 0;
>-	}
>
> 	if (dump_write)
> 		write_dump(dump_write);
>-- 
>2.17.1
>

Re: [PATCH 2/4] modpost: dump missing namespaces into a single modules.nsdeps file

[Matthias Maennich]

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

Re: [PATCH 4/4] mospost: remove unneeded local variable in contains_namespace()

[Masahiro Yamada]

On Tue, Oct 29, 2019 at 9:38 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> The local variable, ns_entry, is unneeded.
>
> While I was here, I also cleaned up the comparison with NULL or 0.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

Fixed the typo in the subject ( mospost -> modpost )
and applied to linux-kbuild.


>
>  scripts/mod/modpost.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index f7425f5d4ab0..f70b924f379f 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -239,10 +239,8 @@ static struct symbol *find_symbol(const char *name)
>  static bool contains_namespace(struct namespace_list *list,
>                                const char *namespace)
>  {
> -       struct namespace_list *ns_entry;
> -
> -       for (ns_entry = list; ns_entry != NULL; ns_entry = ns_entry->next)
> -               if (strcmp(ns_entry->namespace, namespace) == 0)
> +       for (; list; list = list->next)
> +               if (!strcmp(list->namespace, namespace))
>                         return true;
>
>         return false;
> --
> 2.17.1
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/4] scripts/nsdeps: support nsdeps for external module builds

[Matthias Maennich]

On Tue, Oct 29, 2019 at 09:38:08PM +0900, Masahiro Yamada wrote:
>Apparently, scripts/nsdeps is written to take care of only in-tree
>modules. Perhaps, this is not a bug, but just a design. At least,
>Documentation/core-api/symbol-namespaces.rst focuses on in-tree modules:
>
>  Again, `make nsdeps` will eventually add the missing namespace imports for
>  in-tree modules::
>  ^^^^^^^
>
>Having said that, I already saw at least two people trying nsdeps for
>external module builds. So, it would be nice to support it.
>
>Reported-by: Steve French <smfrench@gmail.com>
>Reported-by: Jessica Yu <jeyu@kernel.org>
>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>---
>

Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

> Documentation/core-api/symbol-namespaces.rst |  3 +++
> Makefile                                     |  1 +
> scripts/Makefile.modpost                     |  2 +-
> scripts/nsdeps                               | 10 ++++++++--
> 4 files changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
>index 982ed7b568ac..9b76337f6756 100644
>--- a/Documentation/core-api/symbol-namespaces.rst
>+++ b/Documentation/core-api/symbol-namespaces.rst
>@@ -152,3 +152,6 @@ in-tree modules::
> 	- notice the warning of modpost telling about a missing import
> 	- run `make nsdeps` to add the import to the correct code location
>
>+You can also run nsdeps for external module builds. A typical usage is::
>+
>+	$ make -C <path_to_kernel_src> M=$PWD nsdeps
>diff --git a/Makefile b/Makefile
>index 1e3f307bd49b..780a65493866 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -1007,6 +1007,7 @@ endif
> PHONY += prepare0
>
> export MODORDER := $(extmod-prefix)modules.order
>+export MODULES_NSDEPS := $(extmod-prefix)modules.nsdeps
>
> ifeq ($(KBUILD_EXTMOD),)
> core-y		+= kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
>diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
>index da37128c3f9f..8359f8af5ee6 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 modules.nsdeps)
>+	$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))
>
> ifeq ($(KBUILD_EXTMOD),)
> MODPOST += $(wildcard vmlinux)
>diff --git a/scripts/nsdeps b/scripts/nsdeps
>index 08db427a7fe5..3b8a9e173ebf 100644
>--- a/scripts/nsdeps
>+++ b/scripts/nsdeps
>@@ -21,6 +21,12 @@ if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then
> 	exit 1
> fi
>
>+if [ "$KBUILD_EXTMOD" ]; then
>+	src_prefix=
>+else
>+	src_prefix=$srctree/
>+fi
>+
> generate_deps_for_ns() {
> 	$SPATCH --very-quiet --in-place --sp-file \
> 		$srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2
>@@ -32,7 +38,7 @@ generate_deps() {
> 	local namespaces="$*"
> 	local mod_source_files=`cat $mod.mod | sed -n 1p                      \
> 					      | sed -e 's/\.o/\.c/g'           \
>-					      | sed "s|[^ ]* *|${srctree}/&|g"`
>+					     | sed "s|[^ ]* *|${src_prefix}&|g"`
> 	for ns in $namespaces; do
> 		echo "Adding namespace $ns to module $mod.ko."
> 		generate_deps_for_ns $ns $mod_source_files
>@@ -54,4 +60,4 @@ generate_deps() {
> while read line
> do
> 	generate_deps $line
>-done < modules.nsdeps
>+done < $MODULES_NSDEPS
>-- 
>2.17.1
>