| Message ID | 20180216214117.1947175-3-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | fixing the last failures in randconfig builds | expand |
2018-02-17 6:41 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: > The kernel currently supports two methods of dealing with config > fragments in the tree: > > a) Running "make foo.config" looks for arch/$(ARCH)/configs/foo.config > and kernel/configs/foo.config, and applies the defaults from those > files on top of the current configuration. > > b) Running "KCONFIG_ALLCONFIG=1 make randconfig" (or the equivalent > allmodconfig/allnoconfig/allyesconfig/alldefconfig) will look > for a "allrandconfig.config" file in the current directory or the > top of the $(srctree). These are used as defaults before we generate > the remaining options. > > This is rather inconsistent, and prevents us from easily shipping > good defaults for "randconfig". I'm extending the logic here so that > the second case also looks for the hardcoded file names in the standard > directories (first arch/$(ARCH)/configs/, then kernel/configs) in the > source tree. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > scripts/kconfig/conf.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c > index 866369f10ff8..848bf4d15e9a 100644 > --- a/scripts/kconfig/conf.c > +++ b/scripts/kconfig/conf.c > @@ -493,8 +493,9 @@ int main(int ac, char **av) > { > const char *progname = av[0]; > int opt; > - const char *name, *defconfig_file = NULL /* gcc uninit */; > + const char *arch, *name, *defconfig_file = NULL /* gcc uninit */; > struct stat tmpstat; > + char fullname[PATH_MAX+1]; > > setlocale(LC_ALL, ""); > bindtextdomain(PACKAGE, LOCALEDIR); > @@ -621,14 +622,24 @@ int main(int ac, char **av) > case randconfig: name = "allrandom.config"; break; > default: break; > } > - if (conf_read_simple(name, S_DEF_USER) && > - conf_read_simple("all.config", S_DEF_USER)) { > - fprintf(stderr, > - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), > - name); > - exit(1); > + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ > + if (!conf_read_simple(name, S_DEF_USER)) > + break; > + arch = getenv("ARCH"); > + if (arch) { > + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", > + arch, name); > + if (!conf_read_simple(fullname, S_DEF_USER)) > + break; > } I am not a big fan of hard-coding the kernel directory structure. We already do this [1], but I am thinking of kicking this out. [1] https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33 BTW, I am trying to compiler capability check to Kconfig. If this work is done, I hope some "depends on !COMPILE_TEST" will go away (but not all?) https://patchwork.kernel.org/patch/10225375/ > - break; > + snprintf(fullname, sizeof(fullname), "kernel/configs/%s", name); > + if (!conf_read_simple(fullname, S_DEF_USER)) > + break; > + > + fprintf(stderr, > + _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), > + name); > + exit(1); > default: > break; > } > -- > 2.9.0 > > -- > 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
On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-02-17 6:41 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >> @@ -621,14 +622,24 @@ int main(int ac, char **av) >> case randconfig: name = "allrandom.config"; break; >> default: break; >> } >> - if (conf_read_simple(name, S_DEF_USER) && >> - conf_read_simple("all.config", S_DEF_USER)) { >> - fprintf(stderr, >> - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), >> - name); >> - exit(1); >> + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ >> + if (!conf_read_simple(name, S_DEF_USER)) >> + break; >> + arch = getenv("ARCH"); >> + if (arch) { >> + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", >> + arch, name); >> + if (!conf_read_simple(fullname, S_DEF_USER)) >> + break; >> } > > > I am not a big fan of hard-coding the kernel directory structure. > > We already do this [1], but I am thinking of kicking this out. > [1] https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33 Ok, I see. How about adding a way to detect that we are build-testing with randconfig instead? > BTW, I am trying to compiler capability check to Kconfig. > If this work is done, I hope some "depends on !COMPILE_TEST" will go > away (but not all?) > https://patchwork.kernel.org/patch/10225375/ I think most of the 'depends on !COMPILE_TEST' are for cases that impact compile-testing in some way (more false-positive warnings, fewer real warnings, much slower compile speed), rather than a lack of check for a specific compiler version. We might be able to replace the CONFIG_STANDALONE and CONFIG_PREVENT_FIRMWARE_BUILD with Kconfig $(shell) checks if you think that leads to an improvement. Arnd
2018-02-20 18:59 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: > On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: > >>> @@ -621,14 +622,24 @@ int main(int ac, char **av) >>> case randconfig: name = "allrandom.config"; break; >>> default: break; >>> } >>> - if (conf_read_simple(name, S_DEF_USER) && >>> - conf_read_simple("all.config", S_DEF_USER)) { >>> - fprintf(stderr, >>> - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), >>> - name); >>> - exit(1); >>> + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ >>> + if (!conf_read_simple(name, S_DEF_USER)) >>> + break; >>> + arch = getenv("ARCH"); >>> + if (arch) { >>> + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", >>> + arch, name); >>> + if (!conf_read_simple(fullname, S_DEF_USER)) >>> + break; >>> } >> >> >> I am not a big fan of hard-coding the kernel directory structure. >> >> We already do this [1], but I am thinking of kicking this out. >> [1] https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33 > > Ok, I see. How about adding a way to detect that we are build-testing > with randconfig instead? How about implementing something in scripts/kconfig/Makefile? merge_config collects config fragments into 'configfiles' I was thinking of a similar thing. If KCONFIG_ALLCONFIG is 1, scripts/kconfig/Makefile searches './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/' and sets the found file in it, If KCONFIG_ALLCONFIG already contains a file name, it is used as-is. scripts/kconfig/conf.c will be simplified. >> BTW, I am trying to compiler capability check to Kconfig. >> If this work is done, I hope some "depends on !COMPILE_TEST" will go >> away (but not all?) >> https://patchwork.kernel.org/patch/10225375/ > > I think most of the 'depends on !COMPILE_TEST' are for cases that > impact compile-testing in some way (more false-positive warnings, > fewer real warnings, much slower compile speed), rather than a lack > of check for a specific compiler version. > > We might be able to replace the CONFIG_STANDALONE and > CONFIG_PREVENT_FIRMWARE_BUILD with Kconfig $(shell) > checks if you think that leads to an improvement. > OK, then your kernel/configs/allrandom.config approach will be useful. -- Best Regards Masahiro Yamada
On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-02-20 18:59 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada >> <yamada.masahiro@socionext.com> wrote: >>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >> >>>> @@ -621,14 +622,24 @@ int main(int ac, char **av) >>>> case randconfig: name = "allrandom.config"; break; >>>> default: break; >>>> } >>>> - if (conf_read_simple(name, S_DEF_USER) && >>>> - conf_read_simple("all.config", S_DEF_USER)) { >>>> - fprintf(stderr, >>>> - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), >>>> - name); >>>> - exit(1); >>>> + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ >>>> + if (!conf_read_simple(name, S_DEF_USER)) >>>> + break; >>>> + arch = getenv("ARCH"); >>>> + if (arch) { >>>> + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", >>>> + arch, name); >>>> + if (!conf_read_simple(fullname, S_DEF_USER)) >>>> + break; >>>> } >>> >>> >>> I am not a big fan of hard-coding the kernel directory structure. >>> >>> We already do this [1], but I am thinking of kicking this out. >>> [1] https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33 >> >> Ok, I see. How about adding a way to detect that we are build-testing >> with randconfig instead? > > > How about implementing something in scripts/kconfig/Makefile? > > merge_config collects config fragments into 'configfiles' > > I was thinking of a similar thing. > > > If KCONFIG_ALLCONFIG is 1, > scripts/kconfig/Makefile searches > './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/' > and sets the found file in it, > > If KCONFIG_ALLCONFIG already contains a file name, it is used as-is. > > scripts/kconfig/conf.c will be simplified. Yes, good idea. I'm struggling a bit with the implementation (fitting it into the simple-tagets rule), can you come up with a prototype? We also need to decide what happens when e.g. both a ./all.config and arch/${SRCARCH}/configs/allrand.config file exist, should the first path take precedence or the more specific file? Arnd
2018-02-22 1:57 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: > On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada >>> <yamada.masahiro@socionext.com> wrote: >>>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >>> >>>>> @@ -621,14 +622,24 @@ int main(int ac, char **av) >>>>> case randconfig: name = "allrandom.config"; break; >>>>> default: break; >>>>> } >>>>> - if (conf_read_simple(name, S_DEF_USER) && >>>>> - conf_read_simple("all.config", S_DEF_USER)) { >>>>> - fprintf(stderr, >>>>> - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), >>>>> - name); >>>>> - exit(1); >>>>> + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ >>>>> + if (!conf_read_simple(name, S_DEF_USER)) >>>>> + break; >>>>> + arch = getenv("ARCH"); >>>>> + if (arch) { >>>>> + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", >>>>> + arch, name); >>>>> + if (!conf_read_simple(fullname, S_DEF_USER)) >>>>> + break; >>>>> } >>>> >>>> >>>> I am not a big fan of hard-coding the kernel directory structure. >>>> >>>> We already do this [1], but I am thinking of kicking this out. >>>> [1] https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33 >>> >>> Ok, I see. How about adding a way to detect that we are build-testing >>> with randconfig instead? >> >> >> How about implementing something in scripts/kconfig/Makefile? >> >> merge_config collects config fragments into 'configfiles' >> >> I was thinking of a similar thing. >> >> >> If KCONFIG_ALLCONFIG is 1, >> scripts/kconfig/Makefile searches >> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/' >> and sets the found file in it, >> >> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is. >> >> scripts/kconfig/conf.c will be simplified. > > Yes, good idea. I'm struggling a bit with the implementation (fitting > it into the > simple-tagets rule), can you come up with a prototype? > > We also need to decide what happens when e.g. both a ./all.config and > arch/${SRCARCH}/configs/allrand.config file exist, should the first path > take precedence or the more specific file? > Hmm, maybe the following order? [1] ./allrandom.config [2] $(srctree)/allrandom.config [3] ./all.config [4] $(srctree)/all.config [5] kernel/configs/allrandom.config + arch/$(SRCARCH)/configs/allrandom.config I think you want to include both kernel/configs/allrandom.config and arch/$(SRCARCH)/configs/allrandom.config if exist. Some options are common, some are arch-specific. This is how tinyconfig works. -- Best Regards Masahiro Yamada
2018-02-22 1:57 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: > On Tue, Feb 20, 2018 at 6:04 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> 2018-02-20 18:59 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >>> On Tue, Feb 20, 2018 at 10:26 AM, Masahiro Yamada >>> <yamada.masahiro@socionext.com> wrote: >>>> 2018-02-17 6:41 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: >>> >>>>> @@ -621,14 +622,24 @@ int main(int ac, char **av) >>>>> case randconfig: name = "allrandom.config"; break; >>>>> default: break; >>>>> } >>>>> - if (conf_read_simple(name, S_DEF_USER) && >>>>> - conf_read_simple("all.config", S_DEF_USER)) { >>>>> - fprintf(stderr, >>>>> - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), >>>>> - name); >>>>> - exit(1); >>>>> + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ >>>>> + if (!conf_read_simple(name, S_DEF_USER)) >>>>> + break; >>>>> + arch = getenv("ARCH"); >>>>> + if (arch) { >>>>> + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", >>>>> + arch, name); >>>>> + if (!conf_read_simple(fullname, S_DEF_USER)) >>>>> + break; >>>>> } >>>> >>>> >>>> I am not a big fan of hard-coding the kernel directory structure. >>>> >>>> We already do this [1], but I am thinking of kicking this out. >>>> [1] https://github.com/torvalds/linux/blob/v4.16-rc1/scripts/kconfig/confdata.c#L33 >>> >>> Ok, I see. How about adding a way to detect that we are build-testing >>> with randconfig instead? >> >> >> How about implementing something in scripts/kconfig/Makefile? >> >> merge_config collects config fragments into 'configfiles' >> >> I was thinking of a similar thing. >> >> >> If KCONFIG_ALLCONFIG is 1, >> scripts/kconfig/Makefile searches >> './', '$srctree/', 'arch/$(SRCARCH)/configs/', 'kernel/configs/' >> and sets the found file in it, >> >> If KCONFIG_ALLCONFIG already contains a file name, it is used as-is. >> >> scripts/kconfig/conf.c will be simplified. > > Yes, good idea. I'm struggling a bit with the implementation (fitting > it into the > simple-tagets rule), can you come up with a prototype? Just a prototype for the Makefile part. (We need to agree with the search order specification, though) ------------>8------------- configs_dirs := $(srctree)/kernel/configs/ $(srctree)/arch/$(SRCARCH)/configs/ allconfig_files = $(if $(filter 1, $(KCONFIG_ALLCONFIG)),$(firstword \ $(foreach f, $(1), $(wildcard $(addsuffix $(f), ./ $(srctree)/))) \ $(foreach f, $(1), $(subst $(space),$(comma),$(wildcard $(addsuffix $(f), $(configs_dirs)))))), \ $(KCONFIG_ALLCONFIG)) allyesconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files, allyes.config all.config) allmodconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files, allmoyes.config all.config) (likewise for allnoconfig, alldefconfig) randconfig: override KCONFIG_ALLCONFIG := $(call allconfig_files, allrandom.config all.config) ------------->8----------- If a user specifies a file path in KCONFIG_ALLCONFIG, it is used as-is. IF KCONFIG_ALLCONFIG=1, the input file is searched in the following order. ( [1] - [4] is the same as the current specification) [1] ./allrandom.config [2] $(srctree)/allrandom.config [3] ./all.config [4] $(srctree)/all.config [5] $(srctree)/kernel/configs/allrandom.config + $(srctree)/arch/$(SRCARCH)/configs/allrandom.config [6] $(srctree)/kernel/configs/all.config + $(srctree)/arch/$(SRCARCH)/configs/all.config For [5] and [6], if multiple files are found, KCONFIG_ALLCONFIG is set to comma-separated file paths. scripts/kconfig/conf.c must be tweaked a little bit if we need to read multiple config fragments. Currently, tinyconfig for x86 comes from common part and arch-specific part. ./kernel/configs/tiny.config ./arch/x86/configs/tiny.config What do you think? > We also need to decide what happens when e.g. both a ./all.config and > arch/${SRCARCH}/configs/allrand.config file exist, should the first path > take precedence or the more specific file? > -- Best Regards Masahiro Yamada
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 866369f10ff8..848bf4d15e9a 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -493,8 +493,9 @@ int main(int ac, char **av) { const char *progname = av[0]; int opt; - const char *name, *defconfig_file = NULL /* gcc uninit */; + const char *arch, *name, *defconfig_file = NULL /* gcc uninit */; struct stat tmpstat; + char fullname[PATH_MAX+1]; setlocale(LC_ALL, ""); bindtextdomain(PACKAGE, LOCALEDIR); @@ -621,14 +622,24 @@ int main(int ac, char **av) case randconfig: name = "allrandom.config"; break; default: break; } - if (conf_read_simple(name, S_DEF_USER) && - conf_read_simple("all.config", S_DEF_USER)) { - fprintf(stderr, - _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), - name); - exit(1); + /* try ./name, arch/$(ARCH)/configs/name and kernel/config/name */ + if (!conf_read_simple(name, S_DEF_USER)) + break; + arch = getenv("ARCH"); + if (arch) { + snprintf(fullname, sizeof(fullname), "arch/%s/configs/%s", + arch, name); + if (!conf_read_simple(fullname, S_DEF_USER)) + break; } - break; + snprintf(fullname, sizeof(fullname), "kernel/configs/%s", name); + if (!conf_read_simple(fullname, S_DEF_USER)) + break; + + fprintf(stderr, + _("*** KCONFIG_ALLCONFIG set, but no \"%s\" or \"all.config\" file found\n"), + name); + exit(1); default: break; }
The kernel currently supports two methods of dealing with config fragments in the tree: a) Running "make foo.config" looks for arch/$(ARCH)/configs/foo.config and kernel/configs/foo.config, and applies the defaults from those files on top of the current configuration. b) Running "KCONFIG_ALLCONFIG=1 make randconfig" (or the equivalent allmodconfig/allnoconfig/allyesconfig/alldefconfig) will look for a "allrandconfig.config" file in the current directory or the top of the $(srctree). These are used as defaults before we generate the remaining options. This is rather inconsistent, and prevents us from easily shipping good defaults for "randconfig". I'm extending the logic here so that the second case also looks for the hardcoded file names in the standard directories (first arch/$(ARCH)/configs/, then kernel/configs) in the source tree. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- scripts/kconfig/conf.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) -- 2.9.0