mbox series

[GIT,PULL,2/2] Kconfig updates for v5.10-rc1

Message ID CAK7LNASn4Si3=YhAPtc06wEqajpU0uBh46-4T10f=cHy=LY2iA@mail.gmail.com (mailing list archive)
State New
Headers show
Series None | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git tags/kconfig-v5.10

Message

Masahiro Yamada Oct. 22, 2020, 1:49 p.m. UTC
Hi Linus,

Please pull Kconfig updates for v5.10
Thanks.


The following changes since commit ba4f184e126b751d1bffad5897f263108befc780:

  Linux 5.9-rc6 (2020-09-20 16:33:55 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
tags/kconfig-v5.10

for you to fetch changes up to f9a825a7f65a1c94858667934c4ed59bc548dd1f:

  kconfig: qconf: create QApplication after option checks (2020-09-25
00:37:13 +0900)

----------------------------------------------------------------
Kconfig updates for v5.10

 - Remove unused for useless code from qconf

 - Allow to edit "int", "hex", "string" options in place, and remove the
   separate edit box from qconf

----------------------------------------------------------------
Masahiro Yamada (11):
      kconfig: qconf: reformat the intro message
      kconfig: qconf: update the intro message to match to the current code
      kconfig: qconf: remove unused ConfigItem::okRename()
      kconfig: qconf: move ConfigView::updateList(All) to ConfigList class
      kconfig: qconf: show data column all the time
      kconfig: qconf: allow to edit "int", "hex", "string" menus in-place
      kconfig: qconf: remove ConfigLineEdit class
      kconfig: qconf: move setShowName/Range() to ConfigList from ConfigView
      kconfig: qconf: remove ConfigView class
      kconfig: qconf: remove Y, M, N columns
      kconfig: qconf: create QApplication after option checks

 scripts/kconfig/qconf.cc | 368
+++++++++++++++++++++++++--------------------------------------
 scripts/kconfig/qconf.h  |  77 ++++---------
 2 files changed, 170 insertions(+), 275 deletions(-)

Comments

pr-tracker-bot@kernel.org Oct. 22, 2020, 8:25 p.m. UTC | #1
The pull request you sent on Thu, 22 Oct 2020 22:49:05 +0900:

> git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git tags/kconfig-v5.10

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f9893351acaecf0a414baf9942b48d5bb5c688c6

Thank you!
Linus Torvalds Nov. 27, 2020, 9:08 p.m. UTC | #2
Just a quick note, because it's been a small annoyance for a while (I
don't think it has anything to do with the 5.10 pull, I'm just
responding to your latest pull request)..

I have "make allmodconfig" taking unnecessarily long, and I finally
started asking myself "what's so expensive here". I'd expect it to be
basically instantaneous on my machine, and it isn't.

And when I looked at it, I noticed that it re-compiled
scripts/kconfig/conf every single time.

For no obvious reason I can see.

Doing a

       make --trace allmodconfig

shows a series of

    scripts/Makefile.host:112: target 'scripts/kconfig/....o' does not exist

lines, which is silly and wrong (they definitely exist), and I suspect
it's due to some confusion about the build directory or similar.

It's probably obvious to you once you start looking at it.

And yeah, I realize I'm being silly. Doing a "time make allmodconfig"
shows that it takes 1.5s to do. Should I care? No. But I feel that's
an eternity for something that I think should just be instantaneous.

                Linus
Linus Torvalds Nov. 27, 2020, 9:15 p.m. UTC | #3
On Fri, Nov 27, 2020 at 1:08 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>        make --trace allmodconfig
>
> shows a series of
>
>     scripts/Makefile.host:112: target 'scripts/kconfig/....o' does not exist
>
> lines, which is silly and wrong

Oh, this is a red herring. It's "make" output being misleading, and it
just comes from the FORCE keyword.

And no, those don't actually change the end result for me.

        Linus
Linus Torvalds Nov. 27, 2020, 9:53 p.m. UTC | #4
On Fri, Nov 27, 2020 at 1:15 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Oh, this is a red herring. It's "make" output being misleading, and it
> just comes from the FORCE keyword.
>
> And no, those don't actually change the end result for me.

