* [PATCH] crypto: aegis: fix badly optimized clang output
@ 2019-07-18 13:50 Arnd Bergmann
2019-07-18 16:18 ` Joe Perches
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Arnd Bergmann @ 2019-07-18 13:50 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: Arnd Bergmann, Ondrej Mosnacek, Ard Biesheuvel, linux-crypto,
linux-kernel, clang-built-linux
Clang sometimes makes very different inlining decisions from gcc.
In case of the aegis crypto algorithms, it decides to turn the innermost
primitives (and, xor, ...) into separate functions but inline most of
the rest.
This results in a huge amount of variables spilled on the stack, leading
to rather slow execution as well as kernel stack usage beyond the 32-bit
warning limit when CONFIG_KASAN is enabled:
crypto/aegis256.c:123:13: warning: stack frame size of 648 bytes in function 'crypto_aegis256_encrypt_chunk' [-Wframe-larger-than=]
crypto/aegis256.c:366:13: warning: stack frame size of 1264 bytes in function 'crypto_aegis256_crypt' [-Wframe-larger-than=]
crypto/aegis256.c:187:13: warning: stack frame size of 656 bytes in function 'crypto_aegis256_decrypt_chunk' [-Wframe-larger-than=]
crypto/aegis128l.c:135:13: warning: stack frame size of 832 bytes in function 'crypto_aegis128l_encrypt_chunk' [-Wframe-larger-than=]
crypto/aegis128l.c:415:13: warning: stack frame size of 1480 bytes in function 'crypto_aegis128l_crypt' [-Wframe-larger-than=]
crypto/aegis128l.c:218:13: warning: stack frame size of 848 bytes in function 'crypto_aegis128l_decrypt_chunk' [-Wframe-larger-than=]
crypto/aegis128.c:116:13: warning: stack frame size of 584 bytes in function 'crypto_aegis128_encrypt_chunk' [-Wframe-larger-than=]
crypto/aegis128.c:351:13: warning: stack frame size of 1064 bytes in function 'crypto_aegis128_crypt' [-Wframe-larger-than=]
crypto/aegis128.c:177:13: warning: stack frame size of 592 bytes in function 'crypto_aegis128_decrypt_chunk' [-Wframe-larger-than=]
Forcing the primitives to all get inlined avoids the issue and the
resulting code is similar to what gcc produces.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
crypto/aegis.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/crypto/aegis.h b/crypto/aegis.h
index 41a3090cda8e..efed7251c49d 100644
--- a/crypto/aegis.h
+++ b/crypto/aegis.h
@@ -34,21 +34,21 @@ static const union aegis_block crypto_aegis_const[2] = {
} },
};
-static void crypto_aegis_block_xor(union aegis_block *dst,
+static __always_inline void crypto_aegis_block_xor(union aegis_block *dst,
const union aegis_block *src)
{
dst->words64[0] ^= src->words64[0];
dst->words64[1] ^= src->words64[1];
}
-static void crypto_aegis_block_and(union aegis_block *dst,
+static __always_inline void crypto_aegis_block_and(union aegis_block *dst,
const union aegis_block *src)
{
dst->words64[0] &= src->words64[0];
dst->words64[1] &= src->words64[1];
}
-static void crypto_aegis_aesenc(union aegis_block *dst,
+static __always_inline void crypto_aegis_aesenc(union aegis_block *dst,
const union aegis_block *src,
const union aegis_block *key)
{
--
2.20.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] crypto: aegis: fix badly optimized clang output
2019-07-18 13:50 [PATCH] crypto: aegis: fix badly optimized clang output Arnd Bergmann
@ 2019-07-18 16:18 ` Joe Perches
2019-07-18 21:17 ` Nick Desaulniers
2019-07-26 12:35 ` Herbert Xu
2 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2019-07-18 16:18 UTC (permalink / raw)
To: Arnd Bergmann, Herbert Xu, David S. Miller
Cc: Ondrej Mosnacek, Ard Biesheuvel, linux-crypto, linux-kernel,
clang-built-linux
On Thu, 2019-07-18 at 15:50 +0200, Arnd Bergmann wrote:
> Clang sometimes makes very different inlining decisions from gcc.
> In case of the aegis crypto algorithms, it decides to turn the innermost
> primitives (and, xor, ...) into separate functions but inline most of
> the rest.
> This results in a huge amount of variables spilled on the stack, leading
> to rather slow execution as well as kernel stack usage beyond the 32-bit
> warning limit when CONFIG_KASAN is enabled:
>
> crypto/aegis256.c:123:13: warning: stack frame size of 648 bytes in function 'crypto_aegis256_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis256.c:366:13: warning: stack frame size of 1264 bytes in function 'crypto_aegis256_crypt' [-Wframe-larger-than=]
> crypto/aegis256.c:187:13: warning: stack frame size of 656 bytes in function 'crypto_aegis256_decrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128l.c:135:13: warning: stack frame size of 832 bytes in function 'crypto_aegis128l_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128l.c:415:13: warning: stack frame size of 1480 bytes in function 'crypto_aegis128l_crypt' [-Wframe-larger-than=]
> crypto/aegis128l.c:218:13: warning: stack frame size of 848 bytes in function 'crypto_aegis128l_decrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128.c:116:13: warning: stack frame size of 584 bytes in function 'crypto_aegis128_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128.c:351:13: warning: stack frame size of 1064 bytes in function 'crypto_aegis128_crypt' [-Wframe-larger-than=]
> crypto/aegis128.c:177:13: warning: stack frame size of 592 bytes in function 'crypto_aegis128_decrypt_chunk' [-Wframe-larger-than=]
>
> Forcing the primitives to all get inlined avoids the issue and the
> resulting code is similar to what gcc produces.
Why weren't these functions in .h files
not always marked with inline?
Are there other static non-inlined function
definitions in .h files that should also get
this inline/__always_inline marking?
I presume there are but can't think of a
reasonable way to find them off the top of
my head.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> crypto/aegis.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/aegis.h b/crypto/aegis.h
> index 41a3090cda8e..efed7251c49d 100644
> --- a/crypto/aegis.h
> +++ b/crypto/aegis.h
> @@ -34,21 +34,21 @@ static const union aegis_block crypto_aegis_const[2] = {
> } },
> };
>
> -static void crypto_aegis_block_xor(union aegis_block *dst,
> +static __always_inline void crypto_aegis_block_xor(union aegis_block *dst,
> const union aegis_block *src)
> {
> dst->words64[0] ^= src->words64[0];
> dst->words64[1] ^= src->words64[1];
> }
>
> -static void crypto_aegis_block_and(union aegis_block *dst,
> +static __always_inline void crypto_aegis_block_and(union aegis_block *dst,
> const union aegis_block *src)
> {
> dst->words64[0] &= src->words64[0];
> dst->words64[1] &= src->words64[1];
> }
>
> -static void crypto_aegis_aesenc(union aegis_block *dst,
> +static __always_inline void crypto_aegis_aesenc(union aegis_block *dst,
> const union aegis_block *src,
> const union aegis_block *key)
> {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] crypto: aegis: fix badly optimized clang output
2019-07-18 13:50 [PATCH] crypto: aegis: fix badly optimized clang output Arnd Bergmann
2019-07-18 16:18 ` Joe Perches
@ 2019-07-18 21:17 ` Nick Desaulniers
2019-07-18 21:34 ` Arnd Bergmann
2019-07-26 12:35 ` Herbert Xu
2 siblings, 1 reply; 5+ messages in thread
From: Nick Desaulniers @ 2019-07-18 21:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Herbert Xu, David S. Miller, Ondrej Mosnacek, Ard Biesheuvel,
linux-crypto, LKML, clang-built-linux
On Thu, Jul 18, 2019 at 6:50 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Clang sometimes makes very different inlining decisions from gcc.
> In case of the aegis crypto algorithms, it decides to turn the innermost
> primitives (and, xor, ...) into separate functions but inline most of
> the rest.
>
> This results in a huge amount of variables spilled on the stack, leading
> to rather slow execution as well as kernel stack usage beyond the 32-bit
> warning limit when CONFIG_KASAN is enabled:
>
> crypto/aegis256.c:123:13: warning: stack frame size of 648 bytes in function 'crypto_aegis256_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis256.c:366:13: warning: stack frame size of 1264 bytes in function 'crypto_aegis256_crypt' [-Wframe-larger-than=]
> crypto/aegis256.c:187:13: warning: stack frame size of 656 bytes in function 'crypto_aegis256_decrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128l.c:135:13: warning: stack frame size of 832 bytes in function 'crypto_aegis128l_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128l.c:415:13: warning: stack frame size of 1480 bytes in function 'crypto_aegis128l_crypt' [-Wframe-larger-than=]
> crypto/aegis128l.c:218:13: warning: stack frame size of 848 bytes in function 'crypto_aegis128l_decrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128.c:116:13: warning: stack frame size of 584 bytes in function 'crypto_aegis128_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128.c:351:13: warning: stack frame size of 1064 bytes in function 'crypto_aegis128_crypt' [-Wframe-larger-than=]
> crypto/aegis128.c:177:13: warning: stack frame size of 592 bytes in function 'crypto_aegis128_decrypt_chunk' [-Wframe-larger-than=]
>
> Forcing the primitives to all get inlined avoids the issue and the
> resulting code is similar to what gcc produces.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> crypto/aegis.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/aegis.h b/crypto/aegis.h
> index 41a3090cda8e..efed7251c49d 100644
> --- a/crypto/aegis.h
> +++ b/crypto/aegis.h
> @@ -34,21 +34,21 @@ static const union aegis_block crypto_aegis_const[2] = {
> } },
> };
>
> -static void crypto_aegis_block_xor(union aegis_block *dst,
> +static __always_inline void crypto_aegis_block_xor(union aegis_block *dst,
`static inline` would be more concise and expand to the same
attribute, IIRC. Not sure if that's worth sending a v2. But for now,
thanks for the patch and:
Acked-by: Nick Desaulniers <ndesaulniers@google.com>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] crypto: aegis: fix badly optimized clang output
2019-07-18 21:17 ` Nick Desaulniers
@ 2019-07-18 21:34 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2019-07-18 21:34 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Herbert Xu, David S. Miller, Ondrej Mosnacek, Ard Biesheuvel,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, LKML,
clang-built-linux
On Thu, Jul 18, 2019 at 11:17 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
> On Thu, Jul 18, 2019 at 6:50 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > diff --git a/crypto/aegis.h b/crypto/aegis.h
> > index 41a3090cda8e..efed7251c49d 100644
> > --- a/crypto/aegis.h
> > +++ b/crypto/aegis.h
> > @@ -34,21 +34,21 @@ static const union aegis_block crypto_aegis_const[2] = {
> > } },
> > };
> >
> > -static void crypto_aegis_block_xor(union aegis_block *dst,
> > +static __always_inline void crypto_aegis_block_xor(union aegis_block *dst,
>
> `static inline` would be more concise and expand to the same
> attribute, IIRC. Not sure if that's worth sending a v2. But for now,
I think I tried that first but it had no effect when CONFIG_OPTIMIZE_INLINING
is set, as the compiler can sometimes ignores a plain 'inline'. The version
I posted was needed to pass the randconfig tests.
> Acked-by: Nick Desaulniers <ndesaulniers@google.com>
>
Thanks,
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto: aegis: fix badly optimized clang output
2019-07-18 13:50 [PATCH] crypto: aegis: fix badly optimized clang output Arnd Bergmann
2019-07-18 16:18 ` Joe Perches
2019-07-18 21:17 ` Nick Desaulniers
@ 2019-07-26 12:35 ` Herbert Xu
2 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2019-07-26 12:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: davem, arnd, omosnacek, ard.biesheuvel, linux-crypto,
linux-kernel, clang-built-linux
Arnd Bergmann <arnd@arndb.de> wrote:
> Clang sometimes makes very different inlining decisions from gcc.
> In case of the aegis crypto algorithms, it decides to turn the innermost
> primitives (and, xor, ...) into separate functions but inline most of
> the rest.
>
> This results in a huge amount of variables spilled on the stack, leading
> to rather slow execution as well as kernel stack usage beyond the 32-bit
> warning limit when CONFIG_KASAN is enabled:
>
> crypto/aegis256.c:123:13: warning: stack frame size of 648 bytes in function 'crypto_aegis256_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis256.c:366:13: warning: stack frame size of 1264 bytes in function 'crypto_aegis256_crypt' [-Wframe-larger-than=]
> crypto/aegis256.c:187:13: warning: stack frame size of 656 bytes in function 'crypto_aegis256_decrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128l.c:135:13: warning: stack frame size of 832 bytes in function 'crypto_aegis128l_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128l.c:415:13: warning: stack frame size of 1480 bytes in function 'crypto_aegis128l_crypt' [-Wframe-larger-than=]
> crypto/aegis128l.c:218:13: warning: stack frame size of 848 bytes in function 'crypto_aegis128l_decrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128.c:116:13: warning: stack frame size of 584 bytes in function 'crypto_aegis128_encrypt_chunk' [-Wframe-larger-than=]
> crypto/aegis128.c:351:13: warning: stack frame size of 1064 bytes in function 'crypto_aegis128_crypt' [-Wframe-larger-than=]
> crypto/aegis128.c:177:13: warning: stack frame size of 592 bytes in function 'crypto_aegis128_decrypt_chunk' [-Wframe-larger-than=]
>
> Forcing the primitives to all get inlined avoids the issue and the
> resulting code is similar to what gcc produces.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> crypto/aegis.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-26 12:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-18 13:50 [PATCH] crypto: aegis: fix badly optimized clang output Arnd Bergmann
2019-07-18 16:18 ` Joe Perches
2019-07-18 21:17 ` Nick Desaulniers
2019-07-18 21:34 ` Arnd Bergmann
2019-07-26 12:35 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).