linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jessica Yu <jeyu@kernel.org>,
	Nick Desaulniers <ndesaulniers@gooogle.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	David Miller <davem@davemloft.net>,
	Jakub Jelinek <jakub@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Steven Rostedt <rostedt@goodmis.org>,
	Daniel Kurtz <djkurtz@chromium.org>,
	linux-arch@vger.kernel.org, linux-m68k@lists.linux-m68k.org,
	Johan Hovold <johan@kernel.org>
Subject: [PATCH 0/8] linker-section array fix and clean ups
Date: Tue,  3 Nov 2020 18:57:03 +0100	[thread overview]
Message-ID: <20201103175711.10731-1-johan@kernel.org> (raw)

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.

This series reverts the latter two workarounds in favour of the one-line
fix of aligning the entries according to the requirement of the type.

In principle, there shouldn't be anything preventing us from doing the
same for tracepoints.

The key observation here is that the arrays should be constructed using
the alignment of the type in question (as given by __alignof__()) rather
than some specific alignment such as sizeof(void *). This allows the
structures to be stored efficiently, but more importantly prevents
breakage on architectures like m68k where pointers are 2-byte aligned
should the size or alignment of the type change (e.g. so that the size
is no longer divisible by four).

As a preventive measure in case the kernel-parameter structures are ever
amended (or the code pattern is reused elsewhere), the final patches
switches the parameter declarations to also use type alignment.

The series has been tested using gcc 4.9 and 9.3 on x86_32 and
x86_64 and using gcc 7.2 on arm; and has been compile-tested and
verified using gcc 4.9 and 10.1 on aarch64, sparc and m68k.

Note that the patches are mostly independent and can be merged through
different subsystem trees. I decided to post them as a series to provide
a common background and have a single thread for any general discussion.

Johan

[1] https://github.com/gcc-mirror/gcc/blob/master/gcc/varasm.c
[2] https://lore.kernel.org/lkml/20110126222622.GA10794@Krystal/ 
[3] https://lore.kernel.org/lkml/1297123347-2170-1-git-send-email-dtor@vmware.com/


Johan Hovold (8):
  of: fix linker-section match-table corruption
  earlycon: simplify earlycon-table implementation
  module: drop version-attribute alignment
  module: simplify version-attribute handling
  init: use type alignment for kernel parameters
  params: drop redundant "unused" attributes
  params: use type alignment for kernel parameters
  params: clean up module-param macros

 drivers/of/fdt.c              |  7 ++-----
 drivers/tty/serial/earlycon.c |  6 ++----
 include/linux/init.h          |  2 +-
 include/linux/module.h        | 28 ++++++++++++++--------------
 include/linux/moduleparam.h   | 12 ++++++------
 include/linux/of.h            |  1 +
 include/linux/serial_core.h   | 20 +++++++-------------
 kernel/params.c               | 10 ++++------
 8 files changed, 37 insertions(+), 49 deletions(-)

-- 
2.26.2


             reply	other threads:[~2020-11-03 18:12 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-03 17:57 Johan Hovold [this message]
2020-11-03 17:57 ` [PATCH 1/8] of: fix linker-section match-table corruption Johan Hovold
2020-11-03 17:57 ` [PATCH 2/8] earlycon: simplify earlycon-table implementation Johan Hovold
2020-11-03 17:57 ` [PATCH 3/8] module: drop version-attribute alignment Johan Hovold
2020-11-03 17:57 ` [PATCH 4/8] module: simplify version-attribute handling Johan Hovold
2020-11-03 17:57 ` [PATCH 5/8] init: use type alignment for kernel parameters Johan Hovold
2020-11-03 17:57 ` [PATCH 6/8] params: drop redundant "unused" attributes Johan Hovold
2020-11-03 17:57 ` [PATCH 7/8] params: use type alignment for kernel parameters Johan Hovold
2020-11-03 17:57 ` [PATCH 8/8] params: clean up module-param macros Johan Hovold
2020-11-04  9:16 ` get_maintainer.pl bug? (was: Re: [PATCH 0/8] linker-section array fix and clean ups) Johan Hovold
2020-11-04 12:04   ` Joe Perches
2020-11-04 15:31     ` Johan Hovold
2020-11-06 16:03 ` [PATCH 0/8] linker-section array fix and clean ups Jessica Yu
2020-11-06 16:45   ` Johan Hovold
2020-11-06 16:55     ` Steven Rostedt
2020-11-06 17:02       ` Johan Hovold
2020-11-11 15:47     ` Jessica Yu
2020-11-13 14:18       ` Johan Hovold
2020-11-23 10:39         ` Johan Hovold
2020-11-25 14:51           ` Jessica Yu
2020-11-27  9:59             ` Johan Hovold
2020-12-01  9:55               ` Jessica Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201103175711.10731-1-johan@kernel.org \
    --to=johan@kernel.org \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=djkurtz@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=frowand.list@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jakub@redhat.com \
    --cc=jeyu@kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=ndesaulniers@gooogle.com \
    --cc=peterz@infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).