.. and that red herring was what made me think that it always
recompiles the 'conf' binary. But no, that's not what is going on.

profiling shows that it does spend a lot of time in the compiler
(which was the other thing that made me incorrectly think it was the
conf program getting recompiled every time), but it looks like maybe
it's simply the cc-option testing that causes that:

    33.68%  cc1plus
    16.71%  cc1
    14.75%  ld
    11.36%  conf
     7.51%  sh
     7.21%  as
     3.01%  gcc
     2.44%  make
     0.58%  mkdir
     0.39%  rm
     0.33%  gcc-version.sh
     0.24%  collect2
     0.23%  cat
     0.22%  grep
     0.20%  cc-can-link.sh

Oh well, I clearly misread the problem. Maybe 1.5s is more reasonable
than I really expected it to be.

                  Linus
Linus Torvalds Nov. 27, 2020, 10:05 p.m. UTC | #5
On Fri, Nov 27, 2020 at 1:53 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>     33.68%  cc1plus

So a third of the time is the _single_ invocation of cc1plus, which
happens from scrips/gcc-plugin.sh doing that

     $HOSTCC -c -x c++ -std=gnu++98 - -fsyntax-only

thing. Which is purely to verify that plugins work.

Ugh.

Emese - I'm talking to myself while I'm looking at why "make
allmodconfig" is so unbearably slow. This is part of it.

              Linus
Masahiro Yamada Nov. 28, 2020, 7:04 a.m. UTC | #6
On Sat, Nov 28, 2020 at 7:05 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Nov 27, 2020 at 1:53 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> >     33.68%  cc1plus
>
> So a third of the time is the _single_ invocation of cc1plus, which
> happens from scrips/gcc-plugin.sh doing that
>
>      $HOSTCC -c -x c++ -std=gnu++98 - -fsyntax-only
>
> thing. Which is purely to verify that plugins work.
>
> Ugh.
>
> Emese - I'm talking to myself while I'm looking at why "make
> allmodconfig" is so unbearably slow. This is part of it.
>
>               Linus


If you do 'make allmodconfig' from the clean source tree,
some logs are displayed.

If you do that once again, no logs,
which means no recompilation of the 'conf' binary.

Of course, GNU Make evaluates some recipes due to the FORCE,
but the costs are quite small.



As for the cc1plus cost, I got a similar result.

Running scripts/gcc-plugin.sh directly
took me 0.5 sec, which is a fourth
of the allmodconfig run-time.

Actually, I did not know this shell script
was so expensive to run...

I also added Kees to CC.


Even if we are able to manage this script somehow,
Kconfig invocation still takes more than 1 sec
due to the current design.



masahiro@grover:~/workspace/linux$ make mrproper
masahiro@grover:~/workspace/linux$ time make allmodconfig
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  LEX     scripts/kconfig/lexer.lex.c
  YACC    scripts/kconfig/parser.tab.[ch]
  HOSTCC  scripts/kconfig/lexer.lex.o
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTCC  scripts/kconfig/util.o
  HOSTLD  scripts/kconfig/conf
#
# configuration written to .config
#

real 0m4.415s
user 0m3.686s
sys 0m0.763s
masahiro@grover:~/workspace/linux$ time make allmodconfig
#
# No change to .config
#

real 0m2.041s
user 0m1.564s
sys 0m0.519s

masahiro@grover:~/workspace/linux$ export HOSTCC=gcc
masahiro@grover:~/workspace/linux$ time  scripts/gcc-plugin.sh gcc

real 0m0.560s
user 0m0.512s
sys 0m0.048s
Linus Torvalds Nov. 28, 2020, 6:28 p.m. UTC | #7
On Fri, Nov 27, 2020 at 11:05 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> As for the cc1plus cost, I got a similar result.
>
> Running scripts/gcc-plugin.sh directly
> took me 0.5 sec, which is a fourth
> of the allmodconfig run-time.
>
> Actually, I did not know this shell script
> was so expensive to run...

So it turns out that one reason it's so expensive to run is that it
does a *lot* more than it claims to do.

