[PATCH 0/5] kbuild: always define KBUILD_MODNAME, and clean-up/fix modname

[PATCH 0/5] kbuild: always define KBUILD_MODNAME, and clean-up/fix modname

[Masahiro Yamada]

3/5 takes into account '-m' case for multi-used-m.

2/5 is necessary beforehand because 3/5 would cause a build error for
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c

1, 4, 5 are just clean-ups.



Cao jin (1):
  kbuild: fix modname for composite modules

Masahiro Yamada (4):
  kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi
  kbuild: define KBUILD_MODNAME even if multiple modules share objects
  kbuild: simplify modname calculation
  kbuild: move modname and modname-multi close to modname_flags

 scripts/Makefile.build | 12 ------------
 scripts/Makefile.lib   | 22 +++++++++-------------
 2 files changed, 9 insertions(+), 25 deletions(-)

-- 
2.7.4

[PATCH 1/5] kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi

[Masahiro Yamada]

In the context ...

    $(obj)/%.s: $(src)/%.c FORCE
            $(call if_changed_dep,cc_s_c)

    $(obj)/%.i: $(src)/%.c FORCE
            $(call if_changed_dep,cpp_i_c)

    $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
            $(call cmd,force_checksrc)
            $(call if_changed_rule,cc_o_c)

    $(obj)/%.lst: $(src)/%.c FORCE
            $(call if_changed_dep,cc_lst_c)

'$*' returns the stem of the target (the part of '%'), so $(obj)/ has
already been ripped off.

$(subst $(obj)/,,$*.o) is the same as $(*.o)

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

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 5589bae..a7e315f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -175,7 +175,7 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
 
 # Finds the multi-part object the current object will be linked into
 modname-multi = $(sort $(foreach m,$(multi-used),\
-		$(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
+		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
 
 # Useful for describing the dependency of composite objects
 # Usage:
-- 
2.7.4

[PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Masahiro Yamada]

Currently, KBUILD_MODNAME is defined only when $(modname) contains
just one word.  If an object is shared among multiple modules,
undefined KBUILD_MODNAME could cause a build error.  For example,
if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates
.modname, then fails to build due to undefined KBUILD_MODNAME.

Take the following code as an example:

  obj-m += foo.o
  obj-m += bar.o
  foo-objs := foo-bar-common.o foo-main.o
  bar-objs := foo-bar-common.o bar-main.o

In this case, there is room for argument what to define for
KBUILD_MODNAME when foo-bar-common.o is being compiled.
"foo", "bar", or what else?

One idea is to define colon-separated modules that share the object,
in this case, "bar:foo" (modules are sorted alphabetically by
$(sort ...).

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

 scripts/Makefile.lib | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a7e315f..a1fbd6a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -92,8 +92,7 @@ subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
 #       differ in different configs.
 name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst -,_,$1))$(quote)$(squote)
 basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
-modname_flags  = $(if $(filter 1,$(words $(modname))),\
-                 -DKBUILD_MODNAME=$(call name-fix,$(modname)))
+modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname))
 
 orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) \
                  $(ccflags-y) $(CFLAGS_$(basetarget).o)
@@ -174,8 +173,10 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
 		 -undef -D__DTS__
 
 # Finds the multi-part object the current object will be linked into
-modname-multi = $(sort $(foreach m,$(multi-used),\
-		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
+# If the object belongs to two or more multi-part objects, all of them are
+# concatenated with a colon separator.
+modname-multi = $(subst $(space),:,$(sort $(foreach m,$(multi-used),\
+		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=)))))
 
 # Useful for describing the dependency of composite objects
 # Usage:
-- 
2.7.4

[PATCH 3/5] kbuild: fix modname for composite modules

[Masahiro Yamada]

From: Cao jin <caoj.fnst@cn.fujitsu.com>

Commit cf4f21938e13 ("kbuild: Allow to specify composite modules
with modname-m") added modname-m support, but missed to update the
corresponding multi-objs-m & modname-multi definition.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

I rebased the previous version:
https://patchwork.kernel.org/patch/10055583/


 scripts/Makefile.lib | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a1fbd6a..9217d40 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -50,7 +50,7 @@ single-used-m := $(sort $(filter-out $(multi-used-m),$(obj-m)))
 # Build list of the parts of our composite objects, our composite
 # objects depend on those (obviously)
 multi-objs-y := $(foreach m, $(multi-used-y), $($(m:.o=-objs)) $($(m:.o=-y)))
-multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)) $($(m:.o=-y)))
+multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)))
 
 # $(subdir-obj-y) is the list of objects in $(obj-y) which uses dir/ to
 # tell kbuild to descend
