All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings
@ 2016-07-18 16:00 Arnd Bergmann
  2016-07-18 19:31 ` Josh Triplett
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Arnd Bergmann @ 2016-07-18 16:00 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Michal Marek, Yann E . MORIN, Josh Triplett, Darren Hart,
	kernel-build-reports, Thiago Macieira, Linux Kernel Mailing List,
	Masahiro Yamada, Arnd Bergmann

Using "make tinyconfig" produces a couple of annoying warnings that show up
for build test machines all the time:

    .config:966:warning: override: NOHIGHMEM changes choice state
    .config:965:warning: override: SLOB changes choice state
    .config:963:warning: override: KERNEL_XZ changes choice state
    .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
    .config:933:warning: override: SLOB changes choice state
    .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
    .config:870:warning: override: SLOB changes choice state
    .config:868:warning: override: KERNEL_XZ changes choice state
    .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state

I've made a previous attempt at fixing them and we discussed a number of
alternatives.

I tried changing the Makefile to use "merge_config.sh -n $(fragment-list)"
but couldn't get that to work properly.

This is yet another approach, based on the observation that we do want
to see a warning for conflicting 'choice' options, and that we can simply
make them non-conflicting by listing all other options as disabled.
This is a trivial patch that we can apply independent of plans for other
changes.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log
https://patchwork.kernel.org/patch/9212749/
---
 kernel/configs/tiny.config | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/configs/tiny.config b/kernel/configs/tiny.config
index c2de56ab0fce..3eeade4d876d 100644
--- a/kernel/configs/tiny.config
+++ b/kernel/configs/tiny.config
@@ -1,4 +1,12 @@
 CONFIG_CC_OPTIMIZE_FOR_SIZE=y
+# CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
 CONFIG_KERNEL_XZ=y
+# CONFIG_KERNEL_GZIP is not set
+# CONFIG_KERNEL_BZIP2 is not set
+# CONFIG_KERNEL_LZMA is not set
+# CONFIG_KERNEL_LZO is not set
+# CONFIG_KERNEL_LZ4 is not set
 CONFIG_OPTIMIZE_INLINING=y
+# CONFIG_SLUB is not set
+# CONFIG_SLAB is not set
 CONFIG_SLOB=y
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings
  2016-07-18 16:00 [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Arnd Bergmann
@ 2016-07-18 19:31 ` Josh Triplett
  2016-07-18 19:50   ` Arnd Bergmann
  2016-07-18 19:32 ` [PATCH 1/1] kconfig: tinyconfig: x86: List disabled choices to avoid warning Josh Triplett
  2016-07-21  1:21 ` [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Masahiro Yamada
  2 siblings, 1 reply; 5+ messages in thread
From: Josh Triplett @ 2016-07-18 19:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux Kbuild mailing list, Michal Marek, Yann E . MORIN,
	Darren Hart, kernel-build-reports, Thiago Macieira,
	Linux Kernel Mailing List, Masahiro Yamada

On Mon, Jul 18, 2016 at 06:00:23PM +0200, Arnd Bergmann wrote:
> Using "make tinyconfig" produces a couple of annoying warnings that show up
> for build test machines all the time:
> 
>     .config:966:warning: override: NOHIGHMEM changes choice state
>     .config:965:warning: override: SLOB changes choice state
>     .config:963:warning: override: KERNEL_XZ changes choice state
>     .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
>     .config:933:warning: override: SLOB changes choice state
>     .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
>     .config:870:warning: override: SLOB changes choice state
>     .config:868:warning: override: KERNEL_XZ changes choice state
>     .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> 
> I've made a previous attempt at fixing them and we discussed a number of
> alternatives.
> 
> I tried changing the Makefile to use "merge_config.sh -n $(fragment-list)"
> but couldn't get that to work properly.
> 
> This is yet another approach, based on the observation that we do want
> to see a warning for conflicting 'choice' options, and that we can simply
> make them non-conflicting by listing all other options as disabled.
> This is a trivial patch that we can apply independent of plans for other
> changes.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

This does introduce additional warnings for changing those newly listed
options, but then we only have one type of warning, which will go away
with whatever fix you make to the underlying config merge machinery.

I'll send a follow-up patch doing the same for
arch/x86/configs/tiny.config, whose one config symbol (NOHIGHMEM) also
forms part of a choice and produces the same warning.

I find it *mildly* annoying that this means the configs will need to
change whenever any new choices appear, but at least we'll have warnings
to tell us that.

>  kernel/configs/tiny.config | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/kernel/configs/tiny.config b/kernel/configs/tiny.config
> index c2de56ab0fce..3eeade4d876d 100644
> --- a/kernel/configs/tiny.config
> +++ b/kernel/configs/tiny.config
> @@ -1,4 +1,12 @@
>  CONFIG_CC_OPTIMIZE_FOR_SIZE=y
> +# CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
>  CONFIG_KERNEL_XZ=y
> +# CONFIG_KERNEL_GZIP is not set
> +# CONFIG_KERNEL_BZIP2 is not set
> +# CONFIG_KERNEL_LZMA is not set
> +# CONFIG_KERNEL_LZO is not set
> +# CONFIG_KERNEL_LZ4 is not set
>  CONFIG_OPTIMIZE_INLINING=y
> +# CONFIG_SLUB is not set
> +# CONFIG_SLAB is not set
>  CONFIG_SLOB=y
> -- 
> 2.9.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] kconfig: tinyconfig: x86: List disabled choices to avoid warning
  2016-07-18 16:00 [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Arnd Bergmann
  2016-07-18 19:31 ` Josh Triplett
