| Message ID | 20190510061205.28753-2-yamada.masahiro@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] kconfig: do not accept a directory for configuration output | expand |
On Fri, May 10, 2019 at 03:12:05PM +0900, Masahiro Yamada wrote: > Kconfig updates the .config when it exits even if its content is > exactly the same as before. Since its timestamp becomes newer than > that of other build artifacts, additional processing is invoked, > which is annoying. > > - syncconfig is invoked to update include/config/auto.conf, etc. > > - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, > then vmlinux is relinked as well. > > If the .config is not changed at all, we do not have to even > touch it. Just bail out showing "No change to .config". > > $ make allmodconfig > scripts/kconfig/conf --allmodconfig Kconfig > # > # configuration written to .config > # > $ make allmodconfig > scripts/kconfig/conf --allmodconfig Kconfig > # > # No change to .config > # > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reported-by: Linus Torvalds <torvalds@linux-foundation.org> ? :) Anyway, nice change, looks good to me: Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Hi Yamada-san, On Fri, May 10, 2019 at 8:14 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Kconfig updates the .config when it exits even if its content is > exactly the same as before. Since its timestamp becomes newer than > that of other build artifacts, additional processing is invoked, > which is annoying. > > - syncconfig is invoked to update include/config/auto.conf, etc. > > - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, > then vmlinux is relinked as well. > > If the .config is not changed at all, we do not have to even > touch it. Just bail out showing "No change to .config". > > $ make allmodconfig > scripts/kconfig/conf --allmodconfig Kconfig > # > # configuration written to .config > # > $ make allmodconfig > scripts/kconfig/conf --allmodconfig Kconfig > # > # No change to .config > # > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Thanks for your patch! > --- a/scripts/kconfig/confdata.c > +++ b/scripts/kconfig/confdata.c > @@ -888,6 +935,13 @@ int conf_write(const char *name) > fclose(out); > > if (*tmpname) { > + if (is_same(name, tmpname)) { > + conf_message("No change to %s", name); > + unlink(tmpname); > + sym_set_change_count(0); > + return 0; > + } > + > snprintf(oldname, sizeof(oldname), "%s.old", name); > rename(name, oldname); > if (rename(tmpname, name)) This causes a semantic change for the meaning of ".config.old", which is no longer updated if .config has not changed. Hence its contents may no longer correspond to the previous config, but to an arbitrary older version. My workflow involves always running my own script "linux-oldconfig", instead of "make oldconfig", so I immediately see what has changed: $ cat $(type -p linux-oldconfig) #!/bin/bash make ${0#*/linux-} && colordiff -u .config{.old,} However, after your patch, "linux-oldconfig" keeps on showing the old changes, while .config itself has not changed. I guess other people tracking .config changes are using a similar scheme. One way to fix this is to rename tmpname to oldname if is_same() returns true. However, this may have side effects for other people, as the timestamp of .config.old will be changed. Do you have a better solution? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert/Masahiro. On Fri, May 10, 2019 at 08:46:35AM +0200, Geert Uytterhoeven wrote: > Hi Yamada-san, > > On Fri, May 10, 2019 at 8:14 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > Kconfig updates the .config when it exits even if its content is > > exactly the same as before. Since its timestamp becomes newer than > > that of other build artifacts, additional processing is invoked, > > which is annoying. > > > > - syncconfig is invoked to update include/config/auto.conf, etc. > > > > - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, > > then vmlinux is relinked as well. > > > > If the .config is not changed at all, we do not have to even > > touch it. Just bail out showing "No change to .config". It would be preferable that if nothing changed no output is generated. Like we do not tell that we did not build a .o file because the .c file had not changed. Less noise for a kernel build where nothings happens. > This causes a semantic change for the meaning of ".config.old", which is > no longer updated if .config has not changed. > Hence its contents may no longer correspond to the previous config, but to > an arbitrary older version. This semantic change is good. So we now have a .config.old that correspond to the state before the last change. Also after several kernel builds. > > My workflow involves always running my own script "linux-oldconfig", > instead of "make oldconfig", so I immediately see what has changed: > > $ cat $(type -p linux-oldconfig) > #!/bin/bash > make ${0#*/linux-} && colordiff -u .config{.old,} So scripts relying on the old (broken) behaviour will no longer work. The new behaviour is better as it is usefaul in many typical situations. Hacking, hack. What did I change in the config? Sam
On Fri, May 10, 2019 at 3:21 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, May 10, 2019 at 03:12:05PM +0900, Masahiro Yamada wrote: > > Kconfig updates the .config when it exits even if its content is > > exactly the same as before. Since its timestamp becomes newer than > > that of other build artifacts, additional processing is invoked, > > which is annoying. > > > > - syncconfig is invoked to update include/config/auto.conf, etc. > > > > - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, > > then vmlinux is relinked as well. > > > > If the .config is not changed at all, we do not have to even > > touch it. Just bail out showing "No change to .config". > > > > $ make allmodconfig > > scripts/kconfig/conf --allmodconfig Kconfig > > # > > # configuration written to .config > > # > > $ make allmodconfig > > scripts/kconfig/conf --allmodconfig Kconfig > > # > > # No change to .config > > # > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> ? OK. For reviewers who do not know the context, please see this comment from Linus: https://lkml.org/lkml/2019/5/9/983 -- Best Regards Masahiro Yamada
Hi Sam, On Fri, May 10, 2019 at 9:03 AM Sam Ravnborg <sam@ravnborg.org> wrote: > On Fri, May 10, 2019 at 08:46:35AM +0200, Geert Uytterhoeven wrote: > > On Fri, May 10, 2019 at 8:14 AM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > Kconfig updates the .config when it exits even if its content is > > > exactly the same as before. Since its timestamp becomes newer than > > > that of other build artifacts, additional processing is invoked, > > > which is annoying. > > > > > > - syncconfig is invoked to update include/config/auto.conf, etc. > > > > > > - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, > > > then vmlinux is relinked as well. > > > > > > If the .config is not changed at all, we do not have to even > > > touch it. Just bail out showing "No change to .config". > > This causes a semantic change for the meaning of ".config.old", which is > > no longer updated if .config has not changed. > > Hence its contents may no longer correspond to the previous config, but to > > an arbitrary older version. > This semantic change is good. > So we now have a .config.old that correspond to the state before > the last change. Also after several kernel builds. > > > > My workflow involves always running my own script "linux-oldconfig", > > instead of "make oldconfig", so I immediately see what has changed: > > > > $ cat $(type -p linux-oldconfig) > > #!/bin/bash > > make ${0#*/linux-} && colordiff -u .config{.old,} > So scripts relying on the old (broken) behaviour will no longer work. > The new behaviour is better as it is usefaul in many typical situations. > > Hacking, hack. What did I change in the config? I agree both semantics have their merits. Sometimes I also wanted to see the last real change... No worries, updating my script, so it works with both semantics: $ cat $(type -p linux-oldconfig) #!/bin/bash cp -a .config .config.orig make ${0#*/linux-} && colordiff -u .config{.orig,} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Sam, Geert, On Fri, May 10, 2019 at 4:04 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Geert/Masahiro. > > On Fri, May 10, 2019 at 08:46:35AM +0200, Geert Uytterhoeven wrote: > > Hi Yamada-san, > > > > On Fri, May 10, 2019 at 8:14 AM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > Kconfig updates the .config when it exits even if its content is > > > exactly the same as before. Since its timestamp becomes newer than > > > that of other build artifacts, additional processing is invoked, > > > which is annoying. > > > > > > - syncconfig is invoked to update include/config/auto.conf, etc. > > > > > > - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, > > > then vmlinux is relinked as well. > > > > > > If the .config is not changed at all, we do not have to even > > > touch it. Just bail out showing "No change to .config". > It would be preferable that if nothing changed no output is generated. > Like we do not tell that we did not build a .o file because the .c file > had not changed. > Less noise for a kernel build where nothings happens. > > > This causes a semantic change for the meaning of ".config.old", which is > > no longer updated if .config has not changed. > > Hence its contents may no longer correspond to the previous config, but to > > an arbitrary older version. > This semantic change is good. > So we now have a .config.old that correspond to the state before > the last change. Also after several kernel builds. I agree. When there is no change in the configuration, Kconfig will not even attempt to output anything. Updating only .config.old is strange. Thanks. -- Best Regards Masahiro Yamada
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index 9fd6430c93d2..399973e35533 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -3,6 +3,7 @@ * Copyright (C) 2002 Roman Zippel <zippel@linux-m68k.org> */ +#include <sys/mman.h> #include <sys/stat.h> #include <ctype.h> #include <errno.h> @@ -36,6 +37,52 @@ static bool is_dir(const char *path) return S_ISDIR(st.st_mode); } +/* return true if the given two files are the same, false otherwise */ +static bool is_same(const char *file1, const char *file2) +{ + int fd1, fd2; + struct stat st1, st2; + void *map1, *map2; + bool ret = false; + + fd1 = open(file1, O_RDONLY); + if (fd1 < 0) + return ret; + + fd2 = open(file2, O_RDONLY); + if (fd2 < 0) + goto close1; + + ret = fstat(fd1, &st1); + if (ret) + goto close2; + ret = fstat(fd2, &st2); + if (ret) + goto close2; + + if (st1.st_size != st2.st_size) + goto close2; + + map1 = mmap(NULL, st1.st_size, PROT_READ, MAP_PRIVATE, fd1, 0); + if (map1 == MAP_FAILED) + goto close2; + + map2 = mmap(NULL, st2.st_size, PROT_READ, MAP_PRIVATE, fd2, 0); + if (map2 == MAP_FAILED) + goto close2; + + if (bcmp(map1, map2, st1.st_size)) + goto close2; + + ret = true; +close2: + close(fd2); +close1: + close(fd1); + + return ret; +} + /* * Create the parent directory of the given path. * @@ -888,6 +935,13 @@ int conf_write(const char *name) fclose(out); if (*tmpname) { + if (is_same(name, tmpname)) { + conf_message("No change to %s", name); + unlink(tmpname); + sym_set_change_count(0); + return 0; + } + snprintf(oldname, sizeof(oldname), "%s.old", name); rename(name, oldname); if (rename(tmpname, name))
Kconfig updates the .config when it exits even if its content is exactly the same as before. Since its timestamp becomes newer than that of other build artifacts, additional processing is invoked, which is annoying. - syncconfig is invoked to update include/config/auto.conf, etc. - kernel/config.o is recompiled if CONFIG_IKCONFIG is enabled, then vmlinux is relinked as well. If the .config is not changed at all, we do not have to even touch it. Just bail out showing "No change to .config". $ make allmodconfig scripts/kconfig/conf --allmodconfig Kconfig # # configuration written to .config # $ make allmodconfig scripts/kconfig/conf --allmodconfig Kconfig # # No change to .config # Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- scripts/kconfig/confdata.c | 54 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) -- 2.17.1