@@ -176,7 +176,7 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
 # If the object belongs to two or more multi-part objects, all of them are
 # concatenated with a colon separator.
 modname-multi = $(subst $(space),:,$(sort $(foreach m,$(multi-used),\
-		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=)))))
+		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m))),$(m:.o=)))))
 
 # Useful for describing the dependency of composite objects
 # Usage:
-- 
2.7.4

[PATCH 4/5] kbuild: simplify modname calculation

[Masahiro Yamada]

modname can be calculated much more simply.  If modname-multi is
empty, it is a single-used object.  So, modname = $(basetarget).
Otherwise, modname = $(modname-multi).

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

 scripts/Makefile.build | 12 +-----------
 scripts/Makefile.lib   |  7 -------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4f2b25d..b8aecb7 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -131,17 +131,7 @@ $(real-objs-m:.o=.lst): quiet_modtag := [M]
 
 $(obj-m)              : quiet_modtag := [M]
 
-# Default for not multi-part modules
-modname = $(basetarget)
-
-$(multi-objs-m)         : modname = $(modname-multi)
-$(multi-objs-m:.o=.i)   : modname = $(modname-multi)
-$(multi-objs-m:.o=.s)   : modname = $(modname-multi)
-$(multi-objs-m:.o=.lst) : modname = $(modname-multi)
-$(multi-objs-y)         : modname = $(modname-multi)
-$(multi-objs-y:.o=.i)   : modname = $(modname-multi)
-$(multi-objs-y:.o=.s)   : modname = $(modname-multi)
-$(multi-objs-y:.o=.lst) : modname = $(modname-multi)
+modname = $(if $(modname-multi),$(modname-multi),$(basetarget))
 
 quiet_cmd_cc_s_c = CC $(quiet_modtag)  $@
 cmd_cc_s_c       = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $<
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 9217d40..e4e8e1b 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -47,11 +47,6 @@ multi-used-m := $(sort $(foreach m,$(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m
 multi-used   := $(multi-used-y) $(multi-used-m)
 single-used-m := $(sort $(filter-out $(multi-used-m),$(obj-m)))
 
-# Build list of the parts of our composite objects, our composite
-# objects depend on those (obviously)
-multi-objs-y := $(foreach m, $(multi-used-y), $($(m:.o=-objs)) $($(m:.o=-y)))
-multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)))
-
 # $(subdir-obj-y) is the list of objects in $(obj-y) which uses dir/ to
 # tell kbuild to descend
 subdir-obj-y := $(filter %/built-in.o, $(obj-y))
@@ -80,8 +75,6 @@ real-objs-m	:= $(addprefix $(obj)/,$(real-objs-m))
 single-used-m	:= $(addprefix $(obj)/,$(single-used-m))
 multi-used-y	:= $(addprefix $(obj)/,$(multi-used-y))
 multi-used-m	:= $(addprefix $(obj)/,$(multi-used-m))
-multi-objs-y	:= $(addprefix $(obj)/,$(multi-objs-y))
-multi-objs-m	:= $(addprefix $(obj)/,$(multi-objs-m))
 subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
 
 # These flags are needed for modversions and compiling, so we define them here
-- 
2.7.4

[PATCH 5/5] kbuild: move modname and modname-multi close to modname_flags

[Masahiro Yamada]

Just a cosmetic change to put related code close together.

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

 scripts/Makefile.build |  2 --
 scripts/Makefile.lib   | 14 ++++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index b8aecb7..2d8f90e 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -131,8 +131,6 @@ $(real-objs-m:.o=.lst): quiet_modtag := [M]
 
 $(obj-m)              : quiet_modtag := [M]
 
-modname = $(if $(modname-multi),$(modname-multi),$(basetarget))
-
 quiet_cmd_cc_s_c = CC $(quiet_modtag)  $@
 cmd_cc_s_c       = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $<
 
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index e4e8e1b..b17f4cd 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -77,6 +77,14 @@ multi-used-y	:= $(addprefix $(obj)/,$(multi-used-y))
 multi-used-m	:= $(addprefix $(obj)/,$(multi-used-m))
 subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
 
