In the recent build test of linux-next, Stephen saw a build error
caused by a broken .tmp_versions/*.mod file:
https://lkml.org/lkml/2019/5/13/991
drivers/net/phy/asix.ko and drivers/net/usb/asix.ko have the same
basename, and there is a race in generating .tmp_versions/asix.mod
Kbuild has not checked this before, and it suddenly shows up with
obscure error message when this kind of race occurs.
Non-unique module names cause various sort of problems, but it is
not trivial to catch them by eyes.
Hence, this script.
It checks not only real modules, but also built-in modules (i.e.
controlled by tristate CONFIG option, but currently compiled with =y).
Non-unique names for built-in modules also cause problems because
/sys/modules/ would fall over.
I tested allmodconfig on the latest kernel, and it detected the
following:
warning: same basename if the following are built as modules:
drivers/regulator/88pm800.ko
drivers/mfd/88pm800.ko
warning: same basename if the following are built as modules:
drivers/gpu/drm/bridge/adv7511/adv7511.ko
drivers/media/i2c/adv7511.ko
warning: same basename if the following are built as modules:
drivers/net/phy/asix.ko
drivers/net/usb/asix.ko
warning: same basename if the following are built as modules:
fs/coda/coda.ko
drivers/media/platform/coda/coda.ko
warning: same basename if the following are built as modules:
drivers/net/phy/realtek.ko
drivers/net/dsa/realtek.ko
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
Changes in v2:
- redirect messages to stderr
- use '--' after 'basename -a'
- use '-r' for xargs to cope with empty modules.order/modules.builtin
Makefile | 1 +
scripts/modules-check.sh | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)
create mode 100755 scripts/modules-check.sh
diff --git a/Makefile b/Makefile
index a61a95b6b38f..30792fec7a12 100644
--- a/Makefile
+++ b/Makefile
@@ -1290,6 +1290,7 @@ modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) modules.builtin
$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
@$(kecho) ' Building modules, stage 2.';
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
+ $(Q)$(CONFIG_SHELL) $(srctree)/scripts/modules-check.sh
modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
$(Q)$(AWK) '!x[$$0]++' $^ > $(objtree)/modules.builtin
diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
new file mode 100755
index 000000000000..c875f6eab01e
--- /dev/null
+++ b/scripts/modules-check.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+# Check uniqueness of module names
+check_same_name_modules()
+{
+ same_name_modules=$(cat modules.order modules.builtin | \
+ xargs -r basename -a -- | sort | uniq -d)
+
+ for m in $same_name_modules
+ do
+ echo "warning: same basename if the following are built as modules:" >&2
+ grep -h -e "/$m" modules.order modules.builtin | \
+ sed 's:^kernel/: :' >&2
+ done
+}
+
+check_same_name_modules
--
2.17.1
On Fri, May 17, 2019 at 1:29 PM Masahiro Yamada
<[email protected]> wrote:
>
> In the recent build test of linux-next, Stephen saw a build error
> caused by a broken .tmp_versions/*.mod file:
>
> https://lkml.org/lkml/2019/5/13/991
>
> drivers/net/phy/asix.ko and drivers/net/usb/asix.ko have the same
> basename, and there is a race in generating .tmp_versions/asix.mod
>
> Kbuild has not checked this before, and it suddenly shows up with
> obscure error message when this kind of race occurs.
>
> Non-unique module names cause various sort of problems, but it is
> not trivial to catch them by eyes.
>
> Hence, this script.
>
> It checks not only real modules, but also built-in modules (i.e.
> controlled by tristate CONFIG option, but currently compiled with =y).
> Non-unique names for built-in modules also cause problems because
> /sys/modules/ would fall over.
>
> I tested allmodconfig on the latest kernel, and it detected the
> following:
>
> warning: same basename if the following are built as modules:
> drivers/regulator/88pm800.ko
> drivers/mfd/88pm800.ko
> warning: same basename if the following are built as modules:
> drivers/gpu/drm/bridge/adv7511/adv7511.ko
> drivers/media/i2c/adv7511.ko
> warning: same basename if the following are built as modules:
> drivers/net/phy/asix.ko
> drivers/net/usb/asix.ko
> warning: same basename if the following are built as modules:
> fs/coda/coda.ko
> drivers/media/platform/coda/coda.ko
> warning: same basename if the following are built as modules:
> drivers/net/phy/realtek.ko
> drivers/net/dsa/realtek.ko
>
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> ---
One more question popped up.
External modules are out of scope of the community,
but it is possible that people create an external module
that happens to have the same name as an upstream driver.
drivers/this/is/upstream/subsystem/foo.ko
/home/fred/my/own/external/module/foo.ko
Is modprobe confused in this case too?
Perhaps, checking for the M= build
might be good too...
--
Best Regards
Masahiro Yamada
On Thu, May 16, 2019 at 10:37 PM Greg KH <[email protected]> wrote:
>
> On Fri, May 17, 2019 at 01:45:11PM +0900, Masahiro Yamada wrote:
> > On Fri, May 17, 2019 at 1:29 PM Masahiro Yamada
> > <[email protected]> wrote:
> > >
> > > In the recent build test of linux-next, Stephen saw a build error
> > > caused by a broken .tmp_versions/*.mod file:
> > >
> > > https://lkml.org/lkml/2019/5/13/991
> > >
> > > drivers/net/phy/asix.ko and drivers/net/usb/asix.ko have the same
> > > basename, and there is a race in generating .tmp_versions/asix.mod
> > >
> > > Kbuild has not checked this before, and it suddenly shows up with
> > > obscure error message when this kind of race occurs.
> > >
> > > Non-unique module names cause various sort of problems, but it is
> > > not trivial to catch them by eyes.
> > >
> > > Hence, this script.
> > >
> > > It checks not only real modules, but also built-in modules (i.e.
> > > controlled by tristate CONFIG option, but currently compiled with =y).
> > > Non-unique names for built-in modules also cause problems because
> > > /sys/modules/ would fall over.
> > >
> > > I tested allmodconfig on the latest kernel, and it detected the
> > > following:
> > >
> > > warning: same basename if the following are built as modules:
> > > drivers/regulator/88pm800.ko
> > > drivers/mfd/88pm800.ko
> > > warning: same basename if the following are built as modules:
> > > drivers/gpu/drm/bridge/adv7511/adv7511.ko
> > > drivers/media/i2c/adv7511.ko
> > > warning: same basename if the following are built as modules:
> > > drivers/net/phy/asix.ko
> > > drivers/net/usb/asix.ko
> > > warning: same basename if the following are built as modules:
> > > fs/coda/coda.ko
> > > drivers/media/platform/coda/coda.ko
> > > warning: same basename if the following are built as modules:
> > > drivers/net/phy/realtek.ko
> > > drivers/net/dsa/realtek.ko
> > >
> > > Reported-by: Stephen Rothwell <[email protected]>
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > Reviewed-by: Kees Cook <[email protected]>
> > > ---
> >
> >
> > One more question popped up.
> >
> > External modules are out of scope of the community,
> > but it is possible that people create an external module
> > that happens to have the same name as an upstream driver.
>
> That is their bug, nothing we can do about that :)
It's actually not a bug. For external modules it works pretty much as
intended. See DEPMOD.D(5): the search directive tells what's the
preference among the directories for modules with the same name.
depmod respects that order and put the right one into your
modules.dep.
This allows to put external modules in a different dir and also to
make backports of modules from recent to ancient kernels. These
modules with the same name are usually the same module, with a
different version. Of course what we have here and you are fixing is a
different story.
Reviewed-by: Lucas De Marchi <[email protected]>
Lucas De Marchi
On Fri, May 17, 2019 at 01:45:11PM +0900, Masahiro Yamada wrote:
> On Fri, May 17, 2019 at 1:29 PM Masahiro Yamada
> <[email protected]> wrote:
> >
> > In the recent build test of linux-next, Stephen saw a build error
> > caused by a broken .tmp_versions/*.mod file:
> >
> > https://lkml.org/lkml/2019/5/13/991
> >
> > drivers/net/phy/asix.ko and drivers/net/usb/asix.ko have the same
> > basename, and there is a race in generating .tmp_versions/asix.mod
> >
> > Kbuild has not checked this before, and it suddenly shows up with
> > obscure error message when this kind of race occurs.
> >
> > Non-unique module names cause various sort of problems, but it is
> > not trivial to catch them by eyes.
> >
> > Hence, this script.
> >
> > It checks not only real modules, but also built-in modules (i.e.
> > controlled by tristate CONFIG option, but currently compiled with =y).
> > Non-unique names for built-in modules also cause problems because
> > /sys/modules/ would fall over.
> >
> > I tested allmodconfig on the latest kernel, and it detected the
> > following:
> >
> > warning: same basename if the following are built as modules:
> > drivers/regulator/88pm800.ko
> > drivers/mfd/88pm800.ko
> > warning: same basename if the following are built as modules:
> > drivers/gpu/drm/bridge/adv7511/adv7511.ko
> > drivers/media/i2c/adv7511.ko
> > warning: same basename if the following are built as modules:
> > drivers/net/phy/asix.ko
> > drivers/net/usb/asix.ko
> > warning: same basename if the following are built as modules:
> > fs/coda/coda.ko
> > drivers/media/platform/coda/coda.ko
> > warning: same basename if the following are built as modules:
> > drivers/net/phy/realtek.ko
> > drivers/net/dsa/realtek.ko
> >
> > Reported-by: Stephen Rothwell <[email protected]>
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > Reviewed-by: Kees Cook <[email protected]>
> > ---
>
>
> One more question popped up.
>
> External modules are out of scope of the community,
> but it is possible that people create an external module
> that happens to have the same name as an upstream driver.
That is their bug, nothing we can do about that :)
thanks,
greg k-h
Hi Masahiro,
Thanks for this, looks good to me. Just a nit below.
On Fri, 17 May 2019 13:27:53 +0900 Masahiro Yamada <[email protected]> wrote:
>
> diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
> new file mode 100755
> index 000000000000..c875f6eab01e
> --- /dev/null
> +++ b/scripts/modules-check.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +# Check uniqueness of module names
> +check_same_name_modules()
> +{
> + same_name_modules=$(cat modules.order modules.builtin | \
^
This trailing '\' is unnecessary after a pipe symbol.
> + xargs -r basename -a -- | sort | uniq -d)
> +
> + for m in $same_name_modules
> + do
> + echo "warning: same basename if the following are built as modules:" >&2
> + grep -h -e "/$m" modules.order modules.builtin | \
Same here
> + sed 's:^kernel/: :' >&2
> + done
> +}
> +
> +check_same_name_modules
Reviewed-by: Stephen ROthwell <[email protected]>
--
Cheers,
Stephen Rothwell
On Fri, May 17, 2019 at 8:33 AM Masahiro Yamada
<[email protected]> wrote:
>
> In the recent build test of linux-next, Stephen saw a build error
> caused by a broken .tmp_versions/*.mod file:
>
> https://lkml.org/lkml/2019/5/13/991
>
> drivers/net/phy/asix.ko and drivers/net/usb/asix.ko have the same
> basename, and there is a race in generating .tmp_versions/asix.mod
>
> Kbuild has not checked this before, and it suddenly shows up with
> obscure error message when this kind of race occurs.
>
> Non-unique module names cause various sort of problems, but it is
> not trivial to catch them by eyes.
>
> Hence, this script.
>
> It checks not only real modules, but also built-in modules (i.e.
> controlled by tristate CONFIG option, but currently compiled with =y).
> Non-unique names for built-in modules also cause problems because
> /sys/modules/ would fall over.
>
> I tested allmodconfig on the latest kernel, and it detected the
> following:
>
> warning: same basename if the following are built as modules:
> drivers/regulator/88pm800.ko
> drivers/mfd/88pm800.ko
> warning: same basename if the following are built as modules:
> drivers/gpu/drm/bridge/adv7511/adv7511.ko
> drivers/media/i2c/adv7511.ko
> warning: same basename if the following are built as modules:
> drivers/net/phy/asix.ko
> drivers/net/usb/asix.ko
> warning: same basename if the following are built as modules:
> fs/coda/coda.ko
> drivers/media/platform/coda/coda.ko
> warning: same basename if the following are built as modules:
> drivers/net/phy/realtek.ko
> drivers/net/dsa/realtek.ko
>
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> ---
>
> Changes in v2:
> - redirect messages to stderr
> - use '--' after 'basename -a'
> - use '-r' for xargs to cope with empty modules.order/modules.builtin
>
> Makefile | 1 +
> scripts/modules-check.sh | 20 ++++++++++++++++++++
> 2 files changed, 21 insertions(+)
> create mode 100755 scripts/modules-check.sh
>
> diff --git a/Makefile b/Makefile
> index a61a95b6b38f..30792fec7a12 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1290,6 +1290,7 @@ modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) modules.builtin
> $(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
> @$(kecho) ' Building modules, stage 2.';
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
> + $(Q)$(CONFIG_SHELL) $(srctree)/scripts/modules-check.sh
>
> modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
> $(Q)$(AWK) '!x[$$0]++' $^ > $(objtree)/modules.builtin
> diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
> new file mode 100755
> index 000000000000..c875f6eab01e
> --- /dev/null
> +++ b/scripts/modules-check.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +# Check uniqueness of module names
> +check_same_name_modules()
> +{
> + same_name_modules=$(cat modules.order modules.builtin | \
> + xargs -r basename -a -- | sort | uniq -d)
> +
> + for m in $same_name_modules
> + do
> + echo "warning: same basename if the following are built as modules:" >&2
> + grep -h -e "/$m" modules.order modules.builtin | \
> + sed 's:^kernel/: :' >&2
> + done
> +}
> +
> +check_same_name_modules
> --
> 2.17.1
>
The 'xargs' '-r' flag is a GNU extension.
If POSIX compliance is important here, the use of 'cat', 'xargs' and
'basename' may be substituted with that of 'sed' to initialise
same_name_modules:
sed 's!.*/!!' modules.order modules.builtin | sort | uniq -d
'Sed' may also be used on its own in the 'for' loop instead of as part
of a pipeline along with 'grep' to generate the desired output:
sed '/\/'$m'/!d;s:^kernel/: :' modules.order modules.builtin
Just a suggestion.
On Fri, May 17, 2019 at 11:58 AM Bernd Petrovitsch
<[email protected]> wrote:
>
> On 17/05/2019 10:16, Alexander Kapshuk wrote:
> [...]
> > The 'xargs' '-r' flag is a GNU extension.
> > If POSIX compliance is important here, the use of 'cat', 'xargs' and
> > 'basename' may be substituted with that of 'sed' to initialise
> > same_name_modules:
> > sed 's!.*/!!' modules.order modules.builtin | sort | uniq -d
>
> 's!' is TTBOMK also a GNU-extension:
> sed 's/.*\///' modules.order modules.builtin | sort | uniq -d
It isn't.
Here's an excerpt from the POSIX manpage for 'sed',
http://pubs.opengroup.org/onlinepubs/009695399/utilities/sed.html:
[2addr]s/BRE/replacement/flags
... Any character other than backslash or <newline> can be used
instead of a slash to delimit the BRE and the replacement....
>
> > 'Sed' may also be used on its own in the 'for' loop instead of as part
> > of a pipeline along with 'grep' to generate the desired output:
> > sed '/\/'$m'/!d;s:^kernel/: :' modules.order modules.builtin
>
> sed "/\/${m}/!d;s/^kernel\// /" modules.order modules.builtin
The parameter expansion syntax is redundant here.
See https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02:
The parameter name or symbol can be enclosed in braces, which are
optional except for positional parameters with more than one digit or
when parameter is a name and is followed by a character that could be
interpreted as part of the name.
Here's an alternative version using double quotes.
sed "/\/$m/!d;s:^kernel/: :" modules.order modules.builtin
>
> MfG,
> Bernd
> --
> Bernd Petrovitsch Email : [email protected]
> LUGA : http://www.luga.at
On 17/05/2019 10:16, Alexander Kapshuk wrote:
[...]
> The 'xargs' '-r' flag is a GNU extension.
> If POSIX compliance is important here, the use of 'cat', 'xargs' and
> 'basename' may be substituted with that of 'sed' to initialise
> same_name_modules:
> sed 's!.*/!!' modules.order modules.builtin | sort | uniq -d
's!' is TTBOMK also a GNU-extension:
sed 's/.*\///' modules.order modules.builtin | sort | uniq -d
> 'Sed' may also be used on its own in the 'for' loop instead of as part
> of a pipeline along with 'grep' to generate the desired output:
> sed '/\/'$m'/!d;s:^kernel/: :' modules.order modules.builtin
sed "/\/${m}/!d;s/^kernel\// /" modules.order modules.builtin
MfG,
Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at
On 17/05/2019 11:25, Alexander Kapshuk wrote:
> On Fri, May 17, 2019 at 11:58 AM Bernd Petrovitsch
> <[email protected]> wrote:
>>
>> On 17/05/2019 10:16, Alexander Kapshuk wrote:
>> [...]
>>> The 'xargs' '-r' flag is a GNU extension.
>>> If POSIX compliance is important here, the use of 'cat', 'xargs' and
>>> 'basename' may be substituted with that of 'sed' to initialise
>>> same_name_modules:
>>> sed 's!.*/!!' modules.order modules.builtin | sort | uniq -d
>>
>> 's!' is TTBOMK also a GNU-extension:
>> sed 's/.*\///' modules.order modules.builtin | sort | uniq -d
>
> It isn't.
> Here's an excerpt from the POSIX manpage for 'sed',
> http://pubs.opengroup.org/onlinepubs/009695399/utilities/sed.html:
> [2addr]s/BRE/replacement/flags
> ... Any character other than backslash or <newline> can be used
> instead of a slash to delimit the BRE and the replacement....
Oops, yup, sorry for the noise.
Don't know anymore where I encountered problems with that in the past ....
MfG,
Bernd--
"I dislike type abstraction if it has no real reason. And saving
on typing is not a good reason - if your typing speed is the main
issue when you're coding, you're doing something seriously wrong."
- Linus Torvalds
On Fri, May 17, 2019 at 6:25 PM Alexander Kapshuk
<[email protected]> wrote:
>
> On Fri, May 17, 2019 at 11:58 AM Bernd Petrovitsch
> <[email protected]> wrote:
> >
> > On 17/05/2019 10:16, Alexander Kapshuk wrote:
> > [...]
> > > The 'xargs' '-r' flag is a GNU extension.
> > > If POSIX compliance is important here, the use of 'cat', 'xargs' and
> > > 'basename' may be substituted with that of 'sed' to initialise
> > > same_name_modules:
> > > sed 's!.*/!!' modules.order modules.builtin | sort | uniq -d
> >
> > 's!' is TTBOMK also a GNU-extension:
> > sed 's/.*\///' modules.order modules.builtin | sort | uniq -d
>
> It isn't.
> Here's an excerpt from the POSIX manpage for 'sed',
> http://pubs.opengroup.org/onlinepubs/009695399/utilities/sed.html:
> [2addr]s/BRE/replacement/flags
> ... Any character other than backslash or <newline> can be used
> instead of a slash to delimit the BRE and the replacement....
>
> >
> > > 'Sed' may also be used on its own in the 'for' loop instead of as part
> > > of a pipeline along with 'grep' to generate the desired output:
> > > sed '/\/'$m'/!d;s:^kernel/: :' modules.order modules.builtin
> >
> > sed "/\/${m}/!d;s/^kernel\// /" modules.order modules.builtin
>
> The parameter expansion syntax is redundant here.
> See https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02:
> The parameter name or symbol can be enclosed in braces, which are
> optional except for positional parameters with more than one digit or
> when parameter is a name and is followed by a character that could be
> interpreted as part of the name.
>
> Here's an alternative version using double quotes.
> sed "/\/$m/!d;s:^kernel/: :" modules.order modules.builtin
Awesome!
This is much shorter.
I will use it in v3.
Thanks.
--
Best Regards
Masahiro Yamada
On Fri, May 17, 2019 at 2:46 PM Lucas De Marchi
<[email protected]> wrote:
>
> On Thu, May 16, 2019 at 10:37 PM Greg KH <[email protected]> wrote:
> >
> > On Fri, May 17, 2019 at 01:45:11PM +0900, Masahiro Yamada wrote:
> > > On Fri, May 17, 2019 at 1:29 PM Masahiro Yamada
> > > <[email protected]> wrote:
> > > >
> > > > In the recent build test of linux-next, Stephen saw a build error
> > > > caused by a broken .tmp_versions/*.mod file:
> > > >
> > > > https://lkml.org/lkml/2019/5/13/991
> > > >
> > > > drivers/net/phy/asix.ko and drivers/net/usb/asix.ko have the same
> > > > basename, and there is a race in generating .tmp_versions/asix.mod
> > > >
> > > > Kbuild has not checked this before, and it suddenly shows up with
> > > > obscure error message when this kind of race occurs.
> > > >
> > > > Non-unique module names cause various sort of problems, but it is
> > > > not trivial to catch them by eyes.
> > > >
> > > > Hence, this script.
> > > >
> > > > It checks not only real modules, but also built-in modules (i.e.
> > > > controlled by tristate CONFIG option, but currently compiled with =y).
> > > > Non-unique names for built-in modules also cause problems because
> > > > /sys/modules/ would fall over.
> > > >
> > > > I tested allmodconfig on the latest kernel, and it detected the
> > > > following:
> > > >
> > > > warning: same basename if the following are built as modules:
> > > > drivers/regulator/88pm800.ko
> > > > drivers/mfd/88pm800.ko
> > > > warning: same basename if the following are built as modules:
> > > > drivers/gpu/drm/bridge/adv7511/adv7511.ko
> > > > drivers/media/i2c/adv7511.ko
> > > > warning: same basename if the following are built as modules:
> > > > drivers/net/phy/asix.ko
> > > > drivers/net/usb/asix.ko
> > > > warning: same basename if the following are built as modules:
> > > > fs/coda/coda.ko
> > > > drivers/media/platform/coda/coda.ko
> > > > warning: same basename if the following are built as modules:
> > > > drivers/net/phy/realtek.ko
> > > > drivers/net/dsa/realtek.ko
> > > >
> > > > Reported-by: Stephen Rothwell <[email protected]>
> > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > Reviewed-by: Kees Cook <[email protected]>
> > > > ---
> > >
> > >
> > > One more question popped up.
> > >
> > > External modules are out of scope of the community,
> > > but it is possible that people create an external module
> > > that happens to have the same name as an upstream driver.
> >
> > That is their bug, nothing we can do about that :)
>
> It's actually not a bug. For external modules it works pretty much as
> intended. See DEPMOD.D(5): the search directive tells what's the
> preference among the directories for modules with the same name.
> depmod respects that order and put the right one into your
> modules.dep.
>
> This allows to put external modules in a different dir and also to
> make backports of modules from recent to ancient kernels. These
> modules with the same name are usually the same module, with a
> different version. Of course what we have here and you are fixing is a
> different story.
OK, so external modules should not be checked.
Thanks for the explanation!
> Reviewed-by: Lucas De Marchi <[email protected]>
>
>
> Lucas De Marchi
--
Best Regards
Masahiro Yamada
On Fri, May 17, 2019 at 2:34 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Masahiro,
>
> Thanks for this, looks good to me. Just a nit below.
>
> On Fri, 17 May 2019 13:27:53 +0900 Masahiro Yamada <[email protected]> wrote:
> >
>
> > diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
> > new file mode 100755
> > index 000000000000..c875f6eab01e
> > --- /dev/null
> > +++ b/scripts/modules-check.sh
> > @@ -0,0 +1,20 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +set -e
> > +
> > +# Check uniqueness of module names
> > +check_same_name_modules()
> > +{
> > + same_name_modules=$(cat modules.order modules.builtin | \
> ^
> This trailing '\' is unnecessary after a pipe symbol.
With the suggestion from Alexander Kapshuk,
the code in v3 became short enough to fit in 80-columns.
Anyway, thanks for pointing it out.
> > + xargs -r basename -a -- | sort | uniq -d)
> > +
> > + for m in $same_name_modules
> > + do
> > + echo "warning: same basename if the following are built as modules:" >&2
> > + grep -h -e "/$m" modules.order modules.builtin | \
>
> Same here
>
> > + sed 's:^kernel/: :' >&2
> > + done
> > +}
> > +
> > +check_same_name_modules
>
> Reviewed-by: Stephen ROthwell <[email protected]>
>
> --
> Cheers,
> Stephen Rothwell
--
Best Regards
Masahiro Yamada