@ 2016-07-18 19:32 ` Josh Triplett
  2016-07-21  1:21 ` [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Masahiro Yamada
  2 siblings, 0 replies; 5+ messages in thread
From: Josh Triplett @ 2016-07-18 19:32 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Arnd Bergmann, Michal Marek, Yann E . MORIN, Darren Hart,
	kernel-build-reports, Thiago Macieira, Linux Kernel Mailing List,
	Masahiro Yamada

The Kconfig machinery used to power tinyconfig produces a warning like
this when changing a "choice" option without explicitly disabling the
other choices:

.config:965:warning: override: NOHIGHMEM changes choice state

Expand arch/x86/configs/tiny.config to add entries disabling the other
two choices, to eliminate this warning.  (Doing so then introduces a
different form of warning for changing any option at all, but that needs
fixing in the underlying config merge machinery.)

Based on a similar change from Arnd Bergmann for the
architecture-independent tiny.config.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Reported-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/configs/tiny.config | 2 ++
 1 file changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/configs/tiny.config b/arch/x86/configs/tiny.config
index 4e2ecfa..4b429df 100644
--- a/arch/x86/configs/tiny.config
+++ b/arch/x86/configs/tiny.config
@@ -1 +1,3 @@
 CONFIG_NOHIGHMEM=y
+# CONFIG_HIGHMEM4G is not set
+# CONFIG_HIGHMEM64G is not set
-- 
git-series 0.8.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings
  2016-07-18 19:31 ` Josh Triplett
@ 2016-07-18 19:50   ` Arnd Bergmann
  0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2016-07-18 19:50 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Linux Kbuild mailing list, Michal Marek, Yann E . MORIN,
	Darren Hart, kernel-build-reports, Thiago Macieira,
	Linux Kernel Mailing List, Masahiro Yamada

On Monday, July 18, 2016 12:31:40 PM CEST Josh Triplett wrote:
> On Mon, Jul 18, 2016 at 06:00:23PM +0200, Arnd Bergmann wrote:
> > Using "make tinyconfig" produces a couple of annoying warnings that show up
> > for build test machines all the time:
> > 
> >     .config:966:warning: override: NOHIGHMEM changes choice state
> >     .config:965:warning: override: SLOB changes choice state
> >     .config:963:warning: override: KERNEL_XZ changes choice state
> >     .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> >     .config:933:warning: override: SLOB changes choice state
> >     .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> >     .config:870:warning: override: SLOB changes choice state
> >     .config:868:warning: override: KERNEL_XZ changes choice state
> >     .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
> > 
> > I've made a previous attempt at fixing them and we discussed a number of
> > alternatives.
> > 
> > I tried changing the Makefile to use "merge_config.sh -n $(fragment-list)"
> > but couldn't get that to work properly.
> > 
> > This is yet another approach, based on the observation that we do want
> > to see a warning for conflicting 'choice' options, and that we can simply
> > make them non-conflicting by listing all other options as disabled.
> > This is a trivial patch that we can apply independent of plans for other
> > changes.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> This does introduce additional warnings for changing those newly listed
> options, but then we only have one type of warning, which will go away
> with whatever fix you make to the underlying config merge machinery.

I didn't actually consider them warnings but rather config output:
They are written to stdout instead of stderr, and they don't have
the word 'warning' in them:

Value of CONFIG_CC_OPTIMIZE_FOR_SIZE is redefined by fragment /git/arm-soc/kernel/configs/tiny.config:
Previous value: # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
New value: CONFIG_CC_OPTIMIZE_FOR_SIZE=y

It's probably fine to leave this output present, just like 'make oldconfig'
prints some informational messages about things that have been changed.
In particular, none of the build bots I know will record that as warnings.

On the other hand, I think it's a good idea to not print them when
building with 'make -s', just like we hide all other informational
output, maybe just by redirecting the output of merge_config.sh to
/dev/null, or by adding a '-q' argument.

> I'll send a follow-up patch doing the same for
> arch/x86/configs/tiny.config, whose one config symbol (NOHIGHMEM) also
> forms part of a choice and produces the same warning.
> 
> I find it *mildly* annoying that this means the configs will need to
> change whenever any new choices appear, but at least we'll have warnings
> to tell us that.

Agreed. It's also annoying for the cases of very long choice statements
with dozens of options.

	Arnd

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings
  2016-07-18 16:00 [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Arnd Bergmann
  2016-07-18 19:31 ` Josh Triplett
  2016-07-18 19:32 ` [PATCH 1/1] kconfig: tinyconfig: x86: List disabled choices to avoid warning Josh Triplett
@ 2016-07-21  1:21 ` Masahiro Yamada
  2 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2016-07-21  1:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux Kbuild mailing list, Michal Marek, Yann E . MORIN,
	Josh Triplett, Darren Hart, kernel-build-reports,
	Thiago Macieira, Linux Kernel Mailing List

Hi Arnd,


2016-07-19 1:00 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> Using "make tinyconfig" produces a couple of annoying warnings that show up
> for build test machines all the time:
>
>     .config:966:warning: override: NOHIGHMEM changes choice state
>     .config:965:warning: override: SLOB changes choice state
>     .config:963:warning: override: KERNEL_XZ changes choice state
>     .config:962:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
>     .config:933:warning: override: SLOB changes choice state
>     .config:930:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
>     .config:870:warning: override: SLOB changes choice state
>     .config:868:warning: override: KERNEL_XZ changes choice state
>     .config:867:warning: override: CC_OPTIMIZE_FOR_SIZE changes choice state
>
> I've made a previous attempt at fixing them and we discussed a number of
> alternatives.
>
> I tried changing the Makefile to use "merge_config.sh -n $(fragment-list)"
> but couldn't get that to work properly.
>
> This is yet another approach, based on the observation that we do want
> to see a warning for conflicting 'choice' options, and that we can simply
> make them non-conflicting by listing all other options as disabled.
> This is a trivial patch that we can apply independent of plans for other
> changes.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Link: https://storage.kernelci.org/mainline/v4.7-rc6/x86-tinyconfig/build.log
> https://patchwork.kernel.org/patch/9212749/
> ---
>  kernel/configs/tiny.config | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/kernel/configs/tiny.config b/kernel/configs/tiny.config
> index c2de56ab0fce..3eeade4d876d 100644
> --- a/kernel/configs/tiny.config
> +++ b/kernel/configs/tiny.config
> @@ -1,4 +1,12 @@
>  CONFIG_CC_OPTIMIZE_FOR_SIZE=y
> +# CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
>  CONFIG_KERNEL_XZ=y
> +# CONFIG_KERNEL_GZIP is not set
> +# CONFIG_KERNEL_BZIP2 is not set
> +# CONFIG_KERNEL_LZMA is not set
> +# CONFIG_KERNEL_LZO is not set
> +# CONFIG_KERNEL_LZ4 is not set
>  CONFIG_OPTIMIZE_INLINING=y
> +# CONFIG_SLUB is not set
> +# CONFIG_SLAB is not set
>  CONFIG_SLOB=y


Which sorting policy are you using here?


I prefer the same order as they appear in the .config file.
Like this.


+# CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
 CONFIG_CC_OPTIMIZE_FOR_SIZE=y
+# CONFIG_KERNEL_GZIP is not set
+# CONFIG_KERNEL_BZIP2 is not set
+# CONFIG_KERNEL_LZMA is not set
 CONFIG_KERNEL_XZ=y
+# CONFIG_KERNEL_LZO is not set
+# CONFIG_KERNEL_LZ4 is not set
 CONFIG_OPTIMIZE_INLINING=y
+# CONFIG_SLAB is not set
+# CONFIG_SLUB is not set
 CONFIG_SLOB=y


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-07-21  1:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 16:00 [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Arnd Bergmann
2016-07-18 19:31 ` Josh Triplett
2016-07-18 19:50   ` Arnd Bergmann
2016-07-18 19:32 ` [PATCH 1/1] kconfig: tinyconfig: x86: List disabled choices to avoid warning Josh Triplett
2016-07-21  1:21 ` [PATCH] kconfig: tinyconfig: provide whole choice blocks to avoid warnings Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.