+# Finds the multi-part object the current object will be linked into.
+# If the object belongs to two or more multi-part objects, all of them are
+# concatenated with a colon separator.
+modname-multi = $(subst $(space),:,$(sort $(foreach m,$(multi-used),\
+		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m))),$(m:.o=)))))
+
+modname = $(if $(modname-multi),$(modname-multi),$(basetarget))
+
 # These flags are needed for modversions and compiling, so we define them here
 # $(modname_flags) defines KBUILD_MODNAME as the name of the module it will
 # end up in (or would, if it gets compiled in)
@@ -165,12 +173,6 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
 		 $(addprefix -I,$(DTC_INCLUDE))                          \
 		 -undef -D__DTS__
 
-# Finds the multi-part object the current object will be linked into
-# If the object belongs to two or more multi-part objects, all of them are
-# concatenated with a colon separator.
-modname-multi = $(subst $(space),:,$(sort $(foreach m,$(multi-used),\
-		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m))),$(m:.o=)))))
-
 # Useful for describing the dependency of composite objects
 # Usage:
 #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
-- 
2.7.4

Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Cao jin]

On 03/08/2018 09:05 AM, Masahiro Yamada wrote:
> Currently, KBUILD_MODNAME is defined only when $(modname) contains
> just one word.  If an object is shared among multiple modules,
> undefined KBUILD_MODNAME could cause a build error.  For example,
> if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates
> .modname, then fails to build due to undefined KBUILD_MODNAME.
> 
> Take the following code as an example:
> 
>   obj-m += foo.o
>   obj-m += bar.o
>   foo-objs := foo-bar-common.o foo-main.o
>   bar-objs := foo-bar-common.o bar-main.o
> 
> In this case, there is room for argument what to define for
> KBUILD_MODNAME when foo-bar-common.o is being compiled.
> "foo", "bar", or what else?
> 
> One idea is to define colon-separated modules that share the object,
> in this case, "bar:foo" (modules are sorted alphabetically by
> $(sort ...).
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  scripts/Makefile.lib | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index a7e315f..a1fbd6a 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -92,8 +92,7 @@ subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
>  #       differ in different configs.
>  name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst -,_,$1))$(quote)$(squote)
>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
> -modname_flags  = $(if $(filter 1,$(words $(modname))),\
> -                 -DKBUILD_MODNAME=$(call name-fix,$(modname)))
> +modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname))
>  

I guess there is comment also need to be modified above this code hunk:

Note: Files that end up in two or more modules are compiled without the
      KBUILD_MODNAME definition. The reason is that any made-up name
      would differ in different configs.
-- 
Sincerely,
Cao jin

Re: [PATCH 1/5] kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi

[Cao jin]