It says "we need a c++ compiler that supports the designated
initializer GNU extension", but then it actually includes a header
file from hell, rather than just test designated initializers.

This patch makes the cc1plus overhead go down a lot. That said, I'm
doubtful we really want gcc plugins at all, considering that the only
real users have all apparently migrated to clang builtin functionality
instead.

        Linus
Kees Cook Dec. 2, 2020, 12:55 a.m. UTC | #8
On Sat, Nov 28, 2020 at 10:28:31AM -0800, Linus Torvalds wrote:
> On Fri, Nov 27, 2020 at 11:05 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > As for the cc1plus cost, I got a similar result.
> >
> > Running scripts/gcc-plugin.sh directly
> > took me 0.5 sec, which is a fourth
> > of the allmodconfig run-time.
> >
> > Actually, I did not know this shell script
> > was so expensive to run...
> 
> So it turns out that one reason it's so expensive to run is that it
> does a *lot* more than it claims to do.
> 
> It says "we need a c++ compiler that supports the designated
> initializer GNU extension", but then it actually includes a header
> file from hell, rather than just test designated initializers.
> 
> This patch makes the cc1plus overhead go down a lot. That said, I'm
> doubtful we really want gcc plugins at all, considering that the only
> real users have all apparently migrated to clang builtin functionality
> instead.
> 
>         Linus

