* [PATCH v3 01/12] kconfig: rename file_write_dep and move it to confdata.c
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 02/12] kconfig: split out helpers to check file/directory, create directory Masahiro Yamada
` (11 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, linux-kernel
file_write_dep() is called only from conf_write_autoconf().
Move it from util.c to confdata.c to make it static.
Also, rename it to conf_write_dep() since it should belong to
the group of conf_write* functions.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/confdata.c | 31 ++++++++++++++++++++++++++++++-
scripts/kconfig/lkc.h | 1 -
scripts/kconfig/util.c | 30 ------------------------------
3 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 39e2097..4771820 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -813,6 +813,35 @@ int conf_write(const char *name)
return 0;
}
+/* write a dependency file as used by kbuild to track dependencies */
+static int conf_write_dep(const char *name)
+{
+ struct file *file;
+ FILE *out;
+
+ if (!name)
+ name = ".kconfig.d";
+ out = fopen("..config.tmp", "w");
+ if (!out)
+ return 1;
+ fprintf(out, "deps_config := \\\n");
+ for (file = file_list; file; file = file->next) {
+ if (file->next)
+ fprintf(out, "\t%s \\\n", file->name);
+ else
+ fprintf(out, "\t%s\n", file->name);
+ }
+ fprintf(out, "\n%s: \\\n"
+ "\t$(deps_config)\n\n", conf_get_autoconfig_name());
+
+ env_write_dep(out, conf_get_autoconfig_name());
+
+ fprintf(out, "\n$(deps_config): ;\n");
+ fclose(out);
+ rename("..config.tmp", name);
+ return 0;
+}
+
static int conf_split_config(void)
{
const char *name;
@@ -935,7 +964,7 @@ int conf_write_autoconf(void)
sym_clear_all_valid();
- file_write_dep("include/config/auto.conf.cmd");
+ conf_write_dep("include/config/auto.conf.cmd");
if (conf_split_config())
return 1;
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index ed3ff88..6b7bbc6 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -97,7 +97,6 @@ void menu_set_type(int type);
/* util.c */
struct file *file_lookup(const char *name);
-int file_write_dep(const char *name);
void *xmalloc(size_t size);
void *xcalloc(size_t nmemb, size_t size);
void *xrealloc(void *p, size_t size);
diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
index a365594..d999683 100644
--- a/scripts/kconfig/util.c
+++ b/scripts/kconfig/util.c
@@ -29,36 +29,6 @@ struct file *file_lookup(const char *name)
return file;
}
-/* write a dependency file as used by kbuild to track dependencies */
-int file_write_dep(const char *name)
-{
- struct file *file;
- FILE *out;
-
- if (!name)
- name = ".kconfig.d";
- out = fopen("..config.tmp", "w");
- if (!out)
- return 1;
- fprintf(out, "deps_config := \\\n");
- for (file = file_list; file; file = file->next) {
- if (file->next)
- fprintf(out, "\t%s \\\n", file->name);
- else
- fprintf(out, "\t%s\n", file->name);
- }
- fprintf(out, "\n%s: \\\n"
- "\t$(deps_config)\n\n", conf_get_autoconfig_name());
-
- env_write_dep(out, conf_get_autoconfig_name());
-
- fprintf(out, "\n$(deps_config): ;\n");
- fclose(out);
- rename("..config.tmp", name);
- return 0;
-}
-
-
/* Allocate initial growable string */
struct gstr str_new(void)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 02/12] kconfig: split out helpers to check file/directory, create directory
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 01/12] kconfig: rename file_write_dep and move it to confdata.c Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 03/12] kconfig: remove unneeded directory generation from local*config Masahiro Yamada
` (10 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, linux-kernel
Split out helpers:
is_file() - check if the given path exists and it is a regular file
is_dir() - check if the given path exists and it is a directory
mkdir_p() - create the parent directories of the given path
These helpers will be reused in later commits.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/confdata.c | 85 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 68 insertions(+), 17 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 4771820..881993e 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -16,6 +16,68 @@
#include "lkc.h"
+/* return true if 'path' exists and it is a regular file, false otherwise */
+static bool is_file(const char *path)
+{
+ struct stat st;
+
+ if (stat(path, &st))
+ return 0;
+
+ return S_ISREG(st.st_mode);
+}
+
+/* return true if 'path' exists and it is a directory, false otherwise */
+static bool is_dir(const char *path)
+{
+ struct stat st;
+
+ if (stat(path, &st))
+ return 0;
+
+ return S_ISDIR(st.st_mode);
+}
+
+/*
+ * Create the parent directory of the given path.
+ *
+ * For example, if 'include/config/auto.conf' is given, create 'include/config'.
+ * If the path ends with '/' like 'include/config/', create 'include/config'.
+ */
+static int mkdir_p(const char *path)
+{
+ char tmp[PATH_MAX + 1];
+ char *p;
+
+ strncpy(tmp, path, sizeof(tmp));
+ tmp[sizeof(tmp) - 1] = 0;
+
+ /* Remove the base name. Just return if nothing is left */
+ p = strrchr(tmp, '/');
+ if (!p)
+ return 0;
+ *(p + 1) = 0;
+
+ /* Just in case it is an absolute path */
+ p = tmp;
+ while (*p == '/')
+ p++;
+
+ while ((p = strchr(p, '/'))) {
+ *p = 0;
+
+ /* skip if the directory exists */
+ if (!is_dir(tmp) && mkdir(tmp, 0755))
+ return -1;
+
+ *p = '/';
+ while (*p == '/')
+ p++;
+ }
+
+ return 0;
+}
+
struct conf_printer {
void (*print_symbol)(FILE *, struct symbol *, const char *, void *);
void (*print_comment)(FILE *, const char *, void *);
@@ -83,7 +145,6 @@ const char *conf_get_autoconfig_name(void)
char *conf_get_default_confname(void)
{
- struct stat buf;
static char fullname[PATH_MAX+1];
char *env, *name;
@@ -91,7 +152,7 @@ char *conf_get_default_confname(void)
env = getenv(SRCTREE);
if (env) {
sprintf(fullname, "%s/%s", env, name);
- if (!stat(fullname, &buf))
+ if (is_file(fullname))
return fullname;
}
return name;
@@ -725,10 +786,9 @@ int conf_write(const char *name)
dirname[0] = 0;
if (name && name[0]) {
- struct stat st;
char *slash;
- if (!stat(name, &st) && S_ISDIR(st.st_mode)) {
+ if (is_dir(name)) {
strcpy(dirname, name);
strcat(dirname, "/");
basename = conf_get_configname();
@@ -848,7 +908,6 @@ static int conf_split_config(void)
char path[PATH_MAX+1];
char *s, *d, c;
struct symbol *sym;
- struct stat sb;
int res, i, fd;
name = conf_get_autoconfig_name();
@@ -926,18 +985,10 @@ static int conf_split_config(void)
res = 1;
break;
}
- /*
- * Create directory components,
- * unless they exist already.
- */
- d = path;
- while ((d = strchr(d, '/'))) {
- *d = 0;
- if (stat(path, &sb) && mkdir(path, 0755)) {
- res = 1;
- goto out;
- }
- *d++ = '/';
+
+ if (mkdir_p(path)) {
+ res = 1;
+ goto out;
}
/* Try it again. */
fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 03/12] kconfig: remove unneeded directory generation from local*config
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 01/12] kconfig: rename file_write_dep and move it to confdata.c Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 02/12] kconfig: split out helpers to check file/directory, create directory Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 04/12] kconfig: create directories needed for syncconfig by itself Masahiro Yamada
` (9 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, linux-kernel
Commit 17263baf958b ("kconfig: Create include/generated for
localmodconfig") added the 'mkdir' line because local{yes,mod}config
ran streamline_config.pl followed by silentoldconfig at that time.
Since commit 81d2bc227305 ("kconfig: invoke oldconfig instead of
silentoldconfig from local*config"), no sub-directory is required.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index a3ac2c9..c9e8cf5 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -41,7 +41,6 @@ syncconfig: $(obj)/conf
$< $(silent) --$@ $(Kconfig)
localyesconfig localmodconfig: $(obj)/conf
- $(Q)mkdir -p include/config include/generated
$(Q)perl $(srctree)/$(src)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
$(Q)if [ -f .config ]; then \
cmp -s .tmp.config .config || \
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 04/12] kconfig: create directories needed for syncconfig by itself
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (2 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 03/12] kconfig: remove unneeded directory generation from local*config Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count Masahiro Yamada
` (8 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, linux-kernel
'make syncconfig' creates some files such as include/config/auto.conf,
include/generate/autoconf.h, etc. but the necessary directory creation
relies on scripts/kconfig/Makefile.
To make Kconfig self-contained, create directories as needed in
conf_write_autoconf().
This change allows scripts/kconfig/Makefile cleanups; syncconfig can
be merged into simple-targets.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/Makefile | 15 ++++++---------
scripts/kconfig/confdata.c | 14 ++++++++++++++
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index c9e8cf5..dffe268 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -3,8 +3,7 @@
# Kernel configuration targets
# These targets are used from top-level makefile
-PHONY += xconfig gconfig menuconfig config syncconfig \
- localmodconfig localyesconfig
+PHONY += xconfig gconfig menuconfig config localmodconfig localyesconfig
ifdef KBUILD_KCONFIG
Kconfig := $(KBUILD_KCONFIG)
@@ -34,12 +33,6 @@ config: $(obj)/conf
nconfig: $(obj)/nconf
$< $(silent) $(Kconfig)
-# This has become an internal implementation detail and is now deprecated
-# for external use.
-syncconfig: $(obj)/conf
- $(Q)mkdir -p include/config include/generated
- $< $(silent) --$@ $(Kconfig)
-
localyesconfig localmodconfig: $(obj)/conf
$(Q)perl $(srctree)/$(src)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
$(Q)if [ -f .config ]; then \
@@ -55,8 +48,12 @@ localyesconfig localmodconfig: $(obj)/conf
$(Q)rm -f .tmp.config
# These targets map 1:1 to the commandline options of 'conf'
+#
+# Note:
+# syncconfig has become an internal implementation detail and is now
+# deprecated for external use
simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
- alldefconfig randconfig listnewconfig olddefconfig
+ alldefconfig randconfig listnewconfig olddefconfig syncconfig
PHONY += $(simple-targets)
$(simple-targets): $(obj)/conf
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 881993e..38b3a6c 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -898,6 +898,9 @@ static int conf_write_dep(const char *name)
fprintf(out, "\n$(deps_config): ;\n");
fclose(out);
+
+ if (mkdir_p(name))
+ return 1;
rename("..config.tmp", name);
return 0;
}
@@ -914,6 +917,8 @@ static int conf_split_config(void)
conf_read_simple(name, S_DEF_AUTO);
sym_calc_value(modules_sym);
+ if (mkdir_p("include/config/"))
+ return 1;
if (chdir("include/config"))
return 1;
@@ -990,6 +995,7 @@ static int conf_split_config(void)
res = 1;
goto out;
}
+
/* Try it again. */
fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
if (fd == -1) {
@@ -1062,14 +1068,22 @@ int conf_write_autoconf(void)
name = getenv("KCONFIG_AUTOHEADER");
if (!name)
name = "include/generated/autoconf.h";
+ if (mkdir_p(name))
+ return 1;
if (rename(".tmpconfig.h", name))
return 1;
+
name = getenv("KCONFIG_TRISTATE");
if (!name)
name = "include/config/tristate.conf";
+ if (mkdir_p(name))
+ return 1;
if (rename(".tmpconfig_tristate", name))
return 1;
+
name = conf_get_autoconfig_name();
+ if (mkdir_p(name))
+ return 1;
/*
* This must be the last step, kbuild has a dependency on auto.conf
* and this marks the successful completion of the previous steps.
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (3 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 04/12] kconfig: create directories needed for syncconfig by itself Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-06 12:23 ` Dirk Gouders
2018-07-05 2:39 ` [PATCH v3 06/12] kconfig: allow all config targets to write auto.conf if missing Masahiro Yamada
` (7 subsequent siblings)
12 siblings, 1 reply; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, linux-kernel
syncconfig updates the .config only when sym_change_count > 0, i.e.
any change in config symbols has been detected.
Not only symbols but also comments are contained in the .config file.
If only comments are updated, they are not fed back to the .config,
then the stale comments are left-over. Of course, this is just a
matter of comments, but why not fix it.
I see some scenarios where this happens.
Scenario A:
1. You have a source tree that has already been configured.
2. Linus increments the version number in the top-level Makefile
(i.e. he commits a new release)
3. You pull it, and run 'make'
4. syncconfig is invoked because the environment variable,
KERNELVERSION is updated, but the .config is not updated since
no config symbol is changed.
5. The .config file contains a kernel version in the top line:
# Automatically generated file; DO NOT EDIT.
# Linux/arm64 4.18.0-rc2 Kernel Configuration
... which points to a previous version.
Scenario B:
1. You have a source tree that has already been configured.
2. You upgrade the compiler, but it still has the same version number.
This may happen if you regularly build the latest compiler from
the source code.
3. You run 'make'
4. syncconfig is invoked because the environment variable,
CC_VERSION_TEXT is updated, but the .config is not updated since
no config symbol is changed.
5. The .config file contains the version string of the compiler:
#
# Compiler: aarch64-linux-gcc (GCC) 9.0.0 20180628 (experimental)
#
... which carries the information of the old compiler.
If KCONFIG_NOSILENTUPDATE is set, syncconfig is not allowed to update
the .config file. Otherwise, it is fine to update it regardless of
sym_change_count.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/conf.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 671ff53..5af8991 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -496,6 +496,7 @@ int main(int ac, char **av)
int opt;
const char *name, *defconfig_file = NULL /* gcc uninit */;
struct stat tmpstat;
+ int no_conf_write = 0;
tty_stdio = isatty(0) && isatty(1);
@@ -633,13 +634,14 @@ int main(int ac, char **av)
}
if (sync_kconfig) {
- if (conf_get_changed()) {
- name = getenv("KCONFIG_NOSILENTUPDATE");
- if (name && *name) {
+ name = getenv("KCONFIG_NOSILENTUPDATE");
+ if (name && *name) {
+ if (conf_get_changed()) {
fprintf(stderr,
"\n*** The configuration requires explicit update.\n\n");
return 1;
}
+ no_conf_write = 1;
}
}
@@ -688,7 +690,7 @@ int main(int ac, char **av)
/* syncconfig is used during the build so we shall update autoconf.
* All other commands are only used to generate a config.
*/
- if (conf_get_changed() && conf_write(NULL)) {
+ if (!no_conf_write && conf_write(NULL)) {
fprintf(stderr, "\n*** Error during writing of the configuration.\n\n");
exit(1);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-05 2:39 ` [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count Masahiro Yamada
@ 2018-07-06 12:23 ` Dirk Gouders
2018-07-06 23:38 ` Dirk Gouders
0 siblings, 1 reply; 25+ messages in thread
From: Dirk Gouders @ 2018-07-06 12:23 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Ulf Magnusson, Linus Torvalds, Sam Ravnborg, linux-kernel
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> syncconfig updates the .config only when sym_change_count > 0, i.e.
> any change in config symbols has been detected.
>
> Not only symbols but also comments are contained in the .config file.
> If only comments are updated, they are not fed back to the .config,
> then the stale comments are left-over. Of course, this is just a
> matter of comments, but why not fix it.
Hello Masahiro,
I am currently looking at and testing this series.
First: For this patch I would suggest to also edit the syncconfig
section of "conf --help".
Further, on a slow laptop, I was suspecting, this patch to cause full
rebuilds of everything, each time I ran "make syncconfig" followed by
"make" but could not verify this on another machine, so perhaps I am
just (for testing purposes) removing the wrong files (modules.builtin
for example) -- I am still testing.
But, what irritates me with testing is that (also without your
patches) two consecutive "make" produce different output, one of them
always shows a warning and this is reproducable. I just want to make
sure there is no other problem that influences my testing:
$ make
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
DATAREL arch/x86/boot/compressed/vmlinux
Kernel: arch/x86/boot/bzImage is ready (#15)
Building modules, stage 2.
MODPOST 211 modules
$ make
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
LD arch/x86/boot/compressed/vmlinux
ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
ld: warning: creating a DT_TEXTREL in object.
ZOFFSET arch/x86/boot/zoffset.h
AS arch/x86/boot/header.o
LD arch/x86/boot/setup.elf
OBJCOPY arch/x86/boot/setup.bin
OBJCOPY arch/x86/boot/vmlinux.bin
BUILD arch/x86/boot/bzImage
Setup is 15580 bytes (padded to 15872 bytes).
System is 8069 kB
CRC e01d75ec
Kernel: arch/x86/boot/bzImage is ready (#15)
Building modules, stage 2.
MODPOST 211 modules
Dirk
> I see some scenarios where this happens.
>
> Scenario A:
>
> 1. You have a source tree that has already been configured.
>
> 2. Linus increments the version number in the top-level Makefile
> (i.e. he commits a new release)
>
> 3. You pull it, and run 'make'
>
> 4. syncconfig is invoked because the environment variable,
> KERNELVERSION is updated, but the .config is not updated since
> no config symbol is changed.
>
> 5. The .config file contains a kernel version in the top line:
>
> # Automatically generated file; DO NOT EDIT.
> # Linux/arm64 4.18.0-rc2 Kernel Configuration
>
> ... which points to a previous version.
>
> Scenario B:
>
> 1. You have a source tree that has already been configured.
>
> 2. You upgrade the compiler, but it still has the same version number.
> This may happen if you regularly build the latest compiler from
> the source code.
>
> 3. You run 'make'
>
> 4. syncconfig is invoked because the environment variable,
> CC_VERSION_TEXT is updated, but the .config is not updated since
> no config symbol is changed.
>
> 5. The .config file contains the version string of the compiler:
>
> #
> # Compiler: aarch64-linux-gcc (GCC) 9.0.0 20180628 (experimental)
> #
>
> ... which carries the information of the old compiler.
>
> If KCONFIG_NOSILENTUPDATE is set, syncconfig is not allowed to update
> the .config file. Otherwise, it is fine to update it regardless of
> sym_change_count.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> scripts/kconfig/conf.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 671ff53..5af8991 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -496,6 +496,7 @@ int main(int ac, char **av)
> int opt;
> const char *name, *defconfig_file = NULL /* gcc uninit */;
> struct stat tmpstat;
> + int no_conf_write = 0;
>
> tty_stdio = isatty(0) && isatty(1);
>
> @@ -633,13 +634,14 @@ int main(int ac, char **av)
> }
>
> if (sync_kconfig) {
> - if (conf_get_changed()) {
> - name = getenv("KCONFIG_NOSILENTUPDATE");
> - if (name && *name) {
> + name = getenv("KCONFIG_NOSILENTUPDATE");
> + if (name && *name) {
> + if (conf_get_changed()) {
> fprintf(stderr,
> "\n*** The configuration requires explicit update.\n\n");
> return 1;
> }
> + no_conf_write = 1;
> }
> }
>
> @@ -688,7 +690,7 @@ int main(int ac, char **av)
> /* syncconfig is used during the build so we shall update autoconf.
> * All other commands are only used to generate a config.
> */
> - if (conf_get_changed() && conf_write(NULL)) {
> + if (!no_conf_write && conf_write(NULL)) {
> fprintf(stderr, "\n*** Error during writing of the configuration.\n\n");
> exit(1);
> }
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-06 12:23 ` Dirk Gouders
@ 2018-07-06 23:38 ` Dirk Gouders
2018-07-09 11:39 ` Dirk Gouders
0 siblings, 1 reply; 25+ messages in thread
From: Dirk Gouders @ 2018-07-06 23:38 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
linux-kernel, Kees Cook, Ingo Molnar
Dirk Gouders <dirk@gouders.net> writes:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
>> syncconfig updates the .config only when sym_change_count > 0, i.e.
>> any change in config symbols has been detected.
>>
>> Not only symbols but also comments are contained in the .config file.
>> If only comments are updated, they are not fed back to the .config,
>> then the stale comments are left-over. Of course, this is just a
>> matter of comments, but why not fix it.
>
> Hello Masahiro,
>
> I am currently looking at and testing this series.
>
> First: For this patch I would suggest to also edit the syncconfig
> section of "conf --help".
>
> Further, on a slow laptop, I was suspecting, this patch to cause full
> rebuilds of everything, each time I ran "make syncconfig" followed by
> "make" but could not verify this on another machine, so perhaps I am
> just (for testing purposes) removing the wrong files (modules.builtin
> for example) -- I am still testing.
>
> But, what irritates me with testing is that (also without your
> patches) two consecutive "make" produce different output, one of them
> always shows a warning and this is reproducable. I just want to make
> sure there is no other problem that influences my testing:
>
> $ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> DATAREL arch/x86/boot/compressed/vmlinux
> Kernel: arch/x86/boot/bzImage is ready (#15)
> Building modules, stage 2.
> MODPOST 211 modules
>
> $ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> LD arch/x86/boot/compressed/vmlinux
> ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
> ld: warning: creating a DT_TEXTREL in object.
> ZOFFSET arch/x86/boot/zoffset.h
> AS arch/x86/boot/header.o
> LD arch/x86/boot/setup.elf
> OBJCOPY arch/x86/boot/setup.bin
> OBJCOPY arch/x86/boot/vmlinux.bin
> BUILD arch/x86/boot/bzImage
> Setup is 15580 bytes (padded to 15872 bytes).
> System is 8069 kB
> CRC e01d75ec
> Kernel: arch/x86/boot/bzImage is ready (#15)
> Building modules, stage 2.
> MODPOST 211 modules
I spent some more time with the behaviour described above and bisected
to the commit after that two consecutive invocations of "make" (on an
already compiled tree) seem to do different things. That commit is
98f78525371b55cc (x86/boot: Refuse to build with data relocations), so I
put Kees and Ingo on CC.
I did the bisecting on another system, so I'll provide the output of two
consecutive "make" on an already compiled tree on that machine:
$ make
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
DATAREL arch/x86/boot/compressed/vmlinux
Kernel: arch/x86/boot/bzImage is ready (#48)
Building modules, stage 2.
MODPOST 165 modules
$ make
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
LD arch/x86/boot/compressed/vmlinux
ZOFFSET arch/x86/boot/zoffset.h
AS arch/x86/boot/header.o
LD arch/x86/boot/setup.elf
OBJCOPY arch/x86/boot/setup.bin
OBJCOPY arch/x86/boot/vmlinux.bin
BUILD arch/x86/boot/bzImage
Setup is 15644 bytes (padded to 15872 bytes).
System is 6663 kB
CRC 3eb90f40
Kernel: arch/x86/boot/bzImage is ready (#48)
Building modules, stage 2.
MODPOST 165 modules
If I comment out $(call if_changed,check_data_rel) in
arch/x86/boot/compressed/Makefile, two consecutive "make" produce
identical output i.e. seem to not do different things:
$ make
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready (#49)
Building modules, stage 2.
MODPOST 165 modules
$ make
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready (#49)
Building modules, stage 2.
MODPOST 165 modules
So, I guess this different behaviour of two consecutive "make" is not
intentional but I am failing to understand why it happens.
Dirk
>
> Dirk
>
>> I see some scenarios where this happens.
>>
>> Scenario A:
>>
>> 1. You have a source tree that has already been configured.
>>
>> 2. Linus increments the version number in the top-level Makefile
>> (i.e. he commits a new release)
>>
>> 3. You pull it, and run 'make'
>>
>> 4. syncconfig is invoked because the environment variable,
>> KERNELVERSION is updated, but the .config is not updated since
>> no config symbol is changed.
>>
>> 5. The .config file contains a kernel version in the top line:
>>
>> # Automatically generated file; DO NOT EDIT.
>> # Linux/arm64 4.18.0-rc2 Kernel Configuration
>>
>> ... which points to a previous version.
>>
>> Scenario B:
>>
>> 1. You have a source tree that has already been configured.
>>
>> 2. You upgrade the compiler, but it still has the same version number.
>> This may happen if you regularly build the latest compiler from
>> the source code.
>>
>> 3. You run 'make'
>>
>> 4. syncconfig is invoked because the environment variable,
>> CC_VERSION_TEXT is updated, but the .config is not updated since
>> no config symbol is changed.
>>
>> 5. The .config file contains the version string of the compiler:
>>
>> #
>> # Compiler: aarch64-linux-gcc (GCC) 9.0.0 20180628 (experimental)
>> #
>>
>> ... which carries the information of the old compiler.
>>
>> If KCONFIG_NOSILENTUPDATE is set, syncconfig is not allowed to update
>> the .config file. Otherwise, it is fine to update it regardless of
>> sym_change_count.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>> scripts/kconfig/conf.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 671ff53..5af8991 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -496,6 +496,7 @@ int main(int ac, char **av)
>> int opt;
>> const char *name, *defconfig_file = NULL /* gcc uninit */;
>> struct stat tmpstat;
>> + int no_conf_write = 0;
>>
>> tty_stdio = isatty(0) && isatty(1);
>>
>> @@ -633,13 +634,14 @@ int main(int ac, char **av)
>> }
>>
>> if (sync_kconfig) {
>> - if (conf_get_changed()) {
>> - name = getenv("KCONFIG_NOSILENTUPDATE");
>> - if (name && *name) {
>> + name = getenv("KCONFIG_NOSILENTUPDATE");
>> + if (name && *name) {
>> + if (conf_get_changed()) {
>> fprintf(stderr,
>> "\n*** The configuration requires explicit update.\n\n");
>> return 1;
>> }
>> + no_conf_write = 1;
>> }
>> }
>>
>> @@ -688,7 +690,7 @@ int main(int ac, char **av)
>> /* syncconfig is used during the build so we shall update autoconf.
>> * All other commands are only used to generate a config.
>> */
>> - if (conf_get_changed() && conf_write(NULL)) {
>> + if (!no_conf_write && conf_write(NULL)) {
>> fprintf(stderr, "\n*** Error during writing of the configuration.\n\n");
>> exit(1);
>> }
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-06 23:38 ` Dirk Gouders
@ 2018-07-09 11:39 ` Dirk Gouders
2018-07-10 15:19 ` Kees Cook
2018-07-12 2:12 ` Masahiro Yamada
0 siblings, 2 replies; 25+ messages in thread
From: Dirk Gouders @ 2018-07-09 11:39 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
linux-kernel, Kees Cook, Ingo Molnar
[-- Attachment #1: Type: text/plain, Size: 5309 bytes --]
Dirk Gouders <dirk@gouders.net> writes:
> Dirk Gouders <dirk@gouders.net> writes:
>
>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>
>>> syncconfig updates the .config only when sym_change_count > 0, i.e.
>>> any change in config symbols has been detected.
>>>
>>> Not only symbols but also comments are contained in the .config file.
>>> If only comments are updated, they are not fed back to the .config,
>>> then the stale comments are left-over. Of course, this is just a
>>> matter of comments, but why not fix it.
>>
>> Hello Masahiro,
>>
>> I am currently looking at and testing this series.
>>
>> First: For this patch I would suggest to also edit the syncconfig
>> section of "conf --help".
>>
>> Further, on a slow laptop, I was suspecting, this patch to cause full
>> rebuilds of everything, each time I ran "make syncconfig" followed by
>> "make" but could not verify this on another machine, so perhaps I am
>> just (for testing purposes) removing the wrong files (modules.builtin
>> for example) -- I am still testing.
>>
>> But, what irritates me with testing is that (also without your
>> patches) two consecutive "make" produce different output, one of them
>> always shows a warning and this is reproducable. I just want to make
>> sure there is no other problem that influences my testing:
>>
>> $ make
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> CHK include/generated/compile.h
>> DATAREL arch/x86/boot/compressed/vmlinux
>> Kernel: arch/x86/boot/bzImage is ready (#15)
>> Building modules, stage 2.
>> MODPOST 211 modules
>>
>> $ make
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> CHK include/generated/compile.h
>> LD arch/x86/boot/compressed/vmlinux
>> ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
>> ld: warning: creating a DT_TEXTREL in object.
>> ZOFFSET arch/x86/boot/zoffset.h
>> AS arch/x86/boot/header.o
>> LD arch/x86/boot/setup.elf
>> OBJCOPY arch/x86/boot/setup.bin
>> OBJCOPY arch/x86/boot/vmlinux.bin
>> BUILD arch/x86/boot/bzImage
>> Setup is 15580 bytes (padded to 15872 bytes).
>> System is 8069 kB
>> CRC e01d75ec
>> Kernel: arch/x86/boot/bzImage is ready (#15)
>> Building modules, stage 2.
>> MODPOST 211 modules
>
> I spent some more time with the behaviour described above and bisected
> to the commit after that two consecutive invocations of "make" (on an
> already compiled tree) seem to do different things. That commit is
> 98f78525371b55cc (x86/boot: Refuse to build with data relocations), so I
> put Kees and Ingo on CC.
>
> I did the bisecting on another system, so I'll provide the output of two
> consecutive "make" on an already compiled tree on that machine:
>
> $ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> DATAREL arch/x86/boot/compressed/vmlinux
> Kernel: arch/x86/boot/bzImage is ready (#48)
> Building modules, stage 2.
> MODPOST 165 modules
>
> $ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> LD arch/x86/boot/compressed/vmlinux
> ZOFFSET arch/x86/boot/zoffset.h
> AS arch/x86/boot/header.o
> LD arch/x86/boot/setup.elf
> OBJCOPY arch/x86/boot/setup.bin
> OBJCOPY arch/x86/boot/vmlinux.bin
> BUILD arch/x86/boot/bzImage
> Setup is 15644 bytes (padded to 15872 bytes).
> System is 6663 kB
> CRC 3eb90f40
> Kernel: arch/x86/boot/bzImage is ready (#48)
> Building modules, stage 2.
> MODPOST 165 modules
>
> If I comment out $(call if_changed,check_data_rel) in
> arch/x86/boot/compressed/Makefile, two consecutive "make" produce
> identical output i.e. seem to not do different things:
>
> $ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> Kernel: arch/x86/boot/bzImage is ready (#49)
> Building modules, stage 2.
> MODPOST 165 modules
>
> $ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> Kernel: arch/x86/boot/bzImage is ready (#49)
> Building modules, stage 2.
> MODPOST 165 modules
>
> So, I guess this different behaviour of two consecutive "make" is not
> intentional but I am failing to understand why it happens.
I think, I solved the puzzle and perhaps, that saves others some time:
The problem is that "if_changed" was not designed for multiple use
inside a recipe and in the case of compressed/vmlinux, the 2-fold use
created a kind of flip-flop for situations when nothing has to be done
to build the target.
Because each of the two users of "if_changed" stores it's footprint in
.vmlinux.cmd but that file then isn't re-read, one of the two
"if_changed" calculates that nothing has to be done wheras the other one
recognizes a change in the commandline, because it sees the command-line
for the other part of the reciepe.
In the next make, the roles flip, because the previously satisfied
"if_changed" now sees the command-line of the other one. And so on...
I am not a Kbuild expert but the attached patch fixes that problem by
introducing "if_changed_multi" that accepts two commands -- one whose
commandline should be checked and a second one that should be
executed.
Dirk
[-- Attachment #2: Fix flip-flop --]
[-- Type: text/plain, Size: 1844 bytes --]
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index fa42f895fdde..f39822fca994 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -107,8 +107,8 @@ define cmd_check_data_rel
endef
$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
- $(call if_changed,check_data_rel)
- $(call if_changed,ld)
+ $(call if_changed_multi,ld,check_data_rel)
+ $(call if_changed_multi,ld,ld)
OBJCOPYFLAGS_vmlinux.bin := -R .comment -S
$(obj)/vmlinux.bin: vmlinux FORCE
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index b2d14f1136e8..3bf419319e09 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -265,6 +265,23 @@ if_changed = $(if $(strip $(any-prereq) $(arg-check)), \
$(echo-cmd) $(cmd_$(1)); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+# echo command for command stored in $2
+# Short version is used, if $(quiet) equals `quiet_', otherwise full one.
+echo-cmd2 = $(if $($(quiet)cmd_$(2)),\
+ echo ' $(call escsq,$($(quiet)cmd_$(2)))$(echo-why)';)
+
+# Execute command arg2 if commandline for command arg1 or prerequisite(s) are
+# updated.
+#
+# This is safe for multiple use inside a recipe; arg1 and arg2 may be
+# identical.
+if_changed_multi = $(if $(strip $(any-prereq) $(arg-check)), \
+ @set -e; \
+ $(echo-cmd2) : ; \
+ $(cmd_$(2)); \
+ printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+
+
# Execute the command and also postprocess generated .d dependencies file.
if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ), \
@set -e; \
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-09 11:39 ` Dirk Gouders
@ 2018-07-10 15:19 ` Kees Cook
2018-07-12 2:12 ` Masahiro Yamada
1 sibling, 0 replies; 25+ messages in thread
From: Kees Cook @ 2018-07-10 15:19 UTC (permalink / raw)
To: Dirk Gouders
Cc: Masahiro Yamada, linux-kbuild, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, LKML, Ingo Molnar
On Mon, Jul 9, 2018 at 4:39 AM, Dirk Gouders <dirk@gouders.net> wrote:
> I think, I solved the puzzle and perhaps, that saves others some time:
>
> The problem is that "if_changed" was not designed for multiple use
> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
> created a kind of flip-flop for situations when nothing has to be done
> to build the target.
>
> Because each of the two users of "if_changed" stores it's footprint in
> .vmlinux.cmd but that file then isn't re-read, one of the two
> "if_changed" calculates that nothing has to be done wheras the other one
> recognizes a change in the commandline, because it sees the command-line
> for the other part of the reciepe.
Ooh, nice find. Thanks for debugging this!
> In the next make, the roles flip, because the previously satisfied
> "if_changed" now sees the command-line of the other one. And so on...
>
> I am not a Kbuild expert but the attached patch fixes that problem by
> introducing "if_changed_multi" that accepts two commands -- one whose
> commandline should be checked and a second one that should be
> executed.
>
> Dirk
>
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index fa42f895fdde..f39822fca994 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -107,8 +107,8 @@ define cmd_check_data_rel
> endef
>
> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> - $(call if_changed,check_data_rel)
> - $(call if_changed,ld)
> + $(call if_changed_multi,ld,check_data_rel)
> + $(call if_changed_multi,ld,ld)
This does seem to do the right thing, but mainly because arg1 is what
actually writes vmlinux, which seems a bit fragile here.
However, the combination is "if_changed" with the "FORCE". This is to
make sure the command line is evaluated in addition to the build deps.
The check_data_rel isn't as sensitive, so maybe the right solution is
just:
-$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
- $(call if_changed,check_data_rel)
+$(obj)/vmlinux:: $(vmlinux-objs-y)
+ $(call cmd,check_data_rel)
+$(obj)/vmlinux:: $(vmlinux-objs-y) FORCE
$(call if_changed,ld)
Then check_data_rel is only evaluated with when the deps change (no
FORCE nor if_changed needed), and doesn't interfere with the "ld"
call?
Regardless, the docs for if_changed should be updated to explicitly
mention it should only be called once per target.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-09 11:39 ` Dirk Gouders
2018-07-10 15:19 ` Kees Cook
@ 2018-07-12 2:12 ` Masahiro Yamada
2018-07-12 11:32 ` Dirk Gouders
1 sibling, 1 reply; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-12 2:12 UTC (permalink / raw)
To: Dirk Gouders
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Linux Kernel Mailing List, Kees Cook, Ingo Molnar
2018-07-09 20:39 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
> Dirk Gouders <dirk@gouders.net> writes:
>
>> Dirk Gouders <dirk@gouders.net> writes:
>>
>>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>>
>>>> syncconfig updates the .config only when sym_change_count > 0, i.e.
>>>> any change in config symbols has been detected.
>>>>
>>>> Not only symbols but also comments are contained in the .config file.
>>>> If only comments are updated, they are not fed back to the .config,
>>>> then the stale comments are left-over. Of course, this is just a
>>>> matter of comments, but why not fix it.
>>>
>>> Hello Masahiro,
>>>
>>> I am currently looking at and testing this series.
>>>
>>> First: For this patch I would suggest to also edit the syncconfig
>>> section of "conf --help".
>>>
>>> Further, on a slow laptop, I was suspecting, this patch to cause full
>>> rebuilds of everything, each time I ran "make syncconfig" followed by
>>> "make" but could not verify this on another machine, so perhaps I am
>>> just (for testing purposes) removing the wrong files (modules.builtin
>>> for example) -- I am still testing.
>>>
>>> But, what irritates me with testing is that (also without your
>>> patches) two consecutive "make" produce different output, one of them
>>> always shows a warning and this is reproducable. I just want to make
>>> sure there is no other problem that influences my testing:
>>>
>>> $ make
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> CHK include/generated/compile.h
>>> DATAREL arch/x86/boot/compressed/vmlinux
>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>> Building modules, stage 2.
>>> MODPOST 211 modules
>>>
>>> $ make
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> CHK include/generated/compile.h
>>> LD arch/x86/boot/compressed/vmlinux
>>> ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
>>> ld: warning: creating a DT_TEXTREL in object.
>>> ZOFFSET arch/x86/boot/zoffset.h
>>> AS arch/x86/boot/header.o
>>> LD arch/x86/boot/setup.elf
>>> OBJCOPY arch/x86/boot/setup.bin
>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>> BUILD arch/x86/boot/bzImage
>>> Setup is 15580 bytes (padded to 15872 bytes).
>>> System is 8069 kB
>>> CRC e01d75ec
>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>> Building modules, stage 2.
>>> MODPOST 211 modules
>>
>> I spent some more time with the behaviour described above and bisected
>> to the commit after that two consecutive invocations of "make" (on an
>> already compiled tree) seem to do different things. That commit is
>> 98f78525371b55cc (x86/boot: Refuse to build with data relocations), so I
>> put Kees and Ingo on CC.
>>
>> I did the bisecting on another system, so I'll provide the output of two
>> consecutive "make" on an already compiled tree on that machine:
>>
>> $ make
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> CHK include/generated/compile.h
>> DATAREL arch/x86/boot/compressed/vmlinux
>> Kernel: arch/x86/boot/bzImage is ready (#48)
>> Building modules, stage 2.
>> MODPOST 165 modules
>>
>> $ make
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> CHK include/generated/compile.h
>> LD arch/x86/boot/compressed/vmlinux
>> ZOFFSET arch/x86/boot/zoffset.h
>> AS arch/x86/boot/header.o
>> LD arch/x86/boot/setup.elf
>> OBJCOPY arch/x86/boot/setup.bin
>> OBJCOPY arch/x86/boot/vmlinux.bin
>> BUILD arch/x86/boot/bzImage
>> Setup is 15644 bytes (padded to 15872 bytes).
>> System is 6663 kB
>> CRC 3eb90f40
>> Kernel: arch/x86/boot/bzImage is ready (#48)
>> Building modules, stage 2.
>> MODPOST 165 modules
>>
>> If I comment out $(call if_changed,check_data_rel) in
>> arch/x86/boot/compressed/Makefile, two consecutive "make" produce
>> identical output i.e. seem to not do different things:
>>
>> $ make
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> CHK include/generated/compile.h
>> Kernel: arch/x86/boot/bzImage is ready (#49)
>> Building modules, stage 2.
>> MODPOST 165 modules
>>
>> $ make
>> CALL scripts/checksyscalls.sh
>> DESCEND objtool
>> CHK include/generated/compile.h
>> Kernel: arch/x86/boot/bzImage is ready (#49)
>> Building modules, stage 2.
>> MODPOST 165 modules
>>
>> So, I guess this different behaviour of two consecutive "make" is not
>> intentional but I am failing to understand why it happens.
>
> I think, I solved the puzzle and perhaps, that saves others some time:
>
> The problem is that "if_changed" was not designed for multiple use
> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
> created a kind of flip-flop for situations when nothing has to be done
> to build the target.
>
> Because each of the two users of "if_changed" stores it's footprint in
> .vmlinux.cmd but that file then isn't re-read, one of the two
> "if_changed" calculates that nothing has to be done wheras the other one
> recognizes a change in the commandline, because it sees the command-line
> for the other part of the reciepe.
>
> In the next make, the roles flip, because the previously satisfied
> "if_changed" now sees the command-line of the other one. And so on...
>
> I am not a Kbuild expert but the attached patch fixes that problem by
> introducing "if_changed_multi" that accepts two commands -- one whose
> commandline should be checked and a second one that should be
> executed.
if_changed should not appear multiple times in one target.
I think the simplest fix-up is to
create a new command that combines
'cmd_check_data_rel' and 'cmd_ld'.
quiet_cmd_link-vmlinux = LD $@
cmd_link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
$(call if_changed,link-vmlinux)
Kbuild also supports if_changed_rule,
but the usage is more complex.
There are only a few usages:
https://github.com/torvalds/linux/blob/v4.17/scripts/Makefile.build#L288
> Dirk
>
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index fa42f895fdde..f39822fca994 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -107,8 +107,8 @@ define cmd_check_data_rel
> endef
>
> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> - $(call if_changed,check_data_rel)
> - $(call if_changed,ld)
> + $(call if_changed_multi,ld,check_data_rel)
> + $(call if_changed_multi,ld,ld)
>
> OBJCOPYFLAGS_vmlinux.bin := -R .comment -S
> $(obj)/vmlinux.bin: vmlinux FORCE
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index b2d14f1136e8..3bf419319e09 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -265,6 +265,23 @@ if_changed = $(if $(strip $(any-prereq) $(arg-check)), \
> $(echo-cmd) $(cmd_$(1)); \
> printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
>
> +# echo command for command stored in $2
> +# Short version is used, if $(quiet) equals `quiet_', otherwise full one.
> +echo-cmd2 = $(if $($(quiet)cmd_$(2)),\
> + echo ' $(call escsq,$($(quiet)cmd_$(2)))$(echo-why)';)
> +
> +# Execute command arg2 if commandline for command arg1 or prerequisite(s) are
> +# updated.
> +#
> +# This is safe for multiple use inside a recipe; arg1 and arg2 may be
> +# identical.
> +if_changed_multi = $(if $(strip $(any-prereq) $(arg-check)), \
> + @set -e; \
> + $(echo-cmd2) : ; \
> + $(cmd_$(2)); \
> + printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> +
> +
> # Execute the command and also postprocess generated .d dependencies file.
> if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ), \
> @set -e; \
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-12 2:12 ` Masahiro Yamada
@ 2018-07-12 11:32 ` Dirk Gouders
2018-07-12 21:06 ` Dirk Gouders
2018-07-13 14:57 ` Masahiro Yamada
0 siblings, 2 replies; 25+ messages in thread
From: Dirk Gouders @ 2018-07-12 11:32 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Linux Kernel Mailing List, Kees Cook, Ingo Molnar,
Michal Simek, David S. Miller
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> 2018-07-09 20:39 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
>> Dirk Gouders <dirk@gouders.net> writes:
>>
>>> Dirk Gouders <dirk@gouders.net> writes:
>>>
>>>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>>>
>>>>> syncconfig updates the .config only when sym_change_count > 0, i.e.
>>>>> any change in config symbols has been detected.
>>>>>
>>>>> Not only symbols but also comments are contained in the .config file.
>>>>> If only comments are updated, they are not fed back to the .config,
>>>>> then the stale comments are left-over. Of course, this is just a
>>>>> matter of comments, but why not fix it.
>>>>
>>>> Hello Masahiro,
>>>>
>>>> I am currently looking at and testing this series.
>>>>
>>>> First: For this patch I would suggest to also edit the syncconfig
>>>> section of "conf --help".
>>>>
>>>> Further, on a slow laptop, I was suspecting, this patch to cause full
>>>> rebuilds of everything, each time I ran "make syncconfig" followed by
>>>> "make" but could not verify this on another machine, so perhaps I am
>>>> just (for testing purposes) removing the wrong files (modules.builtin
>>>> for example) -- I am still testing.
>>>>
>>>> But, what irritates me with testing is that (also without your
>>>> patches) two consecutive "make" produce different output, one of them
>>>> always shows a warning and this is reproducable. I just want to make
>>>> sure there is no other problem that influences my testing:
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> DATAREL arch/x86/boot/compressed/vmlinux
>>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>>> Building modules, stage 2.
>>>> MODPOST 211 modules
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> LD arch/x86/boot/compressed/vmlinux
>>>> ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
>>>> ld: warning: creating a DT_TEXTREL in object.
>>>> ZOFFSET arch/x86/boot/zoffset.h
>>>> AS arch/x86/boot/header.o
>>>> LD arch/x86/boot/setup.elf
>>>> OBJCOPY arch/x86/boot/setup.bin
>>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>>> BUILD arch/x86/boot/bzImage
>>>> Setup is 15580 bytes (padded to 15872 bytes).
>>>> System is 8069 kB
>>>> CRC e01d75ec
>>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>>> Building modules, stage 2.
>>>> MODPOST 211 modules
>>>
>>> I spent some more time with the behaviour described above and bisected
>>> to the commit after that two consecutive invocations of "make" (on an
>>> already compiled tree) seem to do different things. That commit is
>>> 98f78525371b55cc (x86/boot: Refuse to build with data relocations), so I
>>> put Kees and Ingo on CC.
>>>
>>> I did the bisecting on another system, so I'll provide the output of two
>>> consecutive "make" on an already compiled tree on that machine:
>>>
>>> $ make
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> CHK include/generated/compile.h
>>> DATAREL arch/x86/boot/compressed/vmlinux
>>> Kernel: arch/x86/boot/bzImage is ready (#48)
>>> Building modules, stage 2.
>>> MODPOST 165 modules
>>>
>>> $ make
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> CHK include/generated/compile.h
>>> LD arch/x86/boot/compressed/vmlinux
>>> ZOFFSET arch/x86/boot/zoffset.h
>>> AS arch/x86/boot/header.o
>>> LD arch/x86/boot/setup.elf
>>> OBJCOPY arch/x86/boot/setup.bin
>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>> BUILD arch/x86/boot/bzImage
>>> Setup is 15644 bytes (padded to 15872 bytes).
>>> System is 6663 kB
>>> CRC 3eb90f40
>>> Kernel: arch/x86/boot/bzImage is ready (#48)
>>> Building modules, stage 2.
>>> MODPOST 165 modules
>>>
>>> If I comment out $(call if_changed,check_data_rel) in
>>> arch/x86/boot/compressed/Makefile, two consecutive "make" produce
>>> identical output i.e. seem to not do different things:
>>>
>>> $ make
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> CHK include/generated/compile.h
>>> Kernel: arch/x86/boot/bzImage is ready (#49)
>>> Building modules, stage 2.
>>> MODPOST 165 modules
>>>
>>> $ make
>>> CALL scripts/checksyscalls.sh
>>> DESCEND objtool
>>> CHK include/generated/compile.h
>>> Kernel: arch/x86/boot/bzImage is ready (#49)
>>> Building modules, stage 2.
>>> MODPOST 165 modules
>>>
>>> So, I guess this different behaviour of two consecutive "make" is not
>>> intentional but I am failing to understand why it happens.
>>
>> I think, I solved the puzzle and perhaps, that saves others some time:
>>
>> The problem is that "if_changed" was not designed for multiple use
>> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
>> created a kind of flip-flop for situations when nothing has to be done
>> to build the target.
>>
>> Because each of the two users of "if_changed" stores it's footprint in
>> .vmlinux.cmd but that file then isn't re-read, one of the two
>> "if_changed" calculates that nothing has to be done wheras the other one
>> recognizes a change in the commandline, because it sees the command-line
>> for the other part of the reciepe.
>>
>> In the next make, the roles flip, because the previously satisfied
>> "if_changed" now sees the command-line of the other one. And so on...
>>
>> I am not a Kbuild expert but the attached patch fixes that problem by
>> introducing "if_changed_multi" that accepts two commands -- one whose
>> commandline should be checked and a second one that should be
>> executed.
>
>
> if_changed should not appear multiple times in one target.
>
> I think the simplest fix-up is to
> create a new command that combines
> 'cmd_check_data_rel' and 'cmd_ld'.
>
>
> quiet_cmd_link-vmlinux = LD $@
> cmd_link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
>
> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> $(call if_changed,link-vmlinux)
>
> Kbuild also supports if_changed_rule,
> but the usage is more complex.
>
> There are only a few usages:
> https://github.com/torvalds/linux/blob/v4.17/scripts/Makefile.build#L288
Just for completeness I will copy in part of a reply from Kees that
shows how double-colon rules can also avoid multiple use of if_changed
for one target:
-$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
- $(call if_changed,check_data_rel)
+$(obj)/vmlinux:: $(vmlinux-objs-y)
+ $(call cmd,check_data_rel)
+$(obj)/vmlinux:: $(vmlinux-objs-y) FORCE
$(call if_changed,ld)
The combined command seems to have the advantage that every command to
build the target gets recorded in the .cmd file
A search showed me that we have two more users that use if_changed more
than once for a single target:
arch/microblaze/boot/Makefile (fourfold)
arch/sparc/boot/Makefile (2 times twofold)
The sparc case seems to apply to any of the two suggested fixes, but
microblaze uses if_changed in a pattern rule and also makes use of
parameter arguments in the sub-commands:
$(obj)/simpleImage.%: vmlinux FORCE
$(call if_changed,cp,.unstrip)
$(call if_changed,objcopy)
$(call if_changed,uimage)
$(call if_changed,strip,.strip)
@echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
In this case, double colons would have a different meaning and the
combined command solution would result in a change of the sub-commands,
as well. I note this in case Michal perhaps has other preferences.
In addition to extend the documentation, we could modify if_changed to
warn about it is being used more than once for a target:
# Execute command if command has changed or prerequisite(s) are updated.
if_changed = $(if $(filter-out undefined,$(origin if_changed_$@)), \
@set -e; \
echo "Warning: $@: multiple use of if_changed!" >&2; , \
@set -e $(eval if_changed_$@ := 1) ; ) \
$(if $(strip $(any-prereq) $(arg-check)), \
$(echo-cmd) $(cmd_$(1)); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
But this fires only if if_changed is actually called and it defines many
variables for just that purpose, so this is perhaps not what we want...
Dirk
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-12 11:32 ` Dirk Gouders
@ 2018-07-12 21:06 ` Dirk Gouders
2018-07-13 14:57 ` Masahiro Yamada
1 sibling, 0 replies; 25+ messages in thread
From: Dirk Gouders @ 2018-07-12 21:06 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Linux Kernel Mailing List, Kees Cook, Ingo Molnar,
Michal Simek, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 10146 bytes --]
Dirk Gouders <dirk@gouders.net> writes:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
>> 2018-07-09 20:39 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
>>> Dirk Gouders <dirk@gouders.net> writes:
>>>
>>>> Dirk Gouders <dirk@gouders.net> writes:
>>>>
>>>>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>>>>
>>>>>> syncconfig updates the .config only when sym_change_count > 0, i.e.
>>>>>> any change in config symbols has been detected.
>>>>>>
>>>>>> Not only symbols but also comments are contained in the .config file.
>>>>>> If only comments are updated, they are not fed back to the .config,
>>>>>> then the stale comments are left-over. Of course, this is just a
>>>>>> matter of comments, but why not fix it.
>>>>>
>>>>> Hello Masahiro,
>>>>>
>>>>> I am currently looking at and testing this series.
>>>>>
>>>>> First: For this patch I would suggest to also edit the syncconfig
>>>>> section of "conf --help".
>>>>>
>>>>> Further, on a slow laptop, I was suspecting, this patch to cause full
>>>>> rebuilds of everything, each time I ran "make syncconfig" followed by
>>>>> "make" but could not verify this on another machine, so perhaps I am
>>>>> just (for testing purposes) removing the wrong files (modules.builtin
>>>>> for example) -- I am still testing.
>>>>>
>>>>> But, what irritates me with testing is that (also without your
>>>>> patches) two consecutive "make" produce different output, one of them
>>>>> always shows a warning and this is reproducable. I just want to make
>>>>> sure there is no other problem that influences my testing:
>>>>>
>>>>> $ make
>>>>> CALL scripts/checksyscalls.sh
>>>>> DESCEND objtool
>>>>> CHK include/generated/compile.h
>>>>> DATAREL arch/x86/boot/compressed/vmlinux
>>>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>>>> Building modules, stage 2.
>>>>> MODPOST 211 modules
>>>>>
>>>>> $ make
>>>>> CALL scripts/checksyscalls.sh
>>>>> DESCEND objtool
>>>>> CHK include/generated/compile.h
>>>>> LD arch/x86/boot/compressed/vmlinux
>>>>> ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
>>>>> ld: warning: creating a DT_TEXTREL in object.
>>>>> ZOFFSET arch/x86/boot/zoffset.h
>>>>> AS arch/x86/boot/header.o
>>>>> LD arch/x86/boot/setup.elf
>>>>> OBJCOPY arch/x86/boot/setup.bin
>>>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>>>> BUILD arch/x86/boot/bzImage
>>>>> Setup is 15580 bytes (padded to 15872 bytes).
>>>>> System is 8069 kB
>>>>> CRC e01d75ec
>>>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>>>> Building modules, stage 2.
>>>>> MODPOST 211 modules
>>>>
>>>> I spent some more time with the behaviour described above and bisected
>>>> to the commit after that two consecutive invocations of "make" (on an
>>>> already compiled tree) seem to do different things. That commit is
>>>> 98f78525371b55cc (x86/boot: Refuse to build with data relocations), so I
>>>> put Kees and Ingo on CC.
>>>>
>>>> I did the bisecting on another system, so I'll provide the output of two
>>>> consecutive "make" on an already compiled tree on that machine:
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> DATAREL arch/x86/boot/compressed/vmlinux
>>>> Kernel: arch/x86/boot/bzImage is ready (#48)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> LD arch/x86/boot/compressed/vmlinux
>>>> ZOFFSET arch/x86/boot/zoffset.h
>>>> AS arch/x86/boot/header.o
>>>> LD arch/x86/boot/setup.elf
>>>> OBJCOPY arch/x86/boot/setup.bin
>>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>>> BUILD arch/x86/boot/bzImage
>>>> Setup is 15644 bytes (padded to 15872 bytes).
>>>> System is 6663 kB
>>>> CRC 3eb90f40
>>>> Kernel: arch/x86/boot/bzImage is ready (#48)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> If I comment out $(call if_changed,check_data_rel) in
>>>> arch/x86/boot/compressed/Makefile, two consecutive "make" produce
>>>> identical output i.e. seem to not do different things:
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> Kernel: arch/x86/boot/bzImage is ready (#49)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> Kernel: arch/x86/boot/bzImage is ready (#49)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> So, I guess this different behaviour of two consecutive "make" is not
>>>> intentional but I am failing to understand why it happens.
>>>
>>> I think, I solved the puzzle and perhaps, that saves others some time:
>>>
>>> The problem is that "if_changed" was not designed for multiple use
>>> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
>>> created a kind of flip-flop for situations when nothing has to be done
>>> to build the target.
>>>
>>> Because each of the two users of "if_changed" stores it's footprint in
>>> .vmlinux.cmd but that file then isn't re-read, one of the two
>>> "if_changed" calculates that nothing has to be done wheras the other one
>>> recognizes a change in the commandline, because it sees the command-line
>>> for the other part of the reciepe.
>>>
>>> In the next make, the roles flip, because the previously satisfied
>>> "if_changed" now sees the command-line of the other one. And so on...
>>>
>>> I am not a Kbuild expert but the attached patch fixes that problem by
>>> introducing "if_changed_multi" that accepts two commands -- one whose
>>> commandline should be checked and a second one that should be
>>> executed.
>>
>>
>> if_changed should not appear multiple times in one target.
>>
>> I think the simplest fix-up is to
>> create a new command that combines
>> 'cmd_check_data_rel' and 'cmd_ld'.
>>
>>
>> quiet_cmd_link-vmlinux = LD $@
>> cmd_link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
>>
>> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>> $(call if_changed,link-vmlinux)
>>
>> Kbuild also supports if_changed_rule,
>> but the usage is more complex.
>>
>> There are only a few usages:
>> https://github.com/torvalds/linux/blob/v4.17/scripts/Makefile.build#L288
>
> Just for completeness I will copy in part of a reply from Kees that
> shows how double-colon rules can also avoid multiple use of if_changed
> for one target:
>
> -$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> - $(call if_changed,check_data_rel)
> +$(obj)/vmlinux:: $(vmlinux-objs-y)
> + $(call cmd,check_data_rel)
> +$(obj)/vmlinux:: $(vmlinux-objs-y) FORCE
> $(call if_changed,ld)
>
> The combined command seems to have the advantage that every command to
> build the target gets recorded in the .cmd file
>
> A search showed me that we have two more users that use if_changed more
> than once for a single target:
>
> arch/microblaze/boot/Makefile (fourfold)
> arch/sparc/boot/Makefile (2 times twofold)
>
> The sparc case seems to apply to any of the two suggested fixes, but
> microblaze uses if_changed in a pattern rule and also makes use of
> parameter arguments in the sub-commands:
>
> $(obj)/simpleImage.%: vmlinux FORCE
> $(call if_changed,cp,.unstrip)
> $(call if_changed,objcopy)
> $(call if_changed,uimage)
> $(call if_changed,strip,.strip)
> @echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
>
> In this case, double colons would have a different meaning and the
> combined command solution would result in a change of the sub-commands,
> as well. I note this in case Michal perhaps has other preferences.
>
>
> In addition to extend the documentation, we could modify if_changed to
> warn about it is being used more than once for a target:
>
> # Execute command if command has changed or prerequisite(s) are updated.
> if_changed = $(if $(filter-out undefined,$(origin if_changed_$@)), \
> @set -e; \
> echo "Warning: $@: multiple use of if_changed!" >&2; , \
> @set -e $(eval if_changed_$@ := 1) ; ) \
> $(if $(strip $(any-prereq) $(arg-check)), \
> $(echo-cmd) $(cmd_$(1)); \
> printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
>
> But this fires only if if_changed is actually called and it defines many
> variables for just that purpose, so this is perhaps not what we want...
Short addition: it seems, we can use target-specific variables,
reducing the mass of additional valiables to just one
(more correctly: the number of targets created in parallel):
# Execute command if command has changed or prerequisite(s) are updated.
if_changed = $(if $(filter-out undefined,$(origin if_changed_cnt)), \
@set -e; \
echo "Warning: $@: multiple use of if_changed!" >&2; , \
@set -e $(eval $@: if_changed_cnt := 1) ; ) \
$(if $(strip $(any-prereq) $(arg-check)), \
$(echo-cmd) $(cmd_$(1)); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
The documentation for make says that target-specific variables are also
in effect for all prerequisites of this target but as far as I examined
(see attached Makefile and target "check") not for the example above.
In my understanding this is because the variable is created when the
recipe for the target is executed, i.e. all prerequisites are already
done.
I did a "quick" measurement for an allnoconfig configuration on my slow
laptop:
Without additional check:
real 14m45.477s
user 14m10.775s
sys 0m42.997s
With additional check:
real 14m45.562s
user 14m12.045s
sys 0m41.844s
Dirk
[-- Attachment #2: Test target-specific variable --]
[-- Type: text/plain, Size: 2385 bytes --]
-include .check.cmd
comma := ,
quote := "
squote := '
empty :=
space := $(empty) $(empty)
space_escape := _-_SPACE_-_
pound := \#
quiet_cmd_a = A
cmd_a = echo "doing a"
quiet_cmd_b = B
cmd_b = echo "doing b"
###
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
dot-target = $(dir $@).$(notdir $@)
###
# Escape single quote for use in echo statements
escsq = $(subst $(squote),'\$(squote)',$1)
ifneq ($(KBUILD_NOCMDDEP),1)
# Check if both arguments are the same including their order. Result is empty
# string if equal. User may override this check using make KBUILD_NOCMDDEP=1
arg-check = $(filter-out $(subst $(space),$(space_escape),$(strip $(cmd_$@))), \
$(subst $(space),$(space_escape),$(strip $(cmd_$1))))
else
arg-check = $(if $(strip $(cmd_$@)),,1)
endif
# Replace >$< with >$$< to preserve $ when reloading the .cmd file
# (needed for make)
# Replace >#< with >$(pound)< to avoid starting a comment in the .cmd file
# (needed for make)
# Replace >'< with >'\''< to be able to enclose the whole string in '...'
# (needed for the shell)
make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
# Find any prerequisites that is newer than target or that does not exist.
# PHONY targets skipped in both cases.
any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
quiet = quiet_
# echo command.
# Short version is used, if $(quiet) equals `quiet_', otherwise full one.
echo-cmd = $(if $($(quiet)cmd_$(1)),\
echo ' $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)
cmd = @$(echo-cmd) $(cmd_$(1))
# Execute command if command has changed or prerequisite(s) are updated.
if_changed = $(if $(filter-out undefined,$(origin if_changed_cnt)), \
@set -e; \
echo "Warning: $@: multiple use of if_changed!" >&2; , \
@set -e $(eval $@: if_changed_cnt := 1) ; ) \
$(if $(strip $(any-prereq) $(arg-check)), \
$(echo-cmd) $(cmd_$(1)); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
PHONY += FORCE
FORCE:
a.o: Makefile
@printf 'if_change_cnt: -%s-\n' '$(origin if_change_cnt)'
@touch a.o
check: a.o FORCE
$(call if_changed,a)
$(call if_changed,b)
@touch check
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-12 11:32 ` Dirk Gouders
2018-07-12 21:06 ` Dirk Gouders
@ 2018-07-13 14:57 ` Masahiro Yamada
2018-07-14 7:12 ` Dirk Gouders
1 sibling, 1 reply; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-13 14:57 UTC (permalink / raw)
To: Dirk Gouders
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Linux Kernel Mailing List, Kees Cook, Ingo Molnar,
Michal Simek, David S. Miller
2018-07-12 20:32 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
>> 2018-07-09 20:39 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
>>> Dirk Gouders <dirk@gouders.net> writes:
>>>
>>>> Dirk Gouders <dirk@gouders.net> writes:
>>>>
>>>>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>>>>
>>>>>> syncconfig updates the .config only when sym_change_count > 0, i.e.
>>>>>> any change in config symbols has been detected.
>>>>>>
>>>>>> Not only symbols but also comments are contained in the .config file.
>>>>>> If only comments are updated, they are not fed back to the .config,
>>>>>> then the stale comments are left-over. Of course, this is just a
>>>>>> matter of comments, but why not fix it.
>>>>>
>>>>> Hello Masahiro,
>>>>>
>>>>> I am currently looking at and testing this series.
>>>>>
>>>>> First: For this patch I would suggest to also edit the syncconfig
>>>>> section of "conf --help".
>>>>>
>>>>> Further, on a slow laptop, I was suspecting, this patch to cause full
>>>>> rebuilds of everything, each time I ran "make syncconfig" followed by
>>>>> "make" but could not verify this on another machine, so perhaps I am
>>>>> just (for testing purposes) removing the wrong files (modules.builtin
>>>>> for example) -- I am still testing.
>>>>>
>>>>> But, what irritates me with testing is that (also without your
>>>>> patches) two consecutive "make" produce different output, one of them
>>>>> always shows a warning and this is reproducable. I just want to make
>>>>> sure there is no other problem that influences my testing:
>>>>>
>>>>> $ make
>>>>> CALL scripts/checksyscalls.sh
>>>>> DESCEND objtool
>>>>> CHK include/generated/compile.h
>>>>> DATAREL arch/x86/boot/compressed/vmlinux
>>>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>>>> Building modules, stage 2.
>>>>> MODPOST 211 modules
>>>>>
>>>>> $ make
>>>>> CALL scripts/checksyscalls.sh
>>>>> DESCEND objtool
>>>>> CHK include/generated/compile.h
>>>>> LD arch/x86/boot/compressed/vmlinux
>>>>> ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
>>>>> ld: warning: creating a DT_TEXTREL in object.
>>>>> ZOFFSET arch/x86/boot/zoffset.h
>>>>> AS arch/x86/boot/header.o
>>>>> LD arch/x86/boot/setup.elf
>>>>> OBJCOPY arch/x86/boot/setup.bin
>>>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>>>> BUILD arch/x86/boot/bzImage
>>>>> Setup is 15580 bytes (padded to 15872 bytes).
>>>>> System is 8069 kB
>>>>> CRC e01d75ec
>>>>> Kernel: arch/x86/boot/bzImage is ready (#15)
>>>>> Building modules, stage 2.
>>>>> MODPOST 211 modules
>>>>
>>>> I spent some more time with the behaviour described above and bisected
>>>> to the commit after that two consecutive invocations of "make" (on an
>>>> already compiled tree) seem to do different things. That commit is
>>>> 98f78525371b55cc (x86/boot: Refuse to build with data relocations), so I
>>>> put Kees and Ingo on CC.
>>>>
>>>> I did the bisecting on another system, so I'll provide the output of two
>>>> consecutive "make" on an already compiled tree on that machine:
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> DATAREL arch/x86/boot/compressed/vmlinux
>>>> Kernel: arch/x86/boot/bzImage is ready (#48)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> LD arch/x86/boot/compressed/vmlinux
>>>> ZOFFSET arch/x86/boot/zoffset.h
>>>> AS arch/x86/boot/header.o
>>>> LD arch/x86/boot/setup.elf
>>>> OBJCOPY arch/x86/boot/setup.bin
>>>> OBJCOPY arch/x86/boot/vmlinux.bin
>>>> BUILD arch/x86/boot/bzImage
>>>> Setup is 15644 bytes (padded to 15872 bytes).
>>>> System is 6663 kB
>>>> CRC 3eb90f40
>>>> Kernel: arch/x86/boot/bzImage is ready (#48)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> If I comment out $(call if_changed,check_data_rel) in
>>>> arch/x86/boot/compressed/Makefile, two consecutive "make" produce
>>>> identical output i.e. seem to not do different things:
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> Kernel: arch/x86/boot/bzImage is ready (#49)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> $ make
>>>> CALL scripts/checksyscalls.sh
>>>> DESCEND objtool
>>>> CHK include/generated/compile.h
>>>> Kernel: arch/x86/boot/bzImage is ready (#49)
>>>> Building modules, stage 2.
>>>> MODPOST 165 modules
>>>>
>>>> So, I guess this different behaviour of two consecutive "make" is not
>>>> intentional but I am failing to understand why it happens.
>>>
>>> I think, I solved the puzzle and perhaps, that saves others some time:
>>>
>>> The problem is that "if_changed" was not designed for multiple use
>>> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
>>> created a kind of flip-flop for situations when nothing has to be done
>>> to build the target.
>>>
>>> Because each of the two users of "if_changed" stores it's footprint in
>>> .vmlinux.cmd but that file then isn't re-read, one of the two
>>> "if_changed" calculates that nothing has to be done wheras the other one
>>> recognizes a change in the commandline, because it sees the command-line
>>> for the other part of the reciepe.
>>>
>>> In the next make, the roles flip, because the previously satisfied
>>> "if_changed" now sees the command-line of the other one. And so on...
>>>
>>> I am not a Kbuild expert but the attached patch fixes that problem by
>>> introducing "if_changed_multi" that accepts two commands -- one whose
>>> commandline should be checked and a second one that should be
>>> executed.
>>
>>
>> if_changed should not appear multiple times in one target.
>>
>> I think the simplest fix-up is to
>> create a new command that combines
>> 'cmd_check_data_rel' and 'cmd_ld'.
>>
>>
>> quiet_cmd_link-vmlinux = LD $@
>> cmd_link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
>>
>> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>> $(call if_changed,link-vmlinux)
>>
>> Kbuild also supports if_changed_rule,
>> but the usage is more complex.
>>
>> There are only a few usages:
>> https://github.com/torvalds/linux/blob/v4.17/scripts/Makefile.build#L288
>
> Just for completeness I will copy in part of a reply from Kees that
> shows how double-colon rules can also avoid multiple use of if_changed
> for one target:
>
> -$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> - $(call if_changed,check_data_rel)
> +$(obj)/vmlinux:: $(vmlinux-objs-y)
> + $(call cmd,check_data_rel)
> +$(obj)/vmlinux:: $(vmlinux-objs-y) FORCE
> $(call if_changed,ld)
It is difficult to use double-colon rules in a _sane_ way.
The first one just checks data_rel,
but does not actually generate anything.
Such targets should be marked as .PHONY,
but $(obj)/vmlinux is not a phony target.
This is strange.
> The combined command seems to have the advantage that every command to
> build the target gets recorded in the .cmd file
>
> A search showed me that we have two more users that use if_changed more
> than once for a single target:
>
> arch/microblaze/boot/Makefile (fourfold)
> arch/sparc/boot/Makefile (2 times twofold)
>
> The sparc case seems to apply to any of the two suggested fixes,
Neither is correct.
$(obj)/uImage: $(obj)/image.gz
$(call if_changed,uimage)
$(call if_changed,uimage.o)
should be split into two targets.
$(obj)/uImage: $(obj)/image.gz FORCE
$(call if_changed,uimage)
$(obj)/uImage.o: $(obj)/uImage FORCE
$(call if_changed,uimage.o)
It is wrong in multiple ways. FORCE is missing too.
but
> microblaze uses if_changed in a pattern rule and also makes use of
> parameter arguments in the sub-commands:
>
> $(obj)/simpleImage.%: vmlinux FORCE
> $(call if_changed,cp,.unstrip)
> $(call if_changed,objcopy)
> $(call if_changed,uimage)
> $(call if_changed,strip,.strip)
> @echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
Probably, this is the same.
Create a target for each step.
> In this case, double colons would have a different meaning and the
> combined command solution would result in a change of the sub-commands,
> as well. I note this in case Michal perhaps has other preferences.
>
>
> In addition to extend the documentation, we could modify if_changed to
> warn about it is being used more than once for a target:
>
> # Execute command if command has changed or prerequisite(s) are updated.
> if_changed = $(if $(filter-out undefined,$(origin if_changed_$@)), \
> @set -e; \
> echo "Warning: $@: multiple use of if_changed!" >&2; , \
> @set -e $(eval if_changed_$@ := 1) ; ) \
> $(if $(strip $(any-prereq) $(arg-check)), \
> $(echo-cmd) $(cmd_$(1)); \
> printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
>
> But this fires only if if_changed is actually called and it defines many
> variables for just that purpose, so this is perhaps not what we want...
>
I do not want to mess up Makefile.
Please do this check in scripts/checkpatch.pl
if you want.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
2018-07-13 14:57 ` Masahiro Yamada
@ 2018-07-14 7:12 ` Dirk Gouders
0 siblings, 0 replies; 25+ messages in thread
From: Dirk Gouders @ 2018-07-14 7:12 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Linux Kernel Mailing List, Kees Cook, Ingo Molnar,
Michal Simek, David S. Miller
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> 2018-07-12 20:32 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>
>>> 2018-07-09 20:39 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
>>>> Dirk Gouders <dirk@gouders.net> writes:
>>>>
>>>> I think, I solved the puzzle and perhaps, that saves others some time:
>>>>
>>>> The problem is that "if_changed" was not designed for multiple use
>>>> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
>>>> created a kind of flip-flop for situations when nothing has to be done
>>>> to build the target.
>>>>
>>>> Because each of the two users of "if_changed" stores it's footprint in
>>>> .vmlinux.cmd but that file then isn't re-read, one of the two
>>>> "if_changed" calculates that nothing has to be done wheras the other one
>>>> recognizes a change in the commandline, because it sees the command-line
>>>> for the other part of the reciepe.
>>>>
>>>> In the next make, the roles flip, because the previously satisfied
>>>> "if_changed" now sees the command-line of the other one. And so on...
>>>>
>>>> I am not a Kbuild expert but the attached patch fixes that problem by
>>>> introducing "if_changed_multi" that accepts two commands -- one whose
>>>> commandline should be checked and a second one that should be
>>>> executed.
>>>
>>>
>>> if_changed should not appear multiple times in one target.
>>>
>>> I think the simplest fix-up is to
>>> create a new command that combines
>>> 'cmd_check_data_rel' and 'cmd_ld'.
>>>
>>>
>>> quiet_cmd_link-vmlinux = LD $@
>>> cmd_link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
>>>
>>> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>>> $(call if_changed,link-vmlinux)
>>>
>>> Kbuild also supports if_changed_rule,
>>> but the usage is more complex.
>>>
>>> There are only a few usages:
>>> https://github.com/torvalds/linux/blob/v4.17/scripts/Makefile.build#L288
>>
>> Just for completeness I will copy in part of a reply from Kees that
>> shows how double-colon rules can also avoid multiple use of if_changed
>> for one target:
>>
>> -$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>> - $(call if_changed,check_data_rel)
>> +$(obj)/vmlinux:: $(vmlinux-objs-y)
>> + $(call cmd,check_data_rel)
>> +$(obj)/vmlinux:: $(vmlinux-objs-y) FORCE
>> $(call if_changed,ld)
>
> It is difficult to use double-colon rules in a _sane_ way.
>
> The first one just checks data_rel,
> but does not actually generate anything.
>
> Such targets should be marked as .PHONY,
> but $(obj)/vmlinux is not a phony target.
> This is strange.
>
>> The combined command seems to have the advantage that every command to
>> build the target gets recorded in the .cmd file
>>
>> A search showed me that we have two more users that use if_changed more
>> than once for a single target:
>>
>> arch/microblaze/boot/Makefile (fourfold)
>> arch/sparc/boot/Makefile (2 times twofold)
>>
>> The sparc case seems to apply to any of the two suggested fixes,
>
> Neither is correct.
>
>
> $(obj)/uImage: $(obj)/image.gz
> $(call if_changed,uimage)
> $(call if_changed,uimage.o)
>
>
> should be split into two targets.
>
>
> $(obj)/uImage: $(obj)/image.gz FORCE
> $(call if_changed,uimage)
>
> $(obj)/uImage.o: $(obj)/uImage FORCE
> $(call if_changed,uimage.o)
>
>
> It is wrong in multiple ways. FORCE is missing too.
>
> but
>> microblaze uses if_changed in a pattern rule and also makes use of
>> parameter arguments in the sub-commands:
>>
>> $(obj)/simpleImage.%: vmlinux FORCE
>> $(call if_changed,cp,.unstrip)
>> $(call if_changed,objcopy)
>> $(call if_changed,uimage)
>> $(call if_changed,strip,.strip)
>> @echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
>
>
> Probably, this is the same.
>
> Create a target for each step.
>
>
>> In this case, double colons would have a different meaning and the
>> combined command solution would result in a change of the sub-commands,
>> as well. I note this in case Michal perhaps has other preferences.
>>
>>
>> In addition to extend the documentation, we could modify if_changed to
>> warn about it is being used more than once for a target:
>>
>> # Execute command if command has changed or prerequisite(s) are updated.
>> if_changed = $(if $(filter-out undefined,$(origin if_changed_$@)), \
>> @set -e; \
>> echo "Warning: $@: multiple use of if_changed!" >&2; , \
>> @set -e $(eval if_changed_$@ := 1) ; ) \
>> $(if $(strip $(any-prereq) $(arg-check)), \
>> $(echo-cmd) $(cmd_$(1)); \
>> printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
>>
>> But this fires only if if_changed is actually called and it defines many
>> variables for just that purpose, so this is perhaps not what we want...
>>
>
> I do not want to mess up Makefile.
>
>
> Please do this check in scripts/checkpatch.pl
> if you want.
Thank you for spending your time in the detailed explanation.
I will use it to assemble that all to a fix and then send it for review.
Dirk
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 06/12] kconfig: allow all config targets to write auto.conf if missing
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (4 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 07/12] kbuild: use 'include' directive to load auto.conf from top Makefile Masahiro Yamada
` (6 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, linux-kernel
Currently, only syncconfig creates or updates include/config/auto.conf
and some other files. Other config targets create or update only the
.config file.
When you configure and build the kernel from a pristine source tree,
any config target is followed by syncconfig in the build stage since
include/config/auto.conf is missing.
We are moving compiler tests from Makefile to Kconfig. It means that
parsing Kconfig files will be more costly since Kconfig invokes the
compiler commands internally. Thus, we want to avoid invoking Kconfig
twice (one for *config to create the .config, and one for syncconfig
to synchronize the auto.conf). If auto.conf does not exist, we can
generate all configuration files in the first configuration stage,
which will save the syncconfig in the build stage.
Please note this should be done only when auto.conf is missing. If
*config blindly did this, time stamp files under include/config/ would
be unnecessarily touched, triggering unneeded rebuild of objects.
I assume a scenario like this:
1. You have a source tree that has already been built
with CONFIG_FOO disabled
2. Run "make menuconfig" to enable CONFIG_FOO
3. CONFIG_FOO turns out to be unnecessary.
Run "make menuconfig" again to disable CONFIG_FOO
4. Run "make"
In this case, include/config/foo.h should not be touched since there
is no change in CONFIG_FOO. The sync process should be delayed until
the user really attempts to build the kernel.
This commit has another motivation; I want to suppress the 'No such
file or directory' warning from the 'include' directive.
The top-level Makefile includes auto.conf with '-include' directive,
like this:
ifeq ($(dot-config),1)
-include include/config/auto.conf
endif
This looks strange because auto.conf is mandatory when dot-config is 1.
I guess only the reason of using '-include' is to suppress the warning
'include/config/auto.conf: No such file or directory' when building
from a clean tree. However, this has a side-effect; Make considers
the files included by '-include' are optional. Hence, Make continues
to build even if it fails to generate include/config/auto.conf. I will
change this in the next commit, but the warning message is annoying.
(At least, kbuild test robot reports it as a regression.)
With this commit, Kconfig will generate all configuration files together
with the .config and I guess it is a solution good enough to suppress
the warning.
Note:
GNU Make 4.2 or later does not display the warning from the 'include'
directive if include files are successfully generated. See GNU Make
commit 87a5f98d248f ("[SV 102] Don't show unnecessary include file
errors.") However, older GNU Make versions are still widely used.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/conf.c | 31 +++++++++++++++++--------------
scripts/kconfig/confdata.c | 11 +++++++----
scripts/kconfig/gconf.c | 1 +
scripts/kconfig/lkc_proto.h | 2 +-
scripts/kconfig/mconf.c | 1 +
scripts/kconfig/nconf.c | 1 +
scripts/kconfig/qconf.cc | 2 ++
7 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 5af8991..b35cc93 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -686,29 +686,32 @@ int main(int ac, char **av)
break;
}
- if (sync_kconfig) {
- /* syncconfig is used during the build so we shall update autoconf.
- * All other commands are only used to generate a config.
- */
- if (!no_conf_write && conf_write(NULL)) {
- fprintf(stderr, "\n*** Error during writing of the configuration.\n\n");
- exit(1);
- }
- if (conf_write_autoconf()) {
- fprintf(stderr, "\n*** Error during update of the configuration.\n\n");
- return 1;
- }
- } else if (input_mode == savedefconfig) {
+ if (input_mode == savedefconfig) {
if (conf_write_defconfig(defconfig_file)) {
fprintf(stderr, "n*** Error while saving defconfig to: %s\n\n",
defconfig_file);
return 1;
}
} else if (input_mode != listnewconfig) {
- if (conf_write(NULL)) {
+ if (!no_conf_write && conf_write(NULL)) {
fprintf(stderr, "\n*** Error during writing of the configuration.\n\n");
exit(1);
}
+
+ /*
+ * Create auto.conf if it does not exist.
+ * This prevents GNU Make 4.1 or older from emitting
+ * "include/config/auto.conf: No such file or directory"
+ * in the top-level Makefile
+ *
+ * syncconfig always creates or updates auto.conf because it is
+ * used during the build.
+ */
+ if (conf_write_autoconf(sync_kconfig) && sync_kconfig) {
+ fprintf(stderr,
+ "\n*** Error during sync of the configuration.\n\n");
+ return 1;
+ }
}
return 0;
}
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 38b3a6c..5af25a0 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -1012,13 +1012,17 @@ static int conf_split_config(void)
return res;
}
-int conf_write_autoconf(void)
+int conf_write_autoconf(int overwrite)
{
struct symbol *sym;
const char *name;
+ const char *autoconf_name = conf_get_autoconfig_name();
FILE *out, *tristate, *out_h;
int i;
+ if (!overwrite && is_file(autoconf_name))
+ return 0;
+
sym_clear_all_valid();
conf_write_dep("include/config/auto.conf.cmd");
@@ -1081,14 +1085,13 @@ int conf_write_autoconf(void)
if (rename(".tmpconfig_tristate", name))
return 1;
- name = conf_get_autoconfig_name();
- if (mkdir_p(name))
+ if (mkdir_p(autoconf_name))
return 1;
/*
* This must be the last step, kbuild has a dependency on auto.conf
* and this marks the successful completion of the previous steps.
*/
- if (rename(".tmpconfig", name))
+ if (rename(".tmpconfig", autoconf_name))
return 1;
return 0;
diff --git a/scripts/kconfig/gconf.c b/scripts/kconfig/gconf.c
index 610c4ab..f16ed51 100644
--- a/scripts/kconfig/gconf.c
+++ b/scripts/kconfig/gconf.c
@@ -525,6 +525,7 @@ void on_save_activate(GtkMenuItem * menuitem, gpointer user_data)
{
if (conf_write(NULL))
text_insert_msg("Error", "Unable to save configuration !");
+ conf_write_autoconf(0);
}
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index a8b7a33..b0cd52f 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -7,7 +7,7 @@ int conf_read(const char *name);
int conf_read_simple(const char *name, int);
int conf_write_defconfig(const char *name);
int conf_write(const char *name);
-int conf_write_autoconf(void);
+int conf_write_autoconf(int overwrite);
bool conf_get_changed(void);
void conf_set_changed_callback(void (*fn)(void));
void conf_set_message_callback(void (*fn)(const char *fmt, va_list ap));
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 5294ed1..82b27a0 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -977,6 +977,7 @@ static int handle_exit(void)
"\n\n");
return 1;
}
+ conf_write_autoconf(0);
/* fall through */
case -1:
if (!silent)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 97b7844..208f7be 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -674,6 +674,7 @@ static int do_exit(void)
"Your configuration changes were NOT saved.",
1,
"<OK>");
+ conf_write_autoconf(0);
break;
default:
btn_dialog(
diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index ad9c22d..62261b3 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1535,6 +1535,8 @@ bool ConfigMainWindow::saveConfig(void)
QMessageBox::information(this, "qconf", "Unable to save configuration!");
return false;
}
+ conf_write_autoconf(0);
+
return true;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 07/12] kbuild: use 'include' directive to load auto.conf from top Makefile
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (5 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 06/12] kconfig: allow all config targets to write auto.conf if missing Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 08/12] kbuild: add .DELETE_ON_ERROR special target Masahiro Yamada
` (5 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, Michal Marek, linux-kernel
When you build targets that require the kernel configuration, dot-config
is set to 1, then the top-level Makefile includes auto.conf. However,
Make considers its inclusion is optional because the '-include' directive
is used here.
If a necessary configuration file is missing for the external module
building, the following error message is displayed:
ERROR: Kernel configuration is invalid.
include/generated/autoconf.h or include/config/auto.conf are missing.
Run 'make oldconfig && make prepare' on kernel src to fix it.
However, Make still continues building; /bin/false let the creation of
'include/config/auto.config' fail, but Make can ignore the error since
it is included by the '-include' directive.
I guess the reason of using '-include' directive was to suppress
the warning when you build the kernel from a pristine source tree:
Makefile:605: include/config/auto.conf: No such file or directory
The previous commit made sure include/config/auto.conf exists after
the 'make *config' stage. Now, we can use the 'include' directive
without showing the warning.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d15ac32..ef24068 100644
--- a/Makefile
+++ b/Makefile
@@ -584,7 +584,7 @@ virt-y := virt/
endif # KBUILD_EXTMOD
ifeq ($(dot-config),1)
--include include/config/auto.conf
+include include/config/auto.conf
endif
# The all: target is the default when no target is given on the
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 08/12] kbuild: add .DELETE_ON_ERROR special target
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (6 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 07/12] kbuild: use 'include' directive to load auto.conf from top Makefile Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 09/12] kbuild: do not update config when running install targets Masahiro Yamada
` (4 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, Michal Marek, linux-kernel
If Make gets a fatal signal while a shell is executing, it may delete
the target file that the recipe was supposed to update. This is needed
to make sure that it is remade from scratch when Make is next run; if
Make is interrupted after the recipe has begun to write the target file,
it results in an incomplete file whose time stamp is newer than that
of the prerequisites files. Make automatically deletes the incomplete
file on interrupt unless the target is marked .PRECIOUS.
The situation is just the same as when the shell fails for some reasons.
Usually when a recipe line fails, if it has changed the target file at
all, the file is corrupted, or at least it is not completely updated.
Yet the file’s time stamp says that it is now up to date, so the next
time Make runs, it will not try to update that file.
However, Make does not cater to delete the incomplete target file in
this case. We need to add .DELETE_ON_ERROR somewhere in the Makefile
to request it.
scripts/Kbuild.include seems a suitable place to add it because it is
included from almost all sub-makes.
Please note .DELETE_ON_ERROR is not effective for phony targets.
The external module building should never ever touch the kernel tree.
The following recipe fails if include/generated/autoconf.h is missing.
However, include/config/auto.conf is not deleted since it is a phony
target.
PHONY += include/config/auto.conf
include/config/auto.conf:
$(Q)test -e include/generated/autoconf.h -a -e $@ || ( \
echo >&2; \
echo >&2 " ERROR: Kernel configuration is invalid."; \
echo >&2 " include/generated/autoconf.h or $@ are missing.";\
echo >&2 " Run 'make oldconfig && make prepare' on kernel src to fix it."; \
echo >&2 ; \
/bin/false)
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/Kbuild.include | 3 +++
1 file changed, 3 insertions(+)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index c8156d6..b2d14f1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -400,3 +400,6 @@ endif
endef
#
###############################################################################
+
+# delete partially updated (i.e. corrupted) files on error
+.DELETE_ON_ERROR:
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 09/12] kbuild: do not update config when running install targets
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (7 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 08/12] kbuild: add .DELETE_ON_ERROR special target Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 10/12] kbuild: do not update config for 'make kernelrelease' Masahiro Yamada
` (3 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, Michal Marek, linux-kernel
"make syncconfig" is automatically invoked when any of the following
happens:
- .config is updated
- any of Kconfig files is updated
- any of environment variables referenced in Kconfig is changed
Then, it updates configuration files such as include/config/auto.conf
include/generated/autoconf.h, etc.
Even install targets (install, modules_install, etc.) are no exception.
However, they should never ever modify the source tree. Install
targets are often run with root privileges. Once those configuration
files are owned by root, "make mrproper" would end up with permission
error.
Install targets should just copy things blindly. They should not care
whether the configuration is up-to-date or not. This makes more sense
because we are interested in the configuration that was used in the
previous kernel building.
This issue has existed since before, but rarely happened. I expect
more chance where people are hit by this; with the new Kconfig syntax
extension, the .config now contains the compiler information. If you
cross-compile the kernel with CROSS_COMPILE, but forget to pass it
for "make install", you meet "any of environment variables referenced
in Kconfig is changed" because $(CC) is referenced in Kconfig.
Another scenario is the compiler upgrade before the installation.
Install targets need the configuration. "make modules_install" refer
to CONFIG_MODULES etc. "make dtbs_install" also needs CONFIG_ARCH_*
to decide which dtb files to install. However, the auto-update of
the configuration files should be avoided. We already do this for
external modules.
Now, Make targets are categorized into 3 groups:
[1] Do not need the kernel configuration at all
help, coccicheck, headers_install etc.
[2] Need the latest kernel configuration
If new config options are added, Kconfig will show prompt to
ask user's selection.
Build targets such as vmlinux, in-kernel modules are the cases.
[3] Need the kernel configuration, but do not want to update it
Install targets except headers_install, and external modules
are the cases.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index ef24068..f0663c8 100644
--- a/Makefile
+++ b/Makefile
@@ -225,10 +225,12 @@ no-dot-config-targets := $(clean-targets) \
cscope gtags TAGS tags help% %docs check% coccicheck \
$(version_h) headers_% archheaders archscripts \
kernelversion %src-pkg
+no-sync-config-targets := $(no-dot-config-targets) install %install
-config-targets := 0
-mixed-targets := 0
-dot-config := 1
+config-targets := 0
+mixed-targets := 0
+dot-config := 1
+may-sync-config := 1
ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
@@ -236,6 +238,16 @@ ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
endif
endif
+ifneq ($(filter $(no-sync-config-targets), $(MAKECMDGOALS)),)
+ ifeq ($(filter-out $(no-sync-config-targets), $(MAKECMDGOALS)),)
+ may-sync-config := 0
+ endif
+endif
+
+ifneq ($(KBUILD_EXTMOD),)
+ may-sync-config := 0
+endif
+
ifeq ($(KBUILD_EXTMOD),)
ifneq ($(filter config %config,$(MAKECMDGOALS)),)
config-targets := 1
@@ -606,7 +618,7 @@ ARCH_CFLAGS :=
include arch/$(SRCARCH)/Makefile
ifeq ($(dot-config),1)
-ifeq ($(KBUILD_EXTMOD),)
+ifeq ($(may-sync-config),1)
# Read in dependencies to all Kconfig* files, make sure to run syncconfig if
# changes are detected. This should be included after arch/$(SRCARCH)/Makefile
# because some architectures define CROSS_COMPILE there.
@@ -621,8 +633,9 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
$(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
else
-# external modules needs include/generated/autoconf.h and include/config/auto.conf
-# but do not care if they are up-to-date. Use auto.conf to trigger the test
+# External modules and some install targets need include/generated/autoconf.h
+# and include/config/auto.conf but do not care if they are up-to-date.
+# Use auto.conf to trigger the test
PHONY += include/config/auto.conf
include/config/auto.conf:
@@ -634,7 +647,7 @@ include/config/auto.conf:
echo >&2 ; \
/bin/false)
-endif # KBUILD_EXTMOD
+endif # may-sync-config
else
# Dummy target needed, because used as prerequisite
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 10/12] kbuild: do not update config for 'make kernelrelease'
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (8 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 09/12] kbuild: do not update config when running install targets Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 11/12] kbuild: remove auto.conf and tristate.conf from prerequisites Masahiro Yamada
` (2 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, Michal Marek, linux-kernel
'make kernelrelease' depends on CONFIG_LOCALVERSION(_AUTO), but
for the same reason as install targets, we do not want to update
the configuration just for printing the kernelrelease string.
This is likely to happen when you compiled the kernel with
CROSS_COMPILE, but forget to pass it to 'make kernelrelease'.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index f0663c8..c258937 100644
--- a/Makefile
+++ b/Makefile
@@ -225,7 +225,8 @@ no-dot-config-targets := $(clean-targets) \
cscope gtags TAGS tags help% %docs check% coccicheck \
$(version_h) headers_% archheaders archscripts \
kernelversion %src-pkg
-no-sync-config-targets := $(no-dot-config-targets) install %install
+no-sync-config-targets := $(no-dot-config-targets) install %install \
+ kernelrelease
config-targets := 0
mixed-targets := 0
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 11/12] kbuild: remove auto.conf and tristate.conf from prerequisites
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (9 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 10/12] kbuild: do not update config for 'make kernelrelease' Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-05 2:39 ` [PATCH v3 12/12] kbuild: replace include/config/%.conf with include/config/auto.conf Masahiro Yamada
2018-07-10 11:34 ` [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Dirk Gouders
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, Michal Marek, linux-kernel
The top-level Makefile adds include/config/auto.conf as
prerequisites of 'scripts', 'prepare1', etc.
They were needed to terminate the build when include/config/auto.conf
is missing.
Now that the inclusion of include/config/auto.conf is mandatory
in the top-level Makefile if dot-config is 1 (Note 'include' directive
is used instead of '-include').
Make terminates the build by itself if it fails to create or update
include/config/auto.conf so we are sure that include/config/auto.conf
exists in the very first stage of make.
We do not have to explicitly list include/config/auto.conf as
prerequisites any more.
include/config/tristate.conf is generated as a side-effect of
syncconfig; if auto.conf exists, tristate.conf exists as well
(unless a user does 'rm include/config/tristate.conf')
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/Makefile b/Makefile
index c258937..c6ab38c4 100644
--- a/Makefile
+++ b/Makefile
@@ -649,10 +649,6 @@ include/config/auto.conf:
/bin/false)
endif # may-sync-config
-
-else
-# Dummy target needed, because used as prerequisite
-include/config/auto.conf: ;
endif # $(dot-config)
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
@@ -1047,15 +1043,14 @@ define filechk_kernel.release
endef
# Store (new) KERNELRELEASE string in include/config/kernel.release
-include/config/kernel.release: include/config/auto.conf FORCE
+include/config/kernel.release: $(srctree)/Makefile FORCE
$(call filechk,kernel.release)
# Additional helpers built in scripts/
# Carefully list dependencies so we do not try to build scripts twice
# in parallel
PHONY += scripts
-scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
- asm-generic gcc-plugins $(autoksyms_h)
+scripts: scripts_basic asm-generic gcc-plugins $(autoksyms_h)
$(Q)$(MAKE) $(build)=$(@)
# Things we need to do before we recursively start building the kernel
@@ -1085,8 +1080,7 @@ endif
# that need to depend on updated CONFIG_* values can be checked here.
prepare2: prepare3 outputmakefile asm-generic
-prepare1: prepare2 $(version_h) $(autoksyms_h) include/generated/utsrelease.h \
- include/config/auto.conf
+prepare1: prepare2 $(version_h) $(autoksyms_h) include/generated/utsrelease.h
$(cmd_crmodverdir)
archprepare: archheaders archscripts prepare1 scripts_basic
@@ -1224,7 +1218,7 @@ modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) modules.builtin
modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
$(Q)$(AWK) '!x[$$0]++' $^ > $(objtree)/modules.builtin
-%/modules.builtin: include/config/auto.conf
+%/modules.builtin:
$(Q)$(MAKE) $(modbuiltin)=$*
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 12/12] kbuild: replace include/config/%.conf with include/config/auto.conf
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (10 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 11/12] kbuild: remove auto.conf and tristate.conf from prerequisites Masahiro Yamada
@ 2018-07-05 2:39 ` Masahiro Yamada
2018-07-10 11:34 ` [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Dirk Gouders
12 siblings, 0 replies; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-05 2:39 UTC (permalink / raw)
To: linux-kbuild
Cc: Dirk Gouders, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Masahiro Yamada, Michal Marek, linux-kernel
Now that include/config/tristate.conf does not appear in the top-level
Makefile. Replace the pattern rule with include/config/auto.conf .
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index c6ab38c4..58bd78b 100644
--- a/Makefile
+++ b/Makefile
@@ -631,7 +631,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
# The actual configuration files used during the build are stored in
# include/generated/ and include/config/. Update them if .config is newer than
# include/config/auto.conf (which mirrors .config).
-include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
+include/config/auto.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
$(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
else
# External modules and some install targets need include/generated/autoconf.h
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 00/12] kbuild/kconfig: do not update config during installation
2018-07-05 2:39 [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Masahiro Yamada
` (11 preceding siblings ...)
2018-07-05 2:39 ` [PATCH v3 12/12] kbuild: replace include/config/%.conf with include/config/auto.conf Masahiro Yamada
@ 2018-07-10 11:34 ` Dirk Gouders
2018-07-12 8:29 ` Masahiro Yamada
12 siblings, 1 reply; 25+ messages in thread
From: Dirk Gouders @ 2018-07-10 11:34 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Ulf Magnusson, Linus Torvalds, Sam Ravnborg,
Michal Marek, linux-kernel
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> The main motivation of this patch series is to suppress the syncconfig
> during running installation targets.
>
> V1 consisted of only two patches:
> https://patchwork.kernel.org/patch/10468105/
> https://patchwork.kernel.org/patch/10468103/
>
> I noticed that installation targets would continue running
> even if the source tree is not configured at all
> because the inclusion of include/config/auto.conf was optional.
>
> So, I added one more patch in V2:
> https://patchwork.kernel.org/patch/10483637/
>
> However, kbuild test robot reported a new warning message was displayed:
>
> Makefile:592: include/config/auto.conf: No such file or directory
>
> This warning is displayed only for Make 4.1 or older.
>
> To fix this annoying warning, I changed Kconfig too,
> which leaded to more clean-up, improvements in Kconfig.
>
> So, V3 is a big patch series.
Hello Masahiro,
I tested your series for a while, now. I did not notice real issues
with it but want to leave some remarks about what I noticed in
the surroundings of your patches.
> Masahiro Yamada (12):
> kconfig: rename file_write_dep and move it to confdata.c
I might be missing some trivial use-case, but when looking at this
patch, I noticed an inconsistency with the file names auto.conf and
auto.conf.cmd.
The first can be modified by an environment variable but when this
happens, auto.conf.cmd remains as is. I noticed that only the
Documentation mentions that KCONFIG_AUTOCONFIG exists and confdata.c
uses it to serve the file name -- no other use anywhere.
Now, I am wondering if I just don't see an important case when the
use of KCONFIG_AUTOCONFIG is really helpful or even mandatory.
> kconfig: split out helpers to check file/directory, create directory
> kconfig: remove unneeded directory generation from local*config
> kconfig: create directories needed for syncconfig by itself
> kconfig: make syncconfig update .config regardless of sym_change_count
For this patch, I already mentioned that `conf --help' perhaps could be
updated. On the other side, none of the entries there tells us such
details, so there is probably no need for syncconfig to do so.
> kconfig: allow all config targets to write auto.conf if missing
> kbuild: use 'include' directive to load auto.conf from top Makefile
> kbuild: add .DELETE_ON_ERROR special target
> kbuild: do not update config when running install targets
> kbuild: do not update config for 'make kernelrelease'
> kbuild: remove auto.conf and tristate.conf from prerequisites
In the surrounding of this patch I noticed -include of auto.conf and
tristate.conf in scripts/Makfile.modbuildin. I tried it in some ways
but was not able to trigger that file being used with a missing
auto.conf. On the other hand, if I now manually remove tristate.conf,
that would not be fixed or even noticed, because of -include and I
wonder if it is safer to also change the -includes in that file.
It seems, if one of those files is missing, one must have done it
manually or some other serious issue is present that we probably want to
notice.
Dirk
> kbuild: replace include/config/%.conf with include/config/auto.conf
>
> Makefile | 46 +++++++++------
> scripts/Kbuild.include | 3 +
> scripts/kconfig/Makefile | 16 ++---
> scripts/kconfig/conf.c | 39 +++++++------
> scripts/kconfig/confdata.c | 139 +++++++++++++++++++++++++++++++++++++-------
> scripts/kconfig/gconf.c | 1 +
> scripts/kconfig/lkc.h | 1 -
> scripts/kconfig/lkc_proto.h | 2 +-
> scripts/kconfig/mconf.c | 1 +
> scripts/kconfig/nconf.c | 1 +
> scripts/kconfig/qconf.cc | 2 +
> scripts/kconfig/util.c | 30 ----------
> 12 files changed, 182 insertions(+), 99 deletions(-)
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 00/12] kbuild/kconfig: do not update config during installation
2018-07-10 11:34 ` [PATCH v3 00/12] kbuild/kconfig: do not update config during installation Dirk Gouders
@ 2018-07-12 8:29 ` Masahiro Yamada
2018-07-13 7:44 ` Dirk Gouders
0 siblings, 1 reply; 25+ messages in thread
From: Masahiro Yamada @ 2018-07-12 8:29 UTC (permalink / raw)
To: Dirk Gouders
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Michal Marek, Linux Kernel Mailing List
2018-07-10 20:34 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
>> The main motivation of this patch series is to suppress the syncconfig
>> during running installation targets.
>>
>> V1 consisted of only two patches:
>> https://patchwork.kernel.org/patch/10468105/
>> https://patchwork.kernel.org/patch/10468103/
>>
>> I noticed that installation targets would continue running
>> even if the source tree is not configured at all
>> because the inclusion of include/config/auto.conf was optional.
>>
>> So, I added one more patch in V2:
>> https://patchwork.kernel.org/patch/10483637/
>>
>> However, kbuild test robot reported a new warning message was displayed:
>>
>> Makefile:592: include/config/auto.conf: No such file or directory
>>
>> This warning is displayed only for Make 4.1 or older.
>>
>> To fix this annoying warning, I changed Kconfig too,
>> which leaded to more clean-up, improvements in Kconfig.
>>
>> So, V3 is a big patch series.
>
> Hello Masahiro,
>
> I tested your series for a while, now. I did not notice real issues
> with it but want to leave some remarks about what I noticed in
> the surroundings of your patches.
>
>
>> Masahiro Yamada (12):
>> kconfig: rename file_write_dep and move it to confdata.c
>
> I might be missing some trivial use-case, but when looking at this
> patch, I noticed an inconsistency with the file names auto.conf and
> auto.conf.cmd.
>
> The first can be modified by an environment variable but when this
> happens, auto.conf.cmd remains as is. I noticed that only the
> Documentation mentions that KCONFIG_AUTOCONFIG exists and confdata.c
> uses it to serve the file name -- no other use anywhere.
Indeed.
I had also noticed this.
Probably, replacing the hardcoded 'include/config/auto.conf.cmd'
with get_env('KCONFIG_AUTOCONFIG') + 'cmd' will be nice.
I did not touch it since it thought it was less important
for this patch set.
If you are willing to contribute to this,
a patch is welcome (after this series).
> Now, I am wondering if I just don't see an important case when the
> use of KCONFIG_AUTOCONFIG is really helpful or even mandatory.
I do not know the historical background,
but I guess predecessors wanted to implement Kconfig
as generic/flexible as possible.
>
>> kconfig: split out helpers to check file/directory, create directory
>> kconfig: remove unneeded directory generation from local*config
>> kconfig: create directories needed for syncconfig by itself
>> kconfig: make syncconfig update .config regardless of sym_change_count
>
> For this patch, I already mentioned that `conf --help' perhaps could be
> updated.
What do you want 'conf --help' to look like ?
> On the other side, none of the entries there tells us such
> details, so there is probably no need for syncconfig to do so.
>
>> kconfig: allow all config targets to write auto.conf if missing
>> kbuild: use 'include' directive to load auto.conf from top Makefile
>> kbuild: add .DELETE_ON_ERROR special target
>> kbuild: do not update config when running install targets
>> kbuild: do not update config for 'make kernelrelease'
>> kbuild: remove auto.conf and tristate.conf from prerequisites
>
> In the surrounding of this patch I noticed -include of auto.conf and
> tristate.conf in scripts/Makfile.modbuildin. I tried it in some ways
> but was not able to trigger that file being used with a missing
> auto.conf.
Right. auto.conf and tristate.conf are mandatory there.
'-include' should be replaced with 'include'.
Cater to send a patch?
> On the other hand, if I now manually remove tristate.conf,
> that would not be fixed or even noticed, because of -include and I
> wonder if it is safer to also change the -includes in that file.
>
> It seems, if one of those files is missing, one must have done it
> manually or some other serious issue is present that we probably want to
> notice.
You are right. If somebody removes tristate.conf on purpose,
it is not self-healing.
I should be fixed by itself, or at least it should fail
with clear message.
I will reconsider this.
Thanks.
> Dirk
>
>> kbuild: replace include/config/%.conf with include/config/auto.conf
>>
>> Makefile | 46 +++++++++------
>> scripts/Kbuild.include | 3 +
>> scripts/kconfig/Makefile | 16 ++---
>> scripts/kconfig/conf.c | 39 +++++++------
>> scripts/kconfig/confdata.c | 139 +++++++++++++++++++++++++++++++++++++-------
>> scripts/kconfig/gconf.c | 1 +
>> scripts/kconfig/lkc.h | 1 -
>> scripts/kconfig/lkc_proto.h | 2 +-
>> scripts/kconfig/mconf.c | 1 +
>> scripts/kconfig/nconf.c | 1 +
>> scripts/kconfig/qconf.cc | 2 +
>> scripts/kconfig/util.c | 30 ----------
>> 12 files changed, 182 insertions(+), 99 deletions(-)
> --
> 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
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 00/12] kbuild/kconfig: do not update config during installation
2018-07-12 8:29 ` Masahiro Yamada
@ 2018-07-13 7:44 ` Dirk Gouders
0 siblings, 0 replies; 25+ messages in thread
From: Dirk Gouders @ 2018-07-13 7:44 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Ulf Magnusson, Linus Torvalds,
Sam Ravnborg, Michal Marek, Linux Kernel Mailing List
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> 2018-07-10 20:34 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>
>>> The main motivation of this patch series is to suppress the syncconfig
>>> during running installation targets.
>>>
>>> V1 consisted of only two patches:
>>> https://patchwork.kernel.org/patch/10468105/
>>> https://patchwork.kernel.org/patch/10468103/
>>>
>>> I noticed that installation targets would continue running
>>> even if the source tree is not configured at all
>>> because the inclusion of include/config/auto.conf was optional.
>>>
>>> So, I added one more patch in V2:
>>> https://patchwork.kernel.org/patch/10483637/
>>>
>>> However, kbuild test robot reported a new warning message was displayed:
>>>
>>> Makefile:592: include/config/auto.conf: No such file or directory
>>>
>>> This warning is displayed only for Make 4.1 or older.
>>>
>>> To fix this annoying warning, I changed Kconfig too,
>>> which leaded to more clean-up, improvements in Kconfig.
>>>
>>> So, V3 is a big patch series.
>>
>> Hello Masahiro,
>>
>> I tested your series for a while, now. I did not notice real issues
>> with it but want to leave some remarks about what I noticed in
>> the surroundings of your patches.
>>
>>
>>> Masahiro Yamada (12):
>>> kconfig: rename file_write_dep and move it to confdata.c
>>
>> I might be missing some trivial use-case, but when looking at this
>> patch, I noticed an inconsistency with the file names auto.conf and
>> auto.conf.cmd.
>>
>> The first can be modified by an environment variable but when this
>> happens, auto.conf.cmd remains as is. I noticed that only the
>> Documentation mentions that KCONFIG_AUTOCONFIG exists and confdata.c
>> uses it to serve the file name -- no other use anywhere.
>
> Indeed.
>
> I had also noticed this.
>
> Probably, replacing the hardcoded 'include/config/auto.conf.cmd'
> with get_env('KCONFIG_AUTOCONFIG') + 'cmd' will be nice.
>
>
> I did not touch it since it thought it was less important
> for this patch set.
>
> If you are willing to contribute to this,
> a patch is welcome (after this series).
Yes, it is not that important but it would probably help people new to
kbuild/kconfig if we make this a bit more consistent. I will send a
patch that also adds some words to the documentation of
KCONFIG_AUTOCONFIG.
>> Now, I am wondering if I just don't see an important case when the
>> use of KCONFIG_AUTOCONFIG is really helpful or even mandatory.
>
>
> I do not know the historical background,
> but I guess predecessors wanted to implement Kconfig
> as generic/flexible as possible.
>
>
>>
>>> kconfig: split out helpers to check file/directory, create directory
>>> kconfig: remove unneeded directory generation from local*config
>>> kconfig: create directories needed for syncconfig by itself
>>> kconfig: make syncconfig update .config regardless of sym_change_count
>>
>> For this patch, I already mentioned that `conf --help' perhaps could be
>> updated.
>
> What do you want 'conf --help' to look like ?
As I said, --help does not say a lot about which files are updated, so I
probably was too pedantic. For --syncconfig it currently says:
--syncconfig Similar to oldconfig but generates configuration in
include/{generated/,config/}
which could let someone guess .config isn't touched (because of the
"but"). If you also think so, the text could perhaps say:
--syncconfig Similar to oldconfig but generates configuration in
include/{generated/,config/} in addition.
>
>> On the other side, none of the entries there tells us such
>> details, so there is probably no need for syncconfig to do so.
>>
>>> kconfig: allow all config targets to write auto.conf if missing
>>> kbuild: use 'include' directive to load auto.conf from top Makefile
>>> kbuild: add .DELETE_ON_ERROR special target
>>> kbuild: do not update config when running install targets
>>> kbuild: do not update config for 'make kernelrelease'
>>> kbuild: remove auto.conf and tristate.conf from prerequisites
>>
>> In the surrounding of this patch I noticed -include of auto.conf and
>> tristate.conf in scripts/Makfile.modbuildin. I tried it in some ways
>> but was not able to trigger that file being used with a missing
>> auto.conf.
>
> Right. auto.conf and tristate.conf are mandatory there.
>
> '-include' should be replaced with 'include'.
>
> Cater to send a patch?
I will do.
Dirk
>
>> On the other hand, if I now manually remove tristate.conf,
>> that would not be fixed or even noticed, because of -include and I
>> wonder if it is safer to also change the -includes in that file.
>>
>> It seems, if one of those files is missing, one must have done it
>> manually or some other serious issue is present that we probably want to
>> notice.
>
> You are right. If somebody removes tristate.conf on purpose,
> it is not self-healing.
>
> I should be fixed by itself, or at least it should fail
> with clear message.
>
> I will reconsider this.
>
>
> Thanks.
>
>
>
>> Dirk
>>
>>> kbuild: replace include/config/%.conf with include/config/auto.conf
>>>
>>> Makefile | 46 +++++++++------
>>> scripts/Kbuild.include | 3 +
>>> scripts/kconfig/Makefile | 16 ++---
>>> scripts/kconfig/conf.c | 39 +++++++------
>>> scripts/kconfig/confdata.c | 139 +++++++++++++++++++++++++++++++++++++-------
>>> scripts/kconfig/gconf.c | 1 +
>>> scripts/kconfig/lkc.h | 1 -
>>> scripts/kconfig/lkc_proto.h | 2 +-
>>> scripts/kconfig/mconf.c | 1 +
>>> scripts/kconfig/nconf.c | 1 +
>>> scripts/kconfig/qconf.cc | 2 +
>>> scripts/kconfig/util.c | 30 ----------
>>> 12 files changed, 182 insertions(+), 99 deletions(-)
>> --
>> 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
^ permalink raw reply [flat|nested] 25+ messages in thread