linux-kernel - Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

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] [day] [month] [year] [list]

Message-ID: <CAK7LNATCmMTTEZUBhSVawVM_+r6KsuYnBCdxX_p3GqbKBH3DEw@mail.gmail.com>
Date:   Wed, 21 Nov 2018 11:47:19 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Nadav Amit <namit@...are.com>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Michal Marek <michal.lkml@...kovi.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

On Tue, Nov 20, 2018 at 2:21 AM Nadav Amit <namit@...are.com> wrote:
>
> at 8:20 PM, Masahiro Yamada <yamada.masahiro@...ionext.com> wrote:
>
> > On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit <namit@...are.com> wrote:
> >> From: Masahiro Yamada
> >> Sent: November 16, 2018 at 7:45:45 AM GMT
> >>> To: Nadav Amit <namit@...are.com>
> >>> Cc: Ingo Molnar <mingo@...hat.com>, Michal Marek <michal.lkml@...kovi.net>, Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>, H. Peter Anvin <hpa@...or.com>, X86 ML <x86@...nel.org>, Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
> >>> Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
> >>>
> >>>
> >>> On Thu, Nov 15, 2018 at 1:01 PM Nadav Amit <namit@...are.com> wrote:
> >>>> Introducing the use of asm macros in c-code broke distcc, since it only
> >>>> sends the preprocessed source file. The solution is to break the
> >>>> compilation into two separate phases of compilation and assembly, and
> >>>> between the two concatenate the assembly macros and the compiled (yet
> >>>> not assembled) source file. Since this is less efficient, this
> >>>> compilation mode is only used when distcc or icecc are used.
> >>>>
> >>>> Note that the assembly stage should also be distributed, if distcc is
> >>>> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
> >>>>
> >>>> Reported-by: Logan Gunthorpe <logang@...tatee.com>
> >>>> Signed-off-by: Nadav Amit <namit@...are.com>
> >>>
> >>>
> >>> Wow, this is so ugly.
> >>
> >> Indeed, it is not pretty from the Makefile system point of view.
> >>
> >> But it does have merits when it comes to the actual use (by developers). If
> >> you look on x86, there are a lot of duplicated implementation for C and Asm
> >> and crazy C macros, for example for PV function call or the alternative
> >> mechanism. The code is also less readable in its current form.
> >>
> >>> I realized how much I hated this by now.
> >>>
> >>> My question is, how long do we need to carry this?
> >>
> >> If it is purely about performance, I am not sure it would make sense to
> >> put it in, as it will indeed be (eventually) solved by the compiler, and
> >> the penalty is not too great.
> >
> >
> > It is unfortunate to mess up the code
> > by having two different ways to achieve the same goal.
>
> Indeed, but I fail to see how this statement fits with the next sentence.

I am not tracking the progress on GCC side.
I just did not sure if the 'asm inline' work was on the right track.


> > When GCC with asm inline support is shipped,
> > would you revert 77b0bf55 ?
>
> This patch and the following ones were not written to be reverted, i.e.,
> reverting them later may not be easy since they remove redundant C inline
> assembly chunks.
>
> Since gcc will solve the inline assembly issues, the value of these patches
> is in unifying the assembly that is used in .c and .S files. Due to the lack
> of m4-like preprocessor in the Linux make-system, the solution is a bit
> “hacky” (in other words, I don’t see a reasonable alternative solution).
>
> So there is a downside in the form of performance degradation of distcc, and
> hacky (not too hacky?) Makefile changes. On the upside, except addressing
> the gcc statement cost computation (inline) issue, the patch removes
> redundant code, and improves assembly code readability.

It is true that the assembly part itself is readable
but the readability as a whole is worse IMHO.

Each macro implementation was split into "asm volatile" (in C) +
".macro" (in ASM) parts.


> In addition, it
> provides the option to make assembly manipulations after compilation, which
> HPA and others (me) look into.
>
>  Anyhow, if after all, the downside is considered greater than the upside,
> it is best to remove the patches sooner than later, as a revert later will
> be more painful.

Then, I'd be happier with the revert now.



--
Best Regards
Masahiro Yamada