| Message ID | 20190813121733.52480-9-maennich@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Symbol namespaces - RFC | expand |
On Tue, 13 Aug 2019, Matthias Maennich wrote: > A script that uses the '<module>.ns_deps' file generated by modpost to > automatically add the required symbol namespace dependencies to each > module. > > Usage: > 1) Move some symbols to a namespace with EXPORT_SYMBOL_NS() or define > DEFAULT_SYMBOL_NAMESPACE > 2) Run 'make' (or 'make modules') and get warnings about modules not > importing that namespace. > 3) Run 'make nsdeps' to automatically add required import statements > to said modules. > > This makes it easer for subsystem maintainers to introduce and maintain > symbol namespaces into their codebase. > > Co-developed-by: Martijn Coenen <maco@android.com> > Signed-off-by: Martijn Coenen <maco@android.com> > Signed-off-by: Matthias Maennich <maennich@google.com> Acked-by: Julia Lawall <julia.lawall@lip6.fr> > --- > MAINTAINERS | 5 ++ > Makefile | 12 +++++ > scripts/Makefile.modpost | 4 +- > scripts/coccinelle/misc/add_namespace.cocci | 23 +++++++++ > scripts/nsdeps | 54 +++++++++++++++++++++ > 5 files changed, 97 insertions(+), 1 deletion(-) > create mode 100644 scripts/coccinelle/misc/add_namespace.cocci > create mode 100644 scripts/nsdeps > > diff --git a/MAINTAINERS b/MAINTAINERS > index e81e60bd7c26..aa169070a052 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11414,6 +11414,11 @@ S: Maintained > T: git git://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git > F: tools/include/nolibc/ > > +NSDEPS > +M: Matthias Maennich <maennich@google.com> > +S: Maintained > +F: scripts/nsdeps > + > NTB AMD DRIVER > M: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > L: linux-ntb@googlegroups.com > diff --git a/Makefile b/Makefile > index 1b23f95db176..c5c3356e133c 100644 > --- a/Makefile > +++ b/Makefile > @@ -1500,6 +1500,9 @@ help: > @echo ' headerdep - Detect inclusion cycles in headers' > @echo ' coccicheck - Check with Coccinelle' > @echo '' > + @echo 'Tools:' > + @echo ' nsdeps - Generate missing symbol namespace dependencies' > + @echo '' > @echo 'Kernel selftest:' > @echo ' kselftest - Build and run kernel selftest (run as root)' > @echo ' Build, install, and boot kernel before' > @@ -1687,6 +1690,15 @@ quiet_cmd_tags = GEN $@ > tags TAGS cscope gtags: FORCE > $(call cmd,tags) > > +# Script to generate missing namespace dependencies > +# --------------------------------------------------------------------------- > + > +PHONY += nsdeps > + > +nsdeps: > + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps > + $(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@ > + > # Scripts to check various things for consistency > # --------------------------------------------------------------------------- > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > index 26e6574ecd08..743fe3a2e885 100644 > --- a/scripts/Makefile.modpost > +++ b/scripts/Makefile.modpost > @@ -56,7 +56,8 @@ 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 $(KBUILD_MODPOST_WARN),-w) \ > + $(if $(filter nsdeps,$(MAKECMDGOALS)),-d) > > ifdef MODPOST_VMLINUX > > @@ -134,6 +135,7 @@ $(modules): %.ko :%.o %.mod.o FORCE > > targets += $(modules) > > +nsdeps: __modpost > > # Add FORCE to the prequisites of a target to force it to be always rebuilt. > # --------------------------------------------------------------------------- > diff --git a/scripts/coccinelle/misc/add_namespace.cocci b/scripts/coccinelle/misc/add_namespace.cocci > new file mode 100644 > index 000000000000..c832bb6445a8 > --- /dev/null > +++ b/scripts/coccinelle/misc/add_namespace.cocci > @@ -0,0 +1,23 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +/// Adds missing MODULE_IMPORT_NS statements to source files > +/// > +/// This script is usually called from scripts/nsdeps with -D ns=<namespace> to > +/// add a missing namespace tag to a module source file. > +/// > + > +@has_ns_import@ > +declarer name MODULE_IMPORT_NS; > +identifier virtual.ns; > +@@ > +MODULE_IMPORT_NS(ns); > + > +// Add missing imports, but only adjacent to a MODULE_LICENSE statement. > +// That ensures we are adding it only to the main module source file. > +@do_import depends on !has_ns_import@ > +declarer name MODULE_LICENSE; > +expression license; > +identifier virtual.ns; > +@@ > +MODULE_LICENSE(license); > ++ MODULE_IMPORT_NS(ns); > diff --git a/scripts/nsdeps b/scripts/nsdeps > new file mode 100644 > index 000000000000..148db65a830f > --- /dev/null > +++ b/scripts/nsdeps > @@ -0,0 +1,54 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Linux kernel symbol namespace import generator > +# > +# This script requires at least spatch > +# version 1.0.4. > +SPATCH_REQ_VERSION="1.0.4" > + > +DIR="$(dirname $(readlink -f $0))/.." > +SPATCH="`which ${SPATCH:=spatch}`" > +if [ ! -x "$SPATCH" ]; then > + echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' > + exit 1 > +fi > + > +SPATCH_REQ_VERSION_NUM=$(echo $SPATCH_REQ_VERSION | ${DIR}/scripts/ld-version.sh) > +SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}') > +SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh) > + > +if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then > + echo 'spatch needs to be version 1.06 or higher' > + exit 1 > +fi > + > +generate_deps_for_ns() { > + $SPATCH --very-quiet --in-place --sp-file \ > + $srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2 > +} > + > +generate_deps() { > + local mod_file=`echo $@ | sed -e 's/\.ns_deps/\.mod/'` > + local mod_name=`cat $mod_file | sed -n 1p | sed -e 's/\/[^.]*$//'` > + local mod_source_files=`cat $mod_file | sed -n 2p | sed -e 's/\.o/\.c/g'` > + for ns in `cat $@`; do > + echo "Adding namespace $ns to module $mod_name (if needed)." > + generate_deps_for_ns $ns $mod_source_files > + # sort the imports > + for source_file in $mod_source_files; do > + sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp > + offset=$(wc -l ${source_file}.tmp | awk '{print $1;}') > + cat $source_file | grep MODULE_IMPORT_NS | sort -u >> ${source_file}.tmp > + tail -n +$((offset +1)) ${source_file} | grep -v MODULE_IMPORT_NS >> ${source_file}.tmp > + if ! diff -q ${source_file} ${source_file}.tmp; then > + mv ${source_file}.tmp ${source_file} > + else > + rm ${source_file}.tmp > + fi > + done > + done > +} > + > +for f in `find $srctree/.tmp_versions/ -name *.ns_deps`; do > + generate_deps $f > +done > -- > 2.23.0.rc1.153.gdeed80330f-goog > >
On Tue, Aug 13, 2019 at 01:17:05PM +0100, Matthias Maennich wrote: > A script that uses the '<module>.ns_deps' file generated by modpost to > automatically add the required symbol namespace dependencies to each > module. > > Usage: > 1) Move some symbols to a namespace with EXPORT_SYMBOL_NS() or define > DEFAULT_SYMBOL_NAMESPACE > 2) Run 'make' (or 'make modules') and get warnings about modules not > importing that namespace. > 3) Run 'make nsdeps' to automatically add required import statements > to said modules. > > This makes it easer for subsystem maintainers to introduce and maintain > symbol namespaces into their codebase. > > Co-developed-by: Martijn Coenen <maco@android.com> > Signed-off-by: Martijn Coenen <maco@android.com> > Signed-off-by: Matthias Maennich <maennich@google.com> > --- I really can't express just how cool this patch is. I was amazed when I first saw it in action a long time ago, and still am. Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Tue, Aug 13, 2019 at 01:17:05PM +0100, Matthias Maennich wrote: > A script that uses the '<module>.ns_deps' file generated by modpost to > automatically add the required symbol namespace dependencies to each > module. > > Usage: > 1) Move some symbols to a namespace with EXPORT_SYMBOL_NS() or define > DEFAULT_SYMBOL_NAMESPACE > 2) Run 'make' (or 'make modules') and get warnings about modules not > importing that namespace. > 3) Run 'make nsdeps' to automatically add required import statements > to said modules. > > This makes it easer for subsystem maintainers to introduce and maintain > symbol namespaces into their codebase. > > Co-developed-by: Martijn Coenen <maco@android.com> > Signed-off-by: Martijn Coenen <maco@android.com> > Signed-off-by: Matthias Maennich <maennich@google.com> > --- [] > MAINTAINERS | 5 ++ > Makefile | 12 +++++ > scripts/Makefile.modpost | 4 +- > scripts/coccinelle/misc/add_namespace.cocci | 23 +++++++++ > scripts/nsdeps | 54 +++++++++++++++++++++ > 5 files changed, 97 insertions(+), 1 deletion(-) > create mode 100644 scripts/coccinelle/misc/add_namespace.cocci > create mode 100644 scripts/nsdeps [] > +if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then > + echo 'spatch needs to be version 1.06 or higher' Nitpick: 1.0.6 > + exit 1 > +fi
On Wed, Aug 14, 2019 at 12:06:11PM +0530, Himanshu Jha wrote: >On Tue, Aug 13, 2019 at 01:17:05PM +0100, Matthias Maennich wrote: >> A script that uses the '<module>.ns_deps' file generated by modpost to >> automatically add the required symbol namespace dependencies to each >> module. >> >> Usage: >> 1) Move some symbols to a namespace with EXPORT_SYMBOL_NS() or define >> DEFAULT_SYMBOL_NAMESPACE >> 2) Run 'make' (or 'make modules') and get warnings about modules not >> importing that namespace. >> 3) Run 'make nsdeps' to automatically add required import statements >> to said modules. >> >> This makes it easer for subsystem maintainers to introduce and maintain >> symbol namespaces into their codebase. >> >> Co-developed-by: Martijn Coenen <maco@android.com> >> Signed-off-by: Martijn Coenen <maco@android.com> >> Signed-off-by: Matthias Maennich <maennich@google.com> >> --- > >[] > >> MAINTAINERS | 5 ++ >> Makefile | 12 +++++ >> scripts/Makefile.modpost | 4 +- >> scripts/coccinelle/misc/add_namespace.cocci | 23 +++++++++ >> scripts/nsdeps | 54 +++++++++++++++++++++ >> 5 files changed, 97 insertions(+), 1 deletion(-) >> create mode 100644 scripts/coccinelle/misc/add_namespace.cocci >> create mode 100644 scripts/nsdeps > >[] > >> +if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then >> + echo 'spatch needs to be version 1.06 or higher' > >Nitpick: 1.0.6 > Good catch! Thanks! Actually it needs to be even 1.0.4, so I will just use $SPATCH_REQ_VERSION from above. >> + exit 1 >> +fi > > >-- >Himanshu Jha >Undergraduate Student >Department of Electronics & Communication >Guru Tegh Bahadur Institute of Technology
+# This script requires at least spatch +# version 1.0.4. How do you think about to avoid the duplicate specification of this identification? Regards, Markus
> +generate_deps_for_ns() { > + $SPATCH --very-quiet --in-place --sp-file \ > + $srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2 > +} * Where will the variable “srctree” be set for the file “scripts/nsdeps”? * Would you like to support a separate build directory for desired adjustments? * How do you think about to check error handling around such commands? > +generate_deps() { … > + for source_file in $mod_source_files; do > + sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp … I suggest to assign the name for the temporary file to a variable which should be used by subsequent commands. Regards, Markus
On Thu, Aug 15, 2019 at 03:50:38PM +0200, Markus Elfring wrote: >> +generate_deps_for_ns() { >> + $SPATCH --very-quiet --in-place --sp-file \ >> + $srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2 >> +} > >* Where will the variable “srctree” be set for the file “scripts/nsdeps”? > $srctree is defined by kbuild in the toplevel Makefile. >* Would you like to support a separate build directory for desired adjustments? > No, as the purpose of this script is to directly patch the kernel sources where applicable. >* How do you think about to check error handling around such commands? > > spatch emits a descriptive message on error. I will add a 'set -e' to the script so that it aborts on errors. >> +generate_deps() { >… >> + for source_file in $mod_source_files; do >> + sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp >… > >I suggest to assign the name for the temporary file to a variable >which should be used by subsequent commands. I somehow don't agree that this is an improvement to the code as the variable would likely be something like ${source_file_tmp}. Sticking to ${source_file}.tmp does express the intent of a temporary file next to the original source file and the reader of the code does not need to reason about the value of ${source_file_tmp}. Cheers, Matthias
> $srctree is defined by kbuild in the toplevel Makefile. How is this variable passed to the file “scripts/nsdeps”? >> * Would you like to support a separate build directory for desired adjustments? > > No, as the purpose of this script is to directly patch the kernel > sources where applicable. Will there occasionally be a need to provide a generated patch (without in-place file modification)? >> I suggest to assign the name for the temporary file to a variable >> which should be used by subsequent commands. > > I somehow don't agree that this is an improvement to the code as the > variable would likely be something like ${source_file_tmp}. Would you dare to choose a shorter variable name? > ${source_file}.tmp does express the intent of a temporary file next to > the original source file and the reader of the code does not need to > reason about the value of ${source_file_tmp}. I would find a code variant with less suffix repetition nicer. Regards, Markus
diff --git a/MAINTAINERS b/MAINTAINERS index e81e60bd7c26..aa169070a052 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11414,6 +11414,11 @@ S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git F: tools/include/nolibc/ +NSDEPS +M: Matthias Maennich <maennich@google.com> +S: Maintained +F: scripts/nsdeps + NTB AMD DRIVER M: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> L: linux-ntb@googlegroups.com diff --git a/Makefile b/Makefile index 1b23f95db176..c5c3356e133c 100644 --- a/Makefile +++ b/Makefile @@ -1500,6 +1500,9 @@ help: @echo ' headerdep - Detect inclusion cycles in headers' @echo ' coccicheck - Check with Coccinelle' @echo '' + @echo 'Tools:' + @echo ' nsdeps - Generate missing symbol namespace dependencies' + @echo '' @echo 'Kernel selftest:' @echo ' kselftest - Build and run kernel selftest (run as root)' @echo ' Build, install, and boot kernel before' @@ -1687,6 +1690,15 @@ quiet_cmd_tags = GEN $@ tags TAGS cscope gtags: FORCE $(call cmd,tags) +# Script to generate missing namespace dependencies +# --------------------------------------------------------------------------- + +PHONY += nsdeps + +nsdeps: + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps + $(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@ + # Scripts to check various things for consistency # --------------------------------------------------------------------------- diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index 26e6574ecd08..743fe3a2e885 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -56,7 +56,8 @@ 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 $(KBUILD_MODPOST_WARN),-w) \ + $(if $(filter nsdeps,$(MAKECMDGOALS)),-d) ifdef MODPOST_VMLINUX @@ -134,6 +135,7 @@ $(modules): %.ko :%.o %.mod.o FORCE targets += $(modules) +nsdeps: __modpost # Add FORCE to the prequisites of a target to force it to be always rebuilt. # --------------------------------------------------------------------------- diff --git a/scripts/coccinelle/misc/add_namespace.cocci b/scripts/coccinelle/misc/add_namespace.cocci new file mode 100644 index 000000000000..c832bb6445a8 --- /dev/null +++ b/scripts/coccinelle/misc/add_namespace.cocci @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +/// Adds missing MODULE_IMPORT_NS statements to source files +/// +/// This script is usually called from scripts/nsdeps with -D ns=<namespace> to +/// add a missing namespace tag to a module source file. +/// + +@has_ns_import@ +declarer name MODULE_IMPORT_NS; +identifier virtual.ns; +@@ +MODULE_IMPORT_NS(ns); + +// Add missing imports, but only adjacent to a MODULE_LICENSE statement. +// That ensures we are adding it only to the main module source file. +@do_import depends on !has_ns_import@ +declarer name MODULE_LICENSE; +expression license; +identifier virtual.ns; +@@ +MODULE_LICENSE(license); ++ MODULE_IMPORT_NS(ns); diff --git a/scripts/nsdeps b/scripts/nsdeps new file mode 100644 index 000000000000..148db65a830f --- /dev/null +++ b/scripts/nsdeps @@ -0,0 +1,54 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Linux kernel symbol namespace import generator +# +# This script requires at least spatch +# version 1.0.4. +SPATCH_REQ_VERSION="1.0.4" + +DIR="$(dirname $(readlink -f $0))/.." +SPATCH="`which ${SPATCH:=spatch}`" +if [ ! -x "$SPATCH" ]; then + echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' + exit 1 +fi + +SPATCH_REQ_VERSION_NUM=$(echo $SPATCH_REQ_VERSION | ${DIR}/scripts/ld-version.sh) +SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}') +SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh) + +if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then + echo 'spatch needs to be version 1.06 or higher' + exit 1 +fi + +generate_deps_for_ns() { + $SPATCH --very-quiet --in-place --sp-file \ + $srctree/scripts/coccinelle/misc/add_namespace.cocci -D ns=$1 $2 +} + +generate_deps() { + local mod_file=`echo $@ | sed -e 's/\.ns_deps/\.mod/'` + local mod_name=`cat $mod_file | sed -n 1p | sed -e 's/\/[^.]*$//'` + local mod_source_files=`cat $mod_file | sed -n 2p | sed -e 's/\.o/\.c/g'` + for ns in `cat $@`; do + echo "Adding namespace $ns to module $mod_name (if needed)." + generate_deps_for_ns $ns $mod_source_files + # sort the imports + for source_file in $mod_source_files; do + sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp + offset=$(wc -l ${source_file}.tmp | awk '{print $1;}') + cat $source_file | grep MODULE_IMPORT_NS | sort -u >> ${source_file}.tmp + tail -n +$((offset +1)) ${source_file} | grep -v MODULE_IMPORT_NS >> ${source_file}.tmp + if ! diff -q ${source_file} ${source_file}.tmp; then + mv ${source_file}.tmp ${source_file} + else + rm ${source_file}.tmp + fi + done + done +} + +for f in `find $srctree/.tmp_versions/ -name *.ns_deps`; do + generate_deps $f +done