On 03/08/2018 09:04 AM, Masahiro Yamada wrote:
> In the context ...
> 
>     $(obj)/%.s: $(src)/%.c FORCE
>             $(call if_changed_dep,cc_s_c)
> 
>     $(obj)/%.i: $(src)/%.c FORCE
>             $(call if_changed_dep,cpp_i_c)
> 
>     $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>             $(call cmd,force_checksrc)
>             $(call if_changed_rule,cc_o_c)
> 
>     $(obj)/%.lst: $(src)/%.c FORCE
>             $(call if_changed_dep,cc_lst_c)
> 
> '$*' returns the stem of the target (the part of '%'), so $(obj)/ has
> already been ripped off.
> 
> $(subst $(obj)/,,$*.o) is the same as $(*.o)
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  scripts/Makefile.lib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 5589bae..a7e315f 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -175,7 +175,7 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
>  
>  # Finds the multi-part object the current object will be linked into
>  modname-multi = $(sort $(foreach m,$(multi-used),\
> -		$(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
> +		$(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
>  
>  # Useful for describing the dependency of composite objects
>  # Usage:
> 

A subtle catch! And in my test, a debug line

    $(info $@ = $*)

in rule:

    $(obj)/%.o: $(src)/%.c xxx

does tell me it is correct. So,

Reviewed-by: Cao jin <caoj.fnst@cn.fujitsu.com>
-- 
Sincerely,
Cao jin

Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Masahiro Yamada]

2018-03-08 19:11 GMT+09:00 Cao jin <caoj.fnst@cn.fujitsu.com>:
>
>
> On 03/08/2018 09:05 AM, Masahiro Yamada wrote:
>> Currently, KBUILD_MODNAME is defined only when $(modname) contains
>> just one word.  If an object is shared among multiple modules,
>> undefined KBUILD_MODNAME could cause a build error.  For example,
>> if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates
>> .modname, then fails to build due to undefined KBUILD_MODNAME.
>>
>> Take the following code as an example:
>>
>>   obj-m += foo.o
>>   obj-m += bar.o
>>   foo-objs := foo-bar-common.o foo-main.o
>>   bar-objs := foo-bar-common.o bar-main.o
>>
>> In this case, there is room for argument what to define for
>> KBUILD_MODNAME when foo-bar-common.o is being compiled.
>> "foo", "bar", or what else?
>>
>> One idea is to define colon-separated modules that share the object,
>> in this case, "bar:foo" (modules are sorted alphabetically by
>> $(sort ...).
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  scripts/Makefile.lib | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index a7e315f..a1fbd6a 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -92,8 +92,7 @@ subdir-ym   := $(addprefix $(obj)/,$(subdir-ym))
>>  #       differ in different configs.
>>  name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst -,_,$1))$(quote)$(squote)
>>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
>> -modname_flags  = $(if $(filter 1,$(words $(modname))),\
>> -                 -DKBUILD_MODNAME=$(call name-fix,$(modname)))
>> +modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname))
>>
>
> I guess there is comment also need to be modified above this code hunk:
>
> Note: Files that end up in two or more modules are compiled without the
>       KBUILD_MODNAME definition. The reason is that any made-up name
>       would differ in different configs.


Why do I have to add lying comments here?

The commit subject/log claims KBUILD_MODNAME should be _always_ defined.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/5] kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi

[Masahiro Yamada]

2018-03-08 10:04 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> In the context ...
>
>     $(obj)/%.s: $(src)/%.c FORCE
>             $(call if_changed_dep,cc_s_c)
>
>     $(obj)/%.i: $(src)/%.c FORCE
>             $(call if_changed_dep,cpp_i_c)
>
>     $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>             $(call cmd,force_checksrc)
>             $(call if_changed_rule,cc_o_c)
>
>     $(obj)/%.lst: $(src)/%.c FORCE
>             $(call if_changed_dep,cc_lst_c)
>
> '$*' returns the stem of the target (the part of '%'), so $(obj)/ has
> already been ripped off.
>
> $(subst $(obj)/,,$*.o) is the same as $(*.o)


Log fix-up:

 ... is the same as $*.o





> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/Makefile.lib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 5589bae..a7e315f 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -175,7 +175,7 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
>
>  # Finds the multi-part object the current object will be linked into
>  modname-multi = $(sort $(foreach m,$(multi-used),\
> -               $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
> +               $(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=))))
>
>  # Useful for describing the dependency of composite objects
>  # Usage:
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Cao jin]

Masahiro-san

On 03/08/2018 06:21 PM, Masahiro Yamada wrote:
> 2018-03-08 19:11 GMT+09:00 Cao jin <caoj.fnst@cn.fujitsu.com>:
>>
>>
>> On 03/08/2018 09:05 AM, Masahiro Yamada wrote:
>>> Currently, KBUILD_MODNAME is defined only when $(modname) contains
>>> just one word.  If an object is shared among multiple modules,
>>> undefined KBUILD_MODNAME could cause a build error.  For example,
>>> if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates
>>> .modname, then fails to build due to undefined KBUILD_MODNAME.
>>>
>>> Take the following code as an example:
>>>
>>>   obj-m += foo.o
>>>   obj-m += bar.o
>>>   foo-objs := foo-bar-common.o foo-main.o
>>>   bar-objs := foo-bar-common.o bar-main.o
>>>
>>> In this case, there is room for argument what to define for
>>> KBUILD_MODNAME when foo-bar-common.o is being compiled.
>>> "foo", "bar", or what else?
>>>
>>> One idea is to define colon-separated modules that share the object,
>>> in this case, "bar:foo" (modules are sorted alphabetically by
>>> $(sort ...).
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  scripts/Makefile.lib | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>> index a7e315f..a1fbd6a 100644
>>> --- a/scripts/Makefile.lib
>>> +++ b/scripts/Makefile.lib
>>> @@ -92,8 +92,7 @@ subdir-ym   := $(addprefix $(obj)/,$(subdir-ym))
>>>  #       differ in different configs.
>>>  name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst -,_,$1))$(quote)$(squote)
>>>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
>>> -modname_flags  = $(if $(filter 1,$(words $(modname))),\
>>> -                 -DKBUILD_MODNAME=$(call name-fix,$(modname)))
>>> +modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname))
>>>
>>
>> I guess there is comment also need to be modified above this code hunk:
>>
>> Note: Files that end up in two or more modules are compiled without the
>>       KBUILD_MODNAME definition. The reason is that any made-up name
>>       would differ in different configs.
> 
> 
> Why do I have to add lying comments here?
> 
> The commit subject/log claims KBUILD_MODNAME should be _always_ defined.
> 

