Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1728622 > unrolled thread

Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of copying _shipped files

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-09-08 08:20 +0200
    Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-08 19:30 +0200
      Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-08 20:10 +0200
        Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-08 20:40 +0200
          Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-08 23:40 +0200
            Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Sam Ravnborg <sam@ravnborg.org> - 2017-09-09 08:40 +0200
              Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-09-10 16:10 +0200
            Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of  copying _shipped files Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-09-10 16:00 +0200

#1728622 — Re: [RFC PATCH 0/3] kbuild: generate intermediate C files instead of copying _shipped files

Hi Linus,


Very sorry that I had not responded quickly.

When I was digging into tool version dependency,
as you pointed out, gperf is a problem (but seemed one time breakage
for gperf 3.1)
flex seemed very stable for a long time.
bison seemed a bit problem if old version is used.


But, I did not have enough time to take a closer look.
I thought I should respond after I tested more, but I has been pressed
by my daily tasks, then time passed...
Very sorry.



Today, I just noticed gperf usage got dropped from the kernel.

If CONFIG_MODVERSIONS is enabled,
I notice lots of error messages.
WARNING: EXPORT symbol "finish_open" [vmlinux] version generation
failed, symbol will not be


So, I think something was broken in scripts/genksyms/.

Of course, it was a trivial conversion, so it should not be hard to fix...



> gperf is clearly written by clowns that don't understand about
> compatibility issues - it would have been trivial for them to add some
> kind of marker define so that you could test for this directly rather
> than depend on some kind of autoconf "try to build and see if it
> fails" crap.


One idea may be to process the output of "gperf -v"
and embed GPERF_VERSION into the output .c files.

But, if you are unhappy with gperf breakage this time,
we can live without gperf.




> It's likely not even any slower, but who the hell knows.. Do we even
> care? It's almost certainly faster if you compare to generating that
> gperf code.
>

For scripts/kconfig/, I think we do not care at all
because we usually invoke it just once when we configure the build setting.


If we enable CONFIG_MODVERSIONS, scripts/genksyms/ is invoked
over and over again.

Sorry, I have not evaluated if
the perfect hash gives us noticeable advantage or not..



-- 
Best Regards
Masahiro Yamada

[toc] | [next] | [standalone]

#1729111

On Thu, Sep 7, 2017 at 11:18 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> If CONFIG_MODVERSIONS is enabled,
> I notice lots of error messages.
> WARNING: EXPORT symbol "finish_open" [vmlinux] version generation
> failed, symbol will not be versioned
>
> So, I think something was broken in scripts/genksyms/.
>
> Of course, it was a trivial conversion, so it should not be hard to fix...

Indeed, hopefully it would be trivial, but I don't even see the error here.

Of course, I only did a "make allmodconfig" to test the MODVERSIONS
case, I didn't actually install the modules. Is that error perhaps
only detected at install time?

I did build and install a kernel with that patch, but that's my actual
"real" config for the machine, and it didn't have MODVERSIONS enabled.

Oh, how I hate modversions. But I'll take a look if I can see what I
did wrong in the "Trivial and Obvious(tm)" conversion.

                  Linus

[toc] | [prev] | [next] | [standalone]

#1729150

On Fri, Sep 8, 2017 at 10:22 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Of course, I only did a "make allmodconfig" to test the MODVERSIONS
> case, I didn't actually install the modules. Is that error perhaps
> only detected at install time?

Oh, I take that back. I just got a ton of warnings with my
allmodconfig after doing a "git clean -dqfx".

So I guess there is a dependency issue - my normal build test after
the merge didn't show any issues, simply because the change in
ksymoops didn't actually cause the version information to be
re-generated.

It doesn't seem to happen for every exported symbol, though. Odd.

Looking at it, but not making sense of it yet.

                Linus

[toc] | [prev] | [next] | [standalone]

#1729193

On Fri, Sep 8, 2017 at 11:01 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> It doesn't seem to happen for every exported symbol, though. Odd.

Fascinating.

Picking one file at random that shows this, I did net/ceph/mon_client.c.

The version file that gets generated for that looks like this:

__crc_ceph_monc_want_map = 0x3389a57e;
__crc_ceph_monc_got_map = 0x707f45a2;
__crc_ceph_monc_renew_subs = 0x9842030a;
__crc_ceph_monc_wait_osdmap = 0xfe38746d;
__crc_ceph_monc_open_session = 0xc9ba8a19;
__crc_ceph_monc_do_statfs = 0xe878801b;
__crc_ceph_monc_get_version = 0xfaac6ce0;
__crc_ceph_monc_get_version_async = 0x3adefe28;
__crc_ceph_monc_blacklist_add = 0xee71d0ef;
__crc_ceph_monc_init = 0xfce99654;
__crc_ceph_monc_stop = 0xb0d197d0;
__crc_ceph_monc_validate_auth = 0xce1c6d69;

and with the gperf-removal patch, it is 100% identical _except_ that
the "__crc_ceph_monc_do_statfs" line is missing.

Same hashes for everything else. But one missing line. WTF?

What is special about that one particular function vs the other ones
in that file? I have absolutely no idea.

So the really odd thing here is how things clearly still _work_. The
parser works fine for everything else. And looking at the
gprof-removal patch it's not at all obvious how everything could work
fine except for some random thing.

Strange. Does anybody see what the pattern to the failure is?

                Linus

[toc] | [prev] | [next] | [standalone]

#1729301

On Fri, Sep 8, 2017 at 11:39 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Strange. Does anybody see what the pattern to the failure is?

Found it. Stupid special case for 'typeof()' that used
is_reserved_word() in ways I hadn't realized.

Fix committed.

             Linus

[toc] | [prev] | [next] | [standalone]

#1729393

On Fri, Sep 08, 2017 at 02:38:23PM -0700, Linus Torvalds wrote:
> On Fri, Sep 8, 2017 at 11:39 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Strange. Does anybody see what the pattern to the failure is?
> 
> Found it. Stupid special case for 'typeof()' that used
> is_reserved_word() in ways I hadn't realized.
> 
> Fix committed.

To get bonus points for this cleanup you should also remove
the now unused gpref support in Makefile.lib

	Sam

[toc] | [prev] | [next] | [standalone]

#1729979

Hi Sam,

2017-09-09 15:39 GMT+09:00 Sam Ravnborg <sam@ravnborg.org>:
> On Fri, Sep 08, 2017 at 02:38:23PM -0700, Linus Torvalds wrote:
>> On Fri, Sep 8, 2017 at 11:39 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>> >
>> > Strange. Does anybody see what the pattern to the failure is?
>>
>> Found it. Stupid special case for 'typeof()' that used
>> is_reserved_word() in ways I hadn't realized.
>>
>> Fix committed.
>
> To get bonus points for this cleanup you should also remove
> the now unused gpref support in Makefile.lib
>
>         Sam

Yes.

Linus committed c054be10ffd

Thanks!



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1729977

Hi Linus,


2017-09-09 6:38 GMT+09:00 Linus Torvalds <torvalds@linux-foundation.org>:
> On Fri, Sep 8, 2017 at 11:39 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Strange. Does anybody see what the pattern to the failure is?
>
> Found it. Stupid special case for 'typeof()' that used
> is_reserved_word() in ways I hadn't realized.
>
> Fix committed.
>
>              Linus


"is_reserved_word()" sounds like a boolean function
that returns 1 or 0.
Maybe, the choice of the function name was not nice.

Anyway, thanks a lot for taking care of all this!



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web