>  scripts/gcc-plugin.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/gcc-plugin.sh b/scripts/gcc-plugin.sh
> index b79fd0bea838..59db87bff456 100755
> --- a/scripts/gcc-plugin.sh
> +++ b/scripts/gcc-plugin.sh
> @@ -8,8 +8,8 @@ srctree=$(dirname "$0")
>  gccplugins_dir=$($* -print-file-name=plugin)
>  
>  # we need a c++ compiler that supports the designated initializer GNU extension
> +test -e "$gccplugins_dir/include/plugin-version.h" &&
>  $HOSTCC -c -x c++ -std=gnu++98 - -fsyntax-only -I $srctree/gcc-plugins -I $gccplugins_dir/include 2>/dev/null <<EOF
> -#include "gcc-common.h"
>  class test {
>  public:
>  	int test;

I'm fine dropping this -- I think the need for that portion of the
script's test has evaporated as we've brought the minimum GCC version
up into the neighborhood of "modern".

As for dropping GCC plugins entirely, I'd prefer not -- the big hold-out
for the very paranoid system builders is the randstruct plugin (though
they tend to also use the entropy one too). Clang's version of randstruct
has not gotten unstuck yet.
Masahiro Yamada Dec. 2, 2020, 12:53 p.m. UTC | #9
Hi Linus,

On Sun, Nov 29, 2020 at 3:28 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Nov 27, 2020 at 11:05 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > As for the cc1plus cost, I got a similar result.
> >
> > Running scripts/gcc-plugin.sh directly
> > took me 0.5 sec, which is a fourth
> > of the allmodconfig run-time.
> >
> > Actually, I did not know this shell script
> > was so expensive to run...
>
> So it turns out that one reason it's so expensive to run is that it
> does a *lot* more than it claims to do.
>
> It says "we need a c++ compiler that supports the designated
> initializer GNU extension", but then it actually includes a header
> file from hell, rather than just test designated initializers.
>
> This patch makes the cc1plus overhead go down a lot. That said, I'm
> doubtful we really want gcc plugins at all, considering that the only
> real users have all apparently migrated to clang builtin functionality
> instead.
>
>         Linus


The attached patch looks OK to me.

Just a nit:
Now that the test code does not include any header,
you can also delete
"-I $srctree/gcc-plugins -I $gccplugins_dir/include"


If you apply it directly, please feel free to add

Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>
Masahiro Yamada Dec. 2, 2020, 1:03 p.m. UTC | #10
On Wed, Dec 2, 2020 at 9:53 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Hi Linus,
>
> On Sun, Nov 29, 2020 at 3:28 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Fri, Nov 27, 2020 at 11:05 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > As for the cc1plus cost, I got a similar result.
> > >
> > > Running scripts/gcc-plugin.sh directly
> > > took me 0.5 sec, which is a fourth
> > > of the allmodconfig run-time.
> > >
> > > Actually, I did not know this shell script
> > > was so expensive to run...
> >
> > So it turns out that one reason it's so expensive to run is that it
> > does a *lot* more than it claims to do.
> >
> > It says "we need a c++ compiler that supports the designated
> > initializer GNU extension", but then it actually includes a header
> > file from hell, rather than just test designated initializers.
> >
> > This patch makes the cc1plus overhead go down a lot. That said, I'm
> > doubtful we really want gcc plugins at all, considering that the only
> > real users have all apparently migrated to clang builtin functionality
> > instead.
> >
> >         Linus
>
>
> The attached patch looks OK to me.
>
> Just a nit:
> Now that the test code does not include any header,
> you can also delete
> "-I $srctree/gcc-plugins -I $gccplugins_dir/include"
>
>
> If you apply it directly, please feel free to add
>
> Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>


BTW, gcc plugins are always compiled with g++.

Why do we need to compile the following in the first place?

class test {
public:
        int test;
} test = {
        .test = 1
};


I think any C++ compiler will succeed
in compiling such simple code.



So,

test -e "$gccplugins_dir/include/plugin-version.h"

looks enough to me.



What is the intention of this compile test?
Kees Cook Dec. 2, 2020, 6:58 p.m. UTC | #11
On Wed, Dec 02, 2020 at 10:03:47PM +0900, Masahiro Yamada wrote:
> On Wed, Dec 2, 2020 at 9:53 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Hi Linus,
> >
> > On Sun, Nov 29, 2020 at 3:28 AM Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > >
> > > On Fri, Nov 27, 2020 at 11:05 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > As for the cc1plus cost, I got a similar result.
> > > >
> > > > Running scripts/gcc-plugin.sh directly
> > > > took me 0.5 sec, which is a fourth
> > > > of the allmodconfig run-time.
> > > >
> > > > Actually, I did not know this shell script
> > > > was so expensive to run...
> > >
> > > So it turns out that one reason it's so expensive to run is that it
> > > does a *lot* more than it claims to do.
> > >
> > > It says "we need a c++ compiler that supports the designated
> > > initializer GNU extension", but then it actually includes a header
> > > file from hell, rather than just test designated initializers.
> > >
> > > This patch makes the cc1plus overhead go down a lot. That said, I'm
> > > doubtful we really want gcc plugins at all, considering that the only
> > > real users have all apparently migrated to clang builtin functionality
> > > instead.
> > >
> > >         Linus
> >
> >
> > The attached patch looks OK to me.
> >
> > Just a nit:
> > Now that the test code does not include any header,
> > you can also delete
> > "-I $srctree/gcc-plugins -I $gccplugins_dir/include"
> >
> >
> > If you apply it directly, please feel free to add
> >
> > Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>
> 
> 
> BTW, gcc plugins are always compiled with g++.
> 
> Why do we need to compile the following in the first place?
> 
> class test {
> public:
>         int test;
> } test = {
>         .test = 1
> };
> 
> 
> I think any C++ compiler will succeed
> in compiling such simple code.
> 
> 
> 
> So,
> 
> test -e "$gccplugins_dir/include/plugin-version.h"
> 
> looks enough to me.
> 
> 
> 
> What is the intention of this compile test?

Yeah, I'd agree: we're just looking for a g++ and plugin-version.h.
Linus Torvalds Dec. 2, 2020, 8:13 p.m. UTC | #12
On Wed, Dec 2, 2020 at 4:54 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Just a nit:
> Now that the test code does not include any header,
> you can also delete
> "-I $srctree/gcc-plugins -I $gccplugins_dir/include"

Ahh,m yes.

It sounds like we might be able to delete the build test entirely if
we just always expect to have a recent enough gcc.

Testing the headers for existence would presumably still be needed,
just to verify "do we have plugin support installed at all".

But I'm not planning on applying this directly - I find the config
overhead to be a bit annoying, but it's not like it is _objectively_
really a problem. More of a personal hangup ;)

        Linus