[1/2] kbuild: provide include/asm/asm-prototypes.h for ARM

On Wed, Nov 23, 2016 at 09:33:32AM +0000, Russell King - ARM Linux wrote:
> 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.

Here's what the diffstat and patch looks like when you combine the
original commit and the three fixes.  The LoC delta of 25 lines can
be accounted for as deleted commentry.  So, I think (as I've detailed
above) that the _technical_ benefit of the approach is very low.

If we want to move the exports into assembly files, I've no problem
with that, provided we can do it better than this - and by better I
mean not creating the fragile asm-prototypes.h which divorses the
prototypes from everything else.

Looking at _this_ patch, there's also an issue with the __raw_*
functions which are conditionally exported, which isn't taken account
of in these fixes.  We currently build most of the io-*.S files whether
we use them or not, and rely on the linker's archive processing, along
with the ifdefs in armksyms to omit them from the kernel image.  This
won't happen with this new approach.

So, I'm reverting the original commit today, because there's no clear
benefit, it's fragile, and I think there's still a few corner cases
that need to be fixed.  We can revisit when we have more time to
properly review and test these changes, rather than at what is now the
11th hour.

 arch/arm/include/asm/Kbuild           |   1 +
 arch/arm/include/asm/asm-prototypes.h |  34 +++++++
 arch/arm/kernel/Makefile              |   2 +-
 arch/arm/kernel/armksyms.c            | 183 ----------------------------------
 arch/arm/kernel/entry-ftrace.S        |   3 +
 arch/arm/kernel/head.S                |   3 +
 arch/arm/kernel/io.c                  |   7 ++
 arch/arm/kernel/smccc-call.S          |   3 +
 arch/arm/lib/ashldi3.S                |   3 +
 arch/arm/lib/ashrdi3.S                |   3 +
 arch/arm/lib/bswapsdi2.S              |   3 +
 arch/arm/lib/changebit.S              |   3 +
 arch/arm/lib/clear_user.S             |   4 +
 arch/arm/lib/clearbit.S               |   3 +
 arch/arm/lib/copy_from_user.S         |   2 +
 arch/arm/lib/copy_page.S              |   2 +
 arch/arm/lib/copy_to_user.S           |   4 +
 arch/arm/lib/csumipv6.S               |   3 +-
 arch/arm/lib/csumpartial.S            |   2 +
 arch/arm/lib/csumpartialcopy.S        |   1 +
 arch/arm/lib/csumpartialcopygeneric.S |   2 +
 arch/arm/lib/csumpartialcopyuser.S    |   1 +
 arch/arm/lib/delay.c                  |   2 +
 arch/arm/lib/div64.S                  |   2 +
 arch/arm/lib/findbit.S                |   9 ++
 arch/arm/lib/getuser.S                |   9 ++
 arch/arm/lib/io-readsb.S              |   2 +
 arch/arm/lib/io-readsl.S              |   2 +
 arch/arm/lib/io-readsw-armv3.S        |   3 +-
 arch/arm/lib/io-readsw-armv4.S        |   2 +
 arch/arm/lib/io-writesb.S             |   2 +
 arch/arm/lib/io-writesl.S             |   2 +
 arch/arm/lib/io-writesw-armv3.S       |   2 +
 arch/arm/lib/io-writesw-armv4.S       |   2 +
 arch/arm/lib/lib1funcs.S              |   9 ++
 arch/arm/lib/lshrdi3.S                |   3 +
 arch/arm/lib/memchr.S                 |   2 +
 arch/arm/lib/memcpy.S                 |   2 +
 arch/arm/lib/memmove.S                |   2 +
 arch/arm/lib/memset.S                 |   2 +
 arch/arm/lib/memzero.S                |   2 +
 arch/arm/lib/muldi3.S                 |   3 +
 arch/arm/lib/putuser.S                |   5 +
 arch/arm/lib/setbit.S                 |   3 +
 arch/arm/lib/strchr.S                 |   2 +
 arch/arm/lib/strrchr.S                |   2 +
 arch/arm/lib/testchangebit.S          |   3 +
 arch/arm/lib/testclearbit.S           |   3 +
 arch/arm/lib/testsetbit.S             |   3 +
 arch/arm/lib/uaccess_with_memcpy.c    |   3 +
 arch/arm/lib/ucmpdi2.S                |   3 +
 arch/arm/mach-imx/Makefile            |   1 -
 arch/arm/mach-imx/ssi-fiq-ksym.c      |  20 ----
 arch/arm/mach-imx/ssi-fiq.S           |   7 +-
 54 files changed, 183 insertions(+), 208 deletions(-)

[1/2] kbuild: provide include/asm/asm-prototypes.h for ARM

Commit Message

Comments

Patch