+++ Johan Hovold [06/11/20 17:45 +0100]:
On Fri, Nov 06, 2020 at 05:03:45PM +0100, Jessica Yu wrote:
+++ Johan Hovold [03/11/20 18:57 +0100]:
>We rely on the linker to create arrays for a number of things including
>kernel parameters and device-tree-match entries.
>
>The stride of these linker-section arrays obviously needs to match the
>expectations of the code accessing them or bad things will happen.
>
>One thing to watch out for is that gcc is known to increase the
>alignment of larger objects with static extent as an optimisation (on
>x86), but this can be suppressed by using the aligned attribute when
>declaring entries.
>
>We've been relying on this behaviour for 16 years for kernel parameters
>(and other structures) and it indeed hasn't changed since the
>introduction of the aligned attribute in gcc 3.1 (see align_variable()
>in [1]).
>
>Occasionally this gcc optimisation do cause problems which have instead
>been worked around in various creative ways including using indirection
>through an array of pointers. This was originally done for tracepoints
>[2] after a number of failed attempts to create properly aligned arrays,
>and the approach was later reused for module-version attributes [3] and
>earlycon entries.
>[2] https://lore.kernel.org/lkml/20110126222622.GA10794@Krystal/
So unfortunately, I am not familiar enough with the semantics of gcc's
aligned attribute. AFAICT from the patch you linked in [2], the
original purpose of the pointer indirection workaround was to avoid
relying on (potentially inconsistent) compiler-specific behavior with
respect to the aligned attribute. The main concern was potential
up-alignment being done by gcc (or the linker) despite the desired
alignment being specified. Indeed, the gcc documentation also states
that the aligned attribute only specifies the *minimum* alignment,
although there's no guarantee that up-alignment wouldn't occur.
So I guess my question is, is there some implicit guarantee that
specifying alignment by type via __alignof__ that's supposed to
prevent gcc from up-aligning? Or are we just assuming that gcc won't
increase the alignment? The gcc docs don't seem to clarify this
unfortunately.
It's simply specifying alignment when declaring the variable that
prevents this optimisation. The relevant code is in the function
align_variable() in [1] where DATA_ALIGNMENT() is never called in case
an alignment has been specified (!DECL_USER_ALIGN(decl)).
There's no mention in the documentation of this that I'm aware of, but
this is the way the aligned attribute has worked since its introduction
judging from the commit history.
As mentioned above, we've been relying on this for kernel parameters and
other structures since 2003-2004 so if it ever were to change we'd find
out soon enough.
It's about to be used for scheduler classes as well. [2]
Johan
[1] https://github.com/gcc-mirror/gcc/blob/master/gcc/varasm.c
[2] https://lore.kernel.org/r/160396870486.397.377616182428528939.tip-bot2@tip-bot2
Thanks for providing the links and references. Your explanation and
this reply from Jakub [1] clarified things for me. I was not aware of
the distinction gcc made between aligned attributes on types vs. on
variables. So from what I understand now, gcc suppresses the
optimization when the alignment is specified in the variable
declaration, but not necessarily when the aligned attribute is just on
the type.
Even though it's been in use for a long time, I think it would be
really helpful if this gcc quirk was explained just a bit more in the
patch changelogs, especially since this is undocumented behavior.
I found the explanation in [1] (as well as in your cover letter) to be
sufficient. Maybe something like "GCC suppresses any optimizations
increasing alignment when the alignment is specified in the variable
declaration, as opposed to just on the type definition. Therefore,
explicitly specify type alignment when declaring entries to prevent
gcc from increasing alignment."
In any case, I can take the module and moduleparam.h patches through
my tree, but I will wait a few days in case there are any objections.
Thanks,
Jessica
[1] https://lore.kernel.org/lkml/20201021131806.GA2176@tucnak/
