On Tue, Nov 22, 2016 at 08:35:48PM -0500, Nicolas Pitre wrote: > On Wed, 23 Nov 2016, Nicholas Piggin wrote: > > > On Tue, 22 Nov 2016 11:34:48 -0500 (EST) > > Nicolas Pitre <nicolas.pi...@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 <u...@kleine-koenig.org> > > > > Fixes: 4dd1837d7589 ("arm: move exports to definitions") > > > > Signed-off-by: Arnd Bergmann <a...@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. > > Hmmm. That's right. That makes it much more justifiable. > My main objection is withdrawn.
I don't see it makes any difference - the armksyms.c originally had the same: -#include <linux/export.h> -#include <linux/sched.h> -#include <linux/string.h> -#include <linux/cryptohash.h> -#include <linux/delay.h> -#include <linux/in6.h> -#include <linux/syscalls.h> -#include <linux/uaccess.h> -#include <linux/io.h> -#include <linux/arm-smccc.h> - -#include <asm/checksum.h> -#include <asm/ftrace.h> followed by prototypes for the GCC internal functions, and: -extern void fpundefinstr(void); - -void mmioset(void *, unsigned int, size_t); -void mmiocpy(void *, const void *, size_t); So, the asm-prototypes.h approach is just the same, only that we now have a bunch of prototypes in a header file, and the EXPORT_SYMBOL()s in the assembly files. As the C prototypes are remote from the definitions, it means that the C prototypes are going to get forgotten about in exactly the same way that armksyms.c would've been forgotten about too. It _is_ worse than that though - with the armksyms.c approach, if the assembly code for it is removed, you get a build error reminding you to remove the export (and prototype). With this approach, you get no reminder to touch asm-prototypes.h. It's also error prone for another reason - adding a new assembly level export, if you forget to add it to asm-prototypes.h, we're back into the problem we have right now with MODVERSIONS breaking. So, I still think the whole approach is wrong - it's added extra fragility that wasn't there with the armksyms.c approach. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.