Re: [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/*

[Masahiro Yamada <yamada.masahiro@socionext.com>]

2018-03-15 19:04 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2018-03-15 3:47 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
>> On Thu, 15 Mar 2018, Masahiro Yamada wrote:
>>
>>> The idea of using fixdep was inspired by Kconfig, but autoksyms
>>> is unrelated to Kconfig.  So, I want to get those touched files
>>> out of include/config/.  The directory include/ksym/ is removed
>>> by "make clean".  We do not need to keep it for external module
>>> building.
>>
>> It could be argued that include/config/ is not strictly containing
>> configuration data either and is slightly misleading.
>
> But, slightly related to configuration, IMHO.
> At least they carry timestamps that are updated
> when kernel configuration is changed.
>
> The difference between include/config/ and include/ksym/ is that
> files under include/config/ are necessary for building
> external modules (so should be cleaned away by mrproper)
> whereas include/ksym/ is unnecessary for external modules
> since vmlinux and in-kernel modules do not depend on
> external modules.
>
>
> I wonder if trimming symbols makes sense for external modules.
>
> EXPORT_SYMBOL(_GPL) in external modules are always trimmed
> since vmlinux and in-kernel modules never rely on them.
>
> If an external module exports symbols,
> it expects they will be used by other external modules.
>
> So, the patch like follows make sense?
>
>
>
>
> kbuild: do not trim symbols in external modules
>
> diff --git a/Makefile b/Makefile
> index 0a7bab6..bdf565e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -998,6 +998,7 @@ autoksyms_recursive: $(vmlinux-deps)
>  # (this can be evaluated only once include/config/auto.conf has been included)
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>    KBUILD_MODULES := 1
> +  export KBUILD_TRIM_UNUSED_KSYMS := 1
>  endif
>
>  autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 045971e..3249d5f 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -345,7 +345,7 @@ if_changed_dep = $(if $(strip $(any-prereq)
> $(arg-check) ),                  \
>         @set -e;                                                             \
>         $(cmd_and_fixdep), @:)
>
> -ifndef CONFIG_TRIM_UNUSED_KSYMS
> +ifndef KBUILD_TRIM_UNUSED_KSYMS
>
>  cmd_and_fixdep =                                                             \
>         $(echo-cmd) $(cmd_$(1));                                             \
>
>
>
>
>> What about moving include/config/ and include/ksym/ under
>> include/depfiles/ ? In fact that could even be
>> include/generated/depfiles/config/ and include/generated/depfiles/ksym/
>> to trim down the top include directory.
>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  .gitignore                  | 1 +
>>>  Makefile                    | 2 +-
>>>  scripts/Kbuild.include      | 2 +-
>>>  scripts/adjust_autoksyms.sh | 2 +-
>>>  scripts/basic/fixdep.c      | 8 ++++----
>>>  5 files changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index 1be78fd..85bcc26 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -87,6 +87,7 @@ modules.builtin
>>>  #
>>>  include/config
>>>  include/generated
>>> +include/ksym
>>>  arch/*/include/generated
>>>
>>>  # stgit generated dirs
>>> diff --git a/Makefile b/Makefile
>>> index e60b16f..1dab647 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1327,7 +1327,7 @@ endif # CONFIG_MODULES
>>>  # make distclean Remove editor backup files, patch leftover files and the like
>>>
>>>  # Directories & files removed with 'make clean'
>>> -CLEAN_DIRS  += $(MODVERDIR)
>>> +CLEAN_DIRS  += $(MODVERDIR) include/ksym
>>>
>>>  # Directories & files removed with 'make mrproper'
>>>  MRPROPER_DIRS  += include/config usr/include include/generated          \
>>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
>>> index 065324a..045971e 100644
>>> --- a/scripts/Kbuild.include
>>> +++ b/scripts/Kbuild.include
>>> @@ -368,7 +368,7 @@ ksym_dep_filter =                                                            \
>>>           $(CPP) $(call flags_nodeps,a_flags) -D__KSYM_DEPS__ $< ;;        \
>>>         boot*|build*|cpp_its_S|*cpp_lds_S|dtc|host*|vdso*) : ;;            \
>>>         *) echo "Don't know how to preprocess $(1)" >&2; false ;;          \
>>> -     esac | tr ";" "\n" | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
>>> +     esac | tr ";" "\n" | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/\1/p'
>>>
>>>  cmd_and_fixdep =                                                             \
>>>       $(echo-cmd) $(cmd_$(1));                                             \
>>> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>>> index a52210b..7bb3618 100755
>>> --- a/scripts/adjust_autoksyms.sh
>>> +++ b/scripts/adjust_autoksyms.sh
>>> @@ -81,7 +81,7 @@ sort "$cur_ksyms_file" "$new_ksyms_file" | uniq -u |
>>>  sed -n 's/^#define __KSYM_\(.*\) 1/\1/p' | tr "A-Z_" "a-z/" |
>>>  while read sympath; do
>>>       if [ -z "$sympath" ]; then continue; fi
>>> -     depfile="include/config/ksym/${sympath}.h"
>>> +     depfile="include/ksym/${sympath}.h"
>>>       mkdir -p "$(dirname "$depfile")"
>>>       touch "$depfile"
>>>       echo $((count += 1))
>>> diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
>>> index 449b68c..f387538 100644
>>> --- a/scripts/basic/fixdep.c
>>> +++ b/scripts/basic/fixdep.c
>>> @@ -113,11 +113,11 @@ static void usage(void)
>>>  /*
>>>   * Print out a dependency path from a symbol name
>>>   */
>>> -static void print_config(const char *m, int slen)
>>> +static void print_dep(const char *m, int slen, const char *dir)
>>>  {
>>>       int c, i;
>>>
>>> -     printf("    $(wildcard include/config/");
>>> +     printf("    $(wildcard %s/", dir);
>>>       for (i = 0; i < slen; i++) {
>>>               c = m[i];
>>>               if (c == '_')
>>> @@ -140,7 +140,7 @@ static void do_extra_deps(void)
>>>                       fprintf(stderr, "fixdep: bad data on stdin\n");
>>>                       exit(1);
>>>               }
>>> -             print_config(buf, len - 1);
>>> +             print_dep(buf, len - 1, "include/ksym");
>>>       }
>>>  }
>>>
>>> @@ -208,7 +208,7 @@ static void use_config(const char *m, int slen)
>>>           return;
>>>
>>>       define_config(m, slen, hash);
>>> -     print_config(m, slen);
>>> +     print_dep(m, slen, "include/config");
>>>  }
>>>
>>>  /* test if s ends in sub */
>>> --


No.  All exported symbols in external modules are
still trimmed.

Hmm.  Probably, this is a "Do not do it" thing.



-- 
Best Regards
Masahiro Yamada