linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: npiggin@gmail.com (Nicholas Piggin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM
Date: Wed, 23 Nov 2016 12:04:36 +1100	[thread overview]
Message-ID: <20161123120436.18dc21e7@roar.ozlabs.ibm.com> (raw)
In-Reply-To: <alpine.LFD.2.20.1611221106410.1814@knanqh.ubzr>

On Tue, 22 Nov 2016 11:34:48 -0500 (EST)
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> On Tue, 22 Nov 2016, Arnd Bergmann wrote:
> 
> > This adds an asm/asm-prototypes.h header for ARM to fix the broken symbol
> > versioning for symbols exported from assembler files.
> > 
> > I couldn't find the correct prototypes for the compiler builtins,
> > so I went with the fake 'void f(void)' prototypes that we had
> > before, restoring the state before they were moved.
> > 
> > Originally I assumed that the problem was just a harmless warning
> > in unusual configurations, but as Uwe found, we actually need this
> > to load most modules when symbol versioning is enabled, as it is
> > in many distro kernels.
> > 
> > Cc: Uwe Kleine-K?nig <uwe@kleine-koenig.org>
> > Fixes: 4dd1837d7589 ("arm: move exports to definitions")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > Compared to the earlier version, I dropped the changes to the
> > csumpartial files, which now get handled correctly by Kbuild
> > even when the export comes from a macro, and I also dropped the
> > changes to the bitops files, which were already fixed in a
> > patch from Nico.
> > 
> > The patch applies cleanly on top of the rmk/fixes tree but has
> > no effect there, as it also needs 4efca4ed05cb ("kbuild: modversions
> > for EXPORT_SYMBOL() for asm") and cc6acc11cad1 ("kbuild: be more
> > careful about matching preprocessed asm ___EXPORT_SYMBOL").
> > 
> > With the combination of rmk/fixes, torvalds/master and these two
> > patches, symbol versioning works again on ARM. As it is still
> > broken on almost all other architectures (powerpc is fixed,
> > x86 has a patch), I wonder if we should make CONFIG_MODVERSIONS
> > as broken for everything else.  
> 
> I'm not sure I like this at all.
> 
> The goal for moving EXPORT_SYMBOL() to assembly code where symbols were 
> defined is to make things close together and avoid those centralized 
> list of symbols that you can easily miss when modifying the actual code.

Right.

> 
> This series is therefore bringing back a centralized list of symbols in 
> a slightly different form, nullifying the advantages from having moved 
> EXPORT_SYMBOL() to asm code.  To me this looks like a big step backward.

Exported symbols have C declarations in headers already. For the most
part, anyway -- these ones Arnd adds are for compiler runtime which is
why some architectures haven't had the prototypes.

> Why not simply extending the original idea of keeping exports close to 
> the actual code by _also_ having a macro that provides the function 
> prototype alongside the EXPORT_SYMBOL() instance?  That could even be 
> expressed with some EXPORT_SYMBOL_PROTO(ret, sym, arg...) macro that 
> does it all.

Well, the reason is to get 4.9 working, I never thought asm-prototypes.h
was a beautiful solution or it should not be changed if we can find ways
to improve it.

EXPORT_SYMBOL_PROTO() for asm code seems possibly a good idea for 4.10.
Of course your exported symbols will still have their prototypes in
various headers -- that redundancy is inherent.

Thanks,
Nick

  parent reply	other threads:[~2016-11-23  1:04 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-22 11:05 [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann
2016-11-22 11:05 ` [PATCH 2/2] ARM: move mmiocpy/mmioset exports to io.c Arnd Bergmann
2016-11-22 16:34 ` [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM Nicolas Pitre
2016-11-23  0:41   ` Russell King - ARM Linux
2016-11-23  1:40     ` Nicholas Piggin
2016-11-23  1:04   ` Nicholas Piggin [this message]
2016-11-23  1:35     ` Nicolas Pitre
2016-11-23  9:33       ` Russell King - ARM Linux
2016-11-23 10:36         ` Russell King - ARM Linux
2016-11-27  2:33           ` Nicolas Pitre

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=20161123120436.18dc21e7@roar.ozlabs.ibm.com \
    --to=npiggin@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.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).