I feel you misunderstand my intention, the comment I mentioned is
already there, so they should be modified according to your patch.

-- 
Sincerely,
Cao jin

Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Masahiro Yamada]

2018-03-08 19:25 GMT+09:00 Cao jin <caoj.fnst@cn.fujitsu.com>:
> Masahiro-san
>
> On 03/08/2018 06:21 PM, Masahiro Yamada wrote:
>> 2018-03-08 19:11 GMT+09:00 Cao jin <caoj.fnst@cn.fujitsu.com>:
>>>
>>>
>>> On 03/08/2018 09:05 AM, Masahiro Yamada wrote:
>>>> Currently, KBUILD_MODNAME is defined only when $(modname) contains
>>>> just one word.  If an object is shared among multiple modules,
>>>> undefined KBUILD_MODNAME could cause a build error.  For example,
>>>> if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates
>>>> .modname, then fails to build due to undefined KBUILD_MODNAME.
>>>>
>>>> Take the following code as an example:
>>>>
>>>>   obj-m += foo.o
>>>>   obj-m += bar.o
>>>>   foo-objs := foo-bar-common.o foo-main.o
>>>>   bar-objs := foo-bar-common.o bar-main.o
>>>>
>>>> In this case, there is room for argument what to define for
>>>> KBUILD_MODNAME when foo-bar-common.o is being compiled.
>>>> "foo", "bar", or what else?
>>>>
>>>> One idea is to define colon-separated modules that share the object,
>>>> in this case, "bar:foo" (modules are sorted alphabetically by
>>>> $(sort ...).
>>>>
>>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>> ---
>>>>
>>>>  scripts/Makefile.lib | 9 +++++----
>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>>> index a7e315f..a1fbd6a 100644
>>>> --- a/scripts/Makefile.lib
>>>> +++ b/scripts/Makefile.lib
>>>> @@ -92,8 +92,7 @@ subdir-ym   := $(addprefix $(obj)/,$(subdir-ym))
>>>>  #       differ in different configs.
>>>>  name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst -,_,$1))$(quote)$(squote)
>>>>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
>>>> -modname_flags  = $(if $(filter 1,$(words $(modname))),\
>>>> -                 -DKBUILD_MODNAME=$(call name-fix,$(modname)))
>>>> +modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname))
>>>>
>>>
>>> I guess there is comment also need to be modified above this code hunk:
>>>
>>> Note: Files that end up in two or more modules are compiled without the
>>>       KBUILD_MODNAME definition. The reason is that any made-up name
>>>       would differ in different configs.
>>
>>
>> Why do I have to add lying comments here?
>>
>> The commit subject/log claims KBUILD_MODNAME should be _always_ defined.
>>
>
> I feel you misunderstand my intention, the comment I mentioned is
> already there, so they should be modified according to your patch.
>

Ugh.  Sorry I missed your intention.

I will remove the comment.  Thanks!



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 4/5] kbuild: simplify modname calculation

[Cao jin]

On 03/08/2018 09:05 AM, Masahiro Yamada wrote:
> modname can be calculated much more simply.  If modname-multi is
> empty, it is a single-used object.  So, modname = $(basetarget).> Otherwise, modname = $(modname-multi).
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  scripts/Makefile.build | 12 +-----------
>  scripts/Makefile.lib   |  7 -------
>  2 files changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 4f2b25d..b8aecb7 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -131,17 +131,7 @@ $(real-objs-m:.o=.lst): quiet_modtag := [M]
>  
>  $(obj-m)              : quiet_modtag := [M]
>  
> -# Default for not multi-part modules
> -modname = $(basetarget)
> -
> -$(multi-objs-m)         : modname = $(modname-multi)
> -$(multi-objs-m:.o=.i)   : modname = $(modname-multi)
> -$(multi-objs-m:.o=.s)   : modname = $(modname-multi)
> -$(multi-objs-m:.o=.lst) : modname = $(modname-multi)
> -$(multi-objs-y)         : modname = $(modname-multi)
> -$(multi-objs-y:.o=.i)   : modname = $(modname-multi)
> -$(multi-objs-y:.o=.s)   : modname = $(modname-multi)
> -$(multi-objs-y:.o=.lst) : modname = $(modname-multi)
> +modname = $(if $(modname-multi),$(modname-multi),$(basetarget))
>  
>  quiet_cmd_cc_s_c = CC $(quiet_modtag)  $@
>  cmd_cc_s_c       = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $<
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 9217d40..e4e8e1b 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -47,11 +47,6 @@ multi-used-m := $(sort $(foreach m,$(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m
>  multi-used   := $(multi-used-y) $(multi-used-m)
>  single-used-m := $(sort $(filter-out $(multi-used-m),$(obj-m)))
>  
> -# Build list of the parts of our composite objects, our composite
> -# objects depend on those (obviously)
> -multi-objs-y := $(foreach m, $(multi-used-y), $($(m:.o=-objs)) $($(m:.o=-y)))
> -multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)))
> -
>  # $(subdir-obj-y) is the list of objects in $(obj-y) which uses dir/ to
>  # tell kbuild to descend
>  subdir-obj-y := $(filter %/built-in.o, $(obj-y))
> @@ -80,8 +75,6 @@ real-objs-m	:= $(addprefix $(obj)/,$(real-objs-m))
>  single-used-m	:= $(addprefix $(obj)/,$(single-used-m))
>  multi-used-y	:= $(addprefix $(obj)/,$(multi-used-y))
>  multi-used-m	:= $(addprefix $(obj)/,$(multi-used-m))
> -multi-objs-y	:= $(addprefix $(obj)/,$(multi-objs-y))
> -multi-objs-m	:= $(addprefix $(obj)/,$(multi-objs-m))
>  subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
>  
>  # These flags are needed for modversions and compiling, so we define them here
> 

multi-objs-y/m seems only exist for rules above with target-specific
variable assignment. A great simplification, So

Reviewed-by: Cao jin <caoj.fnst@cn.fujitsu.com>
-- 
Sincerely,
Cao jin

Re: [PATCH 5/5] kbuild: move modname and modname-multi close to modname_flags

[Cao jin]

On 03/08/2018 09:05 AM, Masahiro Yamada wrote:
> Just a cosmetic change to put related code close together.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Cao jin <caoj.fnst@cn.fujitsu.com>

-- 
Sincerely,
Cao jin

Re: [PATCH 0/5] kbuild: always define KBUILD_MODNAME, and clean-up/fix modname

[Cao jin]

The series build successfully on upstream in my: make allyesconfig and
allmodconfig, so,

Tested-by: Cao jin <caoj.fnst@cn.fujitsu.com>

-- 
Sincerely,
Cao jin

On 03/08/2018 09:04 AM, Masahiro Yamada wrote:
> 
> 3/5 takes into account '-m' case for multi-used-m.
> 
> 2/5 is necessary beforehand because 3/5 would cause a build error for
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
> 
> 1, 4, 5 are just clean-ups.
> 
> 
> 
> Cao jin (1):
>   kbuild: fix modname for composite modules
> 
> Masahiro Yamada (4):
>   kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi
>   kbuild: define KBUILD_MODNAME even if multiple modules share objects
>   kbuild: simplify modname calculation
>   kbuild: move modname and modname-multi close to modname_flags
> 
>  scripts/Makefile.build | 12 ------------
>  scripts/Makefile.lib   | 22 +++++++++-------------
>  2 files changed, 9 insertions(+), 25 deletions(-)
>