linux-kernel - Re: [PATCH v2 06/24] x86: Always inline context_tracking_guest

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 25 Aug 2021 21:10:48 -0700
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Sam Ravnborg <sam@...nborg.org>
Cc:     Peter Zijlstra <peterz@...radead.org>, tglx@...utronix.de,
        linux-kernel@...r.kernel.org, joro@...tes.org,
        boris.ostrovsky@...cle.com, jgross@...e.com, x86@...nel.org,
        mbenes@...e.com, rostedt@...dmis.org, dvyukov@...gle.com,
        elver@...gle.com, masahiroy@...nel.org, hca@...ux.ibm.com,
        nickhu@...estech.com, vgupta@...nel.org
Subject: Re: [PATCH v2 06/24] x86: Always inline
 context_tracking_guest_enter()

On Wed, Aug 25, 2021 at 06:37:08PM +0200, Sam Ravnborg wrote:
> Hi Josh,
> 
> On Wed, Aug 25, 2021 at 02:16:01PM +0200, Peter Zijlstra wrote:
> > On Fri, Aug 20, 2021 at 04:34:40PM -0700, Josh Poimboeuf wrote:
> > 
> > > WARNING: This patch may shrink your patch set and improve your sanity.
> > 
> > I'll go rebuild me some kernels to see how many __always_inline things I
> > can drop.
> > 
> > Lets add a few Cc's though.
> > 
> > > From: Josh Poimboeuf <jpoimboe@...hat.com>
> > > Subject: [PATCH] kbuild: Remove CONFIG_DEBUG_SECTION_MISMATCH
> > > 
> > > So the option isn't actually useful.  It's not even useful for finding
> > > issues on other configs, which might have different inlining decisions,
> > > because with -O2, once-called static functions are always inlined.
> We have following -O possibilities:
> 
>     ifdef CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE
>     KBUILD_CFLAGS += -O2
>     else ifdef CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3
>     KBUILD_CFLAGS += -O3
>     else ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
>     KBUILD_CFLAGS += -Os
>     endif
> 
> We always built with -Os or -O2 (and ARC supports -O3).
> Do we know that gcc and clang will always inline once-called functions
> with -O2 and -Os for the relevant architectures?

Hm, I should have read the GCC manual a little closer:

  -finline-functions-called-once
      Consider all "static" functions called once for inlining into their
      caller even if they are not marked "inline".

So it's not necessarily a guarantee that once-called functions will
always be inlined.  Looking at GCC code, there at least some theoretical
edge cases involving optimization mismatches between caller and callee,
though as a rule the kernel avoids function-specific optimizations.

Still, I think the patch is a net benefit.  I'll update the patch
description and post it again separately since Peter doesn't seem to
need it here.

> If you can answer yes to this with some confidence then you can add my:
> Acked-by: Sam Ravnborg <sam@...nborg.org>
> 
> Note: I added this option long time ago and is unhappy if this only
> have resulted in useless code changes.

It might have been useful in the days before we had all the automated
build bots doing randconfigs.  Nowadays any __init mismatch issues which
would only be seen in obscure config/compiler combinations will be
rooted out by the bots reporting modpost section mismatch warnings.
 
-- 
Josh