linux-kernel - Re: [PATCH v9 04/10] x86: refcount: prevent gcc distortions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181004091222.GB21864@gmail.com>
Date:   Thu, 4 Oct 2018 11:12:22 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Nadav Amit <namit@...are.com>
Cc:     "hpa@...or.com" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jan Beulich <JBeulich@...e.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v9 04/10] x86: refcount: prevent gcc distortions

* Nadav Amit <namit@...are.com> wrote:

> I can run some tests. (@hpa: I thought you asked about the -pipe overhead;
> perhaps I misunderstood).

Well, tests are unlikely to show the overhead of extra lines of this
magnitude, unless done very carefully, yet the added bloat exists and is not even
mentioned by the changelog, it just says:

  Subject: [PATCH v9 02/10] Makefile: Prepare for using macros for inline asm

  Using macros for inline assembly improves both readability and
  compilation decisions that are distorted by big assembly blocks that use
  alternative sections. Compile macros.S and use it to assemble all C
  files. Currently, only x86 will use it.

> I guess you regard to the preprocessing of the assembler. Note that the C 
> preprocessing of macros.S obviously happens only once. That’s the reason
> I assumed it’s not that expensive.

True - so first we build macros.s, and that gets included in every C file build, right?

macros.s is smaller: 275 lines only in the distro test build I tried, which looks
a lot better than my first 4,200 lines guesstimate.

> Anyhow, I remember that we discussed at some point doing something like
> ‘asm(“.include XXX.s”)’ and somebody said it is not good, but I don’t
> remember why and don’t see any reason it is so. Unless I am missing
> something, I think it is possible to take each individual header and
> preprocess the assembly part of into a separate .s file. Then we can put in
> the C part of the header ‘asm(".include XXX.s”)’.
> 
> What do you think?

Hm, this looks quite complex - macros.s is better I think. Also, 275 straight assembly lines is 
a lot better than 4,200.

Another, separate question I wanted to ask: how do we ensure that the kernel stays fixed?
I.e. is there some tooling we can use to actually measure whether there's bad inlining decisions 
done, to detect all these bad patterns that cause bad GCC code generation?

Thanks,

	Ingo