* Re: [GIT PULL] kbuild changes for v4.9-rc1
@ 2016-10-17 6:51 Adam Borowski
2016-10-17 6:59 ` Nicholas Piggin
0 siblings, 1 reply; 58+ messages in thread
From: Adam Borowski @ 2016-10-17 6:51 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr,
viro, linux-kbuild, linux-kernel
On Mon, Oct 17, 2016 at 02:57:09PM +1100, Nicholas Piggin wrote:
> On Sat, 15 Oct 2016 17:22:05 -0700 Omar Sandoval <osandov@osandov.com> wrote:
> > On Fri, Oct 14, 2016 at 10:12:46PM +0200, Michal Marek wrote:
> > > please pull these kbuild changes for v4.9-rc1:
> > >
> > > - EXPORT_SYMBOL for asm source by Al Viro. This does bring a regression,
> > > because genksyms no longer generates checksums for these symbols
> > > (CONFIG_MODVERSIONS). Nick Piggin is working on a patch to fix this.
> > > Plus, we are talking about functions like strcpy(), which rarely
> > > change prototypes.
> >
> > So this has broken all module loading for me. I get the following dmesg
> > spew:
> > ...
> > [ 4.586914] scsi_mod: no symbol version for memset
> > [ 4.587920] scsi_mod: Unknown symbol memset (err -22)
> > [ 4.588443] scsi_mod: no symbol version for ___preempt_schedule
> > [ 4.589026] scsi_mod: Unknown symbol ___preempt_schedule (err -22)
> > ...
> >
> > Reverting 784d5699eddc ("x86: move exports to actual definitions") fixes
> > it for me. This is with GCC 6.2.1, binutils 2.27, attached config.
>
> Thanks for the report. Could you try this patch and see if it helps?
[patch snipped]
Omar probably won't wake up in quite a while, so I've tested the patch.
Alas, doesn't help. Similar spew (for the few modules I don't have =y),
while reverting 784d5699eddc fixes it for me too.
Debian sid toolchain: gcc 6.2.0-6, binutils 2.27-8, config at
https://angband.pl/tmp/config-4.9.0-rc1-debug+.gz
Meow!
--
A MAP07 (Dead Simple) raspberry tincture recipe: 0.5l 95% alcohol, 1kg
raspberries, 0.4kg sugar; put into a big jar for 1 month. Filter out and
throw away the fruits (can dump them into a cake, etc), let the drink age
at least 3-6 months.
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 6:51 [GIT PULL] kbuild changes for v4.9-rc1 Adam Borowski @ 2016-10-17 6:59 ` Nicholas Piggin 2016-10-17 10:01 ` Adam Borowski 2016-10-17 12:26 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann 0 siblings, 2 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-10-17 6:59 UTC (permalink / raw) To: Adam Borowski Cc: Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Mon, 17 Oct 2016 08:51:31 +0200 Adam Borowski <kilobyte@angband.pl> wrote: > On Mon, Oct 17, 2016 at 02:57:09PM +1100, Nicholas Piggin wrote: > > On Sat, 15 Oct 2016 17:22:05 -0700 Omar Sandoval <osandov@osandov.com> wrote: > > > On Fri, Oct 14, 2016 at 10:12:46PM +0200, Michal Marek wrote: > > > > please pull these kbuild changes for v4.9-rc1: > > > > > > > > - EXPORT_SYMBOL for asm source by Al Viro. This does bring a regression, > > > > because genksyms no longer generates checksums for these symbols > > > > (CONFIG_MODVERSIONS). Nick Piggin is working on a patch to fix this. > > > > Plus, we are talking about functions like strcpy(), which rarely > > > > change prototypes. > > > > > > So this has broken all module loading for me. I get the following dmesg > > > spew: > > > ... > > > [ 4.586914] scsi_mod: no symbol version for memset > > > [ 4.587920] scsi_mod: Unknown symbol memset (err -22) > > > [ 4.588443] scsi_mod: no symbol version for ___preempt_schedule > > > [ 4.589026] scsi_mod: Unknown symbol ___preempt_schedule (err -22) > > > ... > > > > > > Reverting 784d5699eddc ("x86: move exports to actual definitions") fixes > > > it for me. This is with GCC 6.2.1, binutils 2.27, attached config. > > > > Thanks for the report. Could you try this patch and see if it helps? > [patch snipped] > > Omar probably won't wake up in quite a while, so I've tested the patch. > Alas, doesn't help. Similar spew (for the few modules I don't have =y), > while reverting 784d5699eddc fixes it for me too. > > Debian sid toolchain: gcc 6.2.0-6, binutils 2.27-8, config at > https://angband.pl/tmp/config-4.9.0-rc1-debug+.gz Forgot to engage my brain before posting. Architectures will need to have an include/asm/asm-prototypes.h that defines or #include<>s C-style prototypes for exported asm functions. We can do an asm-generic version for the common ones like memset so there's not a lot of pointless duplication there. Care to do a patch for x86? Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 6:59 ` Nicholas Piggin 2016-10-17 10:01 ` Adam Borowski @ 2016-10-17 10:01 ` Adam Borowski 1 sibling, 0 replies; 58+ messages in thread From: Adam Borowski @ 2016-10-17 10:01 UTC (permalink / raw) To: Nicholas Piggin Cc: Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: > On Mon, 17 Oct 2016 08:51:31 +0200 > Adam Borowski <kilobyte@angband.pl> wrote: > > On Mon, Oct 17, 2016 at 02:57:09PM +1100, Nicholas Piggin wrote: > > > On Sat, 15 Oct 2016 17:22:05 -0700 Omar Sandoval <osandov@osandov.com> wrote: > > > > So this has broken all module loading for me. I get the following dmesg > > > > spew: > > > > ... > > > > [ 4.586914] scsi_mod: no symbol version for memset > > > > [ 4.587920] scsi_mod: Unknown symbol memset (err -22) > > > > [ 4.588443] scsi_mod: no symbol version for ___preempt_schedule > > > > [ 4.589026] scsi_mod: Unknown symbol ___preempt_schedule (err -22) > > > > ... > > > > > > > > Reverting 784d5699eddc ("x86: move exports to actual definitions") fixes > > > > it for me. This is with GCC 6.2.1, binutils 2.27, attached config. > > > > > > Thanks for the report. Could you try this patch and see if it helps? > > [patch snipped] > > > > Omar probably won't wake up in quite a while, so I've tested the patch. > > Alas, doesn't help. Similar spew (for the few modules I don't have =y), > > while reverting 784d5699eddc fixes it for me too. > > Forgot to engage my brain before posting. > > Architectures will need to have an include/asm/asm-prototypes.h that > defines or #include<>s C-style prototypes for exported asm functions. > We can do an asm-generic version for the common ones like memset so > there's not a lot of pointless duplication there. > > Care to do a patch for x86? Sure, did so. With the prototypes added, your patch works! Tested on a handful of modules, and one out-of-tree (virtualbox). I didn't try a 32-bit build. Note that powerpc already has an include/asm/asm-prototypes.h which might or might not be what you want. It doesn't have memset and co, for example. Anyway, here's my stab at x86: >From db746df65b920591606398b4b244f5b6dc9eea04 Mon Sep 17 00:00:00 2001 From: Adam Borowski <kilobyte@angband.pl> Date: Mon, 17 Oct 2016 11:42:35 +0200 Subject: [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 Nicholas Piggin wrote: > Architectures will need to have an include/asm/asm-prototypes.h that > defines or #include<>s C-style prototypes for exported asm functions. > We can do an asm-generic version for the common ones like memset so > there's not a lot of pointless duplication there. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- arch/x86/include/asm/asm-prototypes.h | 13 +++++++++++++ include/asm-generic/asm-prototypes.h | 7 +++++++ 2 files changed, 20 insertions(+) create mode 100644 arch/x86/include/asm/asm-prototypes.h create mode 100644 include/asm-generic/asm-prototypes.h diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h new file mode 100644 index 0000000..072c97c --- /dev/null +++ b/arch/x86/include/asm/asm-prototypes.h @@ -0,0 +1,13 @@ +#include <asm/ftrace.h> +#include <asm/uaccess.h> +#include <asm/uaccess.h> +#include <asm/string.h> +#include <asm/page.h> +#include <asm/checksum.h> + +#include <asm-generic/asm-prototypes.h> + +#include <asm/page.h> +#include <asm/pgtable.h> +#include <asm/special_insns.h> +#include <asm/preempt.h> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h new file mode 100644 index 0000000..df13637 --- /dev/null +++ b/include/asm-generic/asm-prototypes.h @@ -0,0 +1,7 @@ +#include <linux/bitops.h> +extern void *__memset(void *, int, __kernel_size_t); +extern void *__memcpy(void *, const void *, __kernel_size_t); +extern void *__memmove(void *, const void *, __kernel_size_t); +extern void *memset(void *, int, __kernel_size_t); +extern void *memcpy(void *, const void *, __kernel_size_t); +extern void *memmove(void *, const void *, __kernel_size_t); -- 2.9.3 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 @ 2016-10-17 10:01 ` Adam Borowski 0 siblings, 0 replies; 58+ messages in thread From: Adam Borowski @ 2016-10-17 10:01 UTC (permalink / raw) To: Nicholas Piggin Cc: Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: > On Mon, 17 Oct 2016 08:51:31 +0200 > Adam Borowski <kilobyte@angband.pl> wrote: > > On Mon, Oct 17, 2016 at 02:57:09PM +1100, Nicholas Piggin wrote: > > > On Sat, 15 Oct 2016 17:22:05 -0700 Omar Sandoval <osandov@osandov.com> wrote: > > > > So this has broken all module loading for me. I get the following dmesg > > > > spew: > > > > ... > > > > [ 4.586914] scsi_mod: no symbol version for memset > > > > [ 4.587920] scsi_mod: Unknown symbol memset (err -22) > > > > [ 4.588443] scsi_mod: no symbol version for ___preempt_schedule > > > > [ 4.589026] scsi_mod: Unknown symbol ___preempt_schedule (err -22) > > > > ... > > > > > > > > Reverting 784d5699eddc ("x86: move exports to actual definitions") fixes > > > > it for me. This is with GCC 6.2.1, binutils 2.27, attached config. > > > > > > Thanks for the report. Could you try this patch and see if it helps? > > [patch snipped] > > > > Omar probably won't wake up in quite a while, so I've tested the patch. > > Alas, doesn't help. Similar spew (for the few modules I don't have =y), > > while reverting 784d5699eddc fixes it for me too. > > Forgot to engage my brain before posting. > > Architectures will need to have an include/asm/asm-prototypes.h that > defines or #include<>s C-style prototypes for exported asm functions. > We can do an asm-generic version for the common ones like memset so > there's not a lot of pointless duplication there. > > Care to do a patch for x86? Sure, did so. With the prototypes added, your patch works! Tested on a handful of modules, and one out-of-tree (virtualbox). I didn't try a 32-bit build. Note that powerpc already has an include/asm/asm-prototypes.h which might or might not be what you want. It doesn't have memset and co, for example. Anyway, here's my stab at x86: ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 @ 2016-10-17 10:01 ` Adam Borowski 0 siblings, 0 replies; 58+ messages in thread From: Adam Borowski @ 2016-10-17 10:01 UTC (permalink / raw) To: Nicholas Piggin Cc: Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: > On Mon, 17 Oct 2016 08:51:31 +0200 > Adam Borowski <kilobyte@angband.pl> wrote: > > On Mon, Oct 17, 2016 at 02:57:09PM +1100, Nicholas Piggin wrote: > > > On Sat, 15 Oct 2016 17:22:05 -0700 Omar Sandoval <osandov@osandov.com> wrote: > > > > So this has broken all module loading for me. I get the following dmesg > > > > spew: > > > > ... > > > > [ 4.586914] scsi_mod: no symbol version for memset > > > > [ 4.587920] scsi_mod: Unknown symbol memset (err -22) > > > > [ 4.588443] scsi_mod: no symbol version for ___preempt_schedule > > > > [ 4.589026] scsi_mod: Unknown symbol ___preempt_schedule (err -22) > > > > ... > > > > > > > > Reverting 784d5699eddc ("x86: move exports to actual definitions") fixes > > > > it for me. This is with GCC 6.2.1, binutils 2.27, attached config. > > > > > > Thanks for the report. Could you try this patch and see if it helps? > > [patch snipped] > > > > Omar probably won't wake up in quite a while, so I've tested the patch. > > Alas, doesn't help. Similar spew (for the few modules I don't have =y), > > while reverting 784d5699eddc fixes it for me too. > > Forgot to engage my brain before posting. > > Architectures will need to have an include/asm/asm-prototypes.h that > defines or #include<>s C-style prototypes for exported asm functions. > We can do an asm-generic version for the common ones like memset so > there's not a lot of pointless duplication there. > > Care to do a patch for x86? Sure, did so. With the prototypes added, your patch works! Tested on a handful of modules, and one out-of-tree (virtualbox). I didn't try a 32-bit build. Note that powerpc already has an include/asm/asm-prototypes.h which might or might not be what you want. It doesn't have memset and co, for example. Anyway, here's my stab at x86: From db746df65b920591606398b4b244f5b6dc9eea04 Mon Sep 17 00:00:00 2001 From: Adam Borowski <kilobyte@angband.pl> Date: Mon, 17 Oct 2016 11:42:35 +0200 Subject: [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 Nicholas Piggin wrote: > Architectures will need to have an include/asm/asm-prototypes.h that > defines or #include<>s C-style prototypes for exported asm functions. > We can do an asm-generic version for the common ones like memset so > there's not a lot of pointless duplication there. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- arch/x86/include/asm/asm-prototypes.h | 13 +++++++++++++ include/asm-generic/asm-prototypes.h | 7 +++++++ 2 files changed, 20 insertions(+) create mode 100644 arch/x86/include/asm/asm-prototypes.h create mode 100644 include/asm-generic/asm-prototypes.h diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h new file mode 100644 index 0000000..072c97c --- /dev/null +++ b/arch/x86/include/asm/asm-prototypes.h @@ -0,0 +1,13 @@ +#include <asm/ftrace.h> +#include <asm/uaccess.h> +#include <asm/uaccess.h> +#include <asm/string.h> +#include <asm/page.h> +#include <asm/checksum.h> + +#include <asm-generic/asm-prototypes.h> + +#include <asm/page.h> +#include <asm/pgtable.h> +#include <asm/special_insns.h> +#include <asm/preempt.h> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h new file mode 100644 index 0000000..df13637 --- /dev/null +++ b/include/asm-generic/asm-prototypes.h @@ -0,0 +1,7 @@ +#include <linux/bitops.h> +extern void *__memset(void *, int, __kernel_size_t); +extern void *__memcpy(void *, const void *, __kernel_size_t); +extern void *__memmove(void *, const void *, __kernel_size_t); +extern void *memset(void *, int, __kernel_size_t); +extern void *memcpy(void *, const void *, __kernel_size_t); +extern void *memmove(void *, const void *, __kernel_size_t); -- 2.9.3 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 10:01 ` Adam Borowski (?) (?) @ 2016-10-17 11:12 ` Alexey Dobriyan 2016-10-17 11:17 ` Geert Uytterhoeven -1 siblings, 1 reply; 58+ messages in thread From: Alexey Dobriyan @ 2016-10-17 11:12 UTC (permalink / raw) To: Adam Borowski Cc: Nicholas Piggin, Omar Sandoval, Michal Marek, Linus Torvalds, Stephen Rothwell, Al Viro, linux-kbuild, Linux Kernel, linux-arch On Mon, Oct 17, 2016 at 1:01 PM, Adam Borowski <kilobyte@angband.pl> wrote: > On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: >> On Mon, 17 Oct 2016 08:51:31 +0200 >> Adam Borowski <kilobyte@angband.pl> wrote: > --- /dev/null > +++ b/include/asm-generic/asm-prototypes.h > @@ -0,0 +1,7 @@ > +#include <linux/bitops.h> > +extern void *__memset(void *, int, __kernel_size_t); > +extern void *__memcpy(void *, const void *, __kernel_size_t); > +extern void *__memmove(void *, const void *, __kernel_size_t); > +extern void *memset(void *, int, __kernel_size_t); > +extern void *memcpy(void *, const void *, __kernel_size_t); > +extern void *memmove(void *, const void *, __kernel_size_t); Before too late, those extern keywords aren't needed and only slowdown compilation. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 11:12 ` Alexey Dobriyan @ 2016-10-17 11:17 ` Geert Uytterhoeven 2016-10-17 11:32 ` Alexey Dobriyan 0 siblings, 1 reply; 58+ messages in thread From: Geert Uytterhoeven @ 2016-10-17 11:17 UTC (permalink / raw) To: Alexey Dobriyan Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Linus Torvalds, Stephen Rothwell, Al Viro, linux-kbuild, Linux Kernel, Linux-Arch Hi Alexey, On Mon, Oct 17, 2016 at 1:12 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > On Mon, Oct 17, 2016 at 1:01 PM, Adam Borowski <kilobyte@angband.pl> wrote: >> On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: >>> On Mon, 17 Oct 2016 08:51:31 +0200 >>> Adam Borowski <kilobyte@angband.pl> wrote: > >> --- /dev/null >> +++ b/include/asm-generic/asm-prototypes.h >> @@ -0,0 +1,7 @@ >> +#include <linux/bitops.h> >> +extern void *__memset(void *, int, __kernel_size_t); >> +extern void *__memcpy(void *, const void *, __kernel_size_t); >> +extern void *__memmove(void *, const void *, __kernel_size_t); >> +extern void *memset(void *, int, __kernel_size_t); >> +extern void *memcpy(void *, const void *, __kernel_size_t); >> +extern void *memmove(void *, const void *, __kernel_size_t); > > Before too late, those extern keywords aren't needed and > only slowdown compilation. Do you have any profiling data backing this? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 11:17 ` Geert Uytterhoeven @ 2016-10-17 11:32 ` Alexey Dobriyan 0 siblings, 0 replies; 58+ messages in thread From: Alexey Dobriyan @ 2016-10-17 11:32 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Linus Torvalds, Stephen Rothwell, Al Viro, linux-kbuild, Linux Kernel, Linux-Arch On Mon, Oct 17, 2016 at 2:17 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Alexey, > > On Mon, Oct 17, 2016 at 1:12 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: >> On Mon, Oct 17, 2016 at 1:01 PM, Adam Borowski <kilobyte@angband.pl> wrote: >>> On Mon, Oct 17, 2016 at 05:59:51PM +1100, Nicholas Piggin wrote: >>>> On Mon, 17 Oct 2016 08:51:31 +0200 >>>> Adam Borowski <kilobyte@angband.pl> wrote: >> >>> --- /dev/null >>> +++ b/include/asm-generic/asm-prototypes.h >>> @@ -0,0 +1,7 @@ >>> +#include <linux/bitops.h> >>> +extern void *__memset(void *, int, __kernel_size_t); >>> +extern void *__memcpy(void *, const void *, __kernel_size_t); >>> +extern void *__memmove(void *, const void *, __kernel_size_t); >>> +extern void *memset(void *, int, __kernel_size_t); >>> +extern void *memcpy(void *, const void *, __kernel_size_t); >>> +extern void *memmove(void *, const void *, __kernel_size_t); >> >> Before too late, those extern keywords aren't needed and >> only slowdown compilation. > > Do you have any profiling data backing this? It is easy to obtain. Here is top 10 with runtime bigger than 0.50%: _int_malloc ht_lookup_with_hash _cpp_lex_direct cpp_get_token_1 ggc_internal_alloc_statm _int_free malloc_consolidate lex_identifier enter_macro_context c_lex_with_flags bitmap_set_bit malloc grokdeclarator htab_find_slot_with_hash c_lex_one_token _cpp_lex_token pop_scopev c_parser_declspecs Imagine you're a compiler. Those extern's are tokens. They need to be parsed (byte-by-byte), allocated, and, in case of "extern" with prototype, thrown away. They make line longer forcing people to dance around with splitting per 80 characters and generally making everything somewhat slower. Might as well don't new ones. bitops.h is wrong header as well. Why do you need bitops for bunch of function prototypes? ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 10:01 ` Adam Borowski ` (2 preceding siblings ...) (?) @ 2016-10-17 12:22 ` Mathieu OTHACEHE 2016-10-18 0:16 ` Adam Borowski -1 siblings, 1 reply; 58+ messages in thread From: Mathieu OTHACEHE @ 2016-10-17 12:22 UTC (permalink / raw) To: Adam Borowski Cc: Nicholas Piggin, Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch > +#include <asm/uaccess.h> > +#include <asm/uaccess.h> Included twice. > +#include <asm/string.h> > +#include <asm/page.h> > +#include <asm/checksum.h> > + > +#include <asm-generic/asm-prototypes.h> > + > +#include <asm/page.h> > +#include <asm/pgtable.h> > +#include <asm/special_insns.h> > +#include <asm/preempt.h> No <asm/arch_hweight.h> for __sw_hweight32 and __sw_hweight64 ? ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 12:22 ` Mathieu OTHACEHE @ 2016-10-18 0:16 ` Adam Borowski 2016-10-18 1:34 ` Nicholas Piggin 0 siblings, 1 reply; 58+ messages in thread From: Adam Borowski @ 2016-10-18 0:16 UTC (permalink / raw) To: Mathieu OTHACEHE Cc: Nicholas Piggin, Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Mon, Oct 17, 2016 at 02:22:34PM +0200, Mathieu OTHACEHE wrote: > > +#include <asm/uaccess.h> > > +#include <asm/uaccess.h> > > Included twice. D'oh! > > +#include <asm/string.h> > > +#include <asm/page.h> > > +#include <asm/checksum.h> > > + > > +#include <asm-generic/asm-prototypes.h> > > + > > +#include <asm/page.h> > > +#include <asm/pgtable.h> > > +#include <asm/special_insns.h> > > +#include <asm/preempt.h> > > No <asm/arch_hweight.h> for __sw_hweight32 and __sw_hweight64 ? diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h new file mode 100644 index 0000000..df13637 --- /dev/null +++ b/include/asm-generic/asm-prototypes.h @@ -0,0 +1,7 @@ +#include <linux/bitops.h> ... which has these. Alexey Dobriyan <adobriyan@gmail.com> wrote: } bitops.h is wrong header as well. } Why do you need bitops for bunch of function prototypes? Unless you guys prefer using low-level headers only, that is. Meow! -- A MAP07 (Dead Simple) raspberry tincture recipe: 0.5l 95% alcohol, 1kg raspberries, 0.4kg sugar; put into a big jar for 1 month. Filter out and throw away the fruits (can dump them into a cake, etc), let the drink age at least 3-6 months. ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-18 0:16 ` Adam Borowski @ 2016-10-18 1:34 ` Nicholas Piggin 2016-10-19 14:38 ` Michal Marek 2016-11-01 15:48 ` Michal Marek 0 siblings, 2 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-10-18 1:34 UTC (permalink / raw) To: Adam Borowski Cc: Mathieu OTHACEHE, Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch Hi Adam, Thanks, this is looking good. powerpc will be able to use the generic header. On Tue, 18 Oct 2016 02:16:26 +0200 Adam Borowski <kilobyte@angband.pl> wrote: > On Mon, Oct 17, 2016 at 02:22:34PM +0200, Mathieu OTHACEHE wrote: > > > +#include <asm/uaccess.h> > > > +#include <asm/uaccess.h> > > > > Included twice. > > D'oh! > > > > +#include <asm/string.h> > > > +#include <asm/page.h> > > > +#include <asm/checksum.h> > > > + > > > +#include <asm-generic/asm-prototypes.h> > > > + > > > +#include <asm/page.h> > > > +#include <asm/pgtable.h> > > > +#include <asm/special_insns.h> > > > +#include <asm/preempt.h> > > > > No <asm/arch_hweight.h> for __sw_hweight32 and __sw_hweight64 ? > > diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h > new file mode 100644 > index 0000000..df13637 > --- /dev/null > +++ b/include/asm-generic/asm-prototypes.h > @@ -0,0 +1,7 @@ > +#include <linux/bitops.h> > > ... which has these. > > Alexey Dobriyan <adobriyan@gmail.com> wrote: > } bitops.h is wrong header as well. > } Why do you need bitops for bunch of function prototypes? > > Unless you guys prefer using low-level headers only, that is. Well you can't use asm/arch_hweight.h in a generic header of course. I would suggest just including linux/ variants where practical for the asm-generic/asm-prototypes.h header. We should probably just bring all these arch patches through the kbuild tree. I'm sorry for the breakage: I didn't realize it broke the build with some configs, otherwise I would have given Michal a heads up before his pull request, and worked to get this stuff in first. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-18 1:34 ` Nicholas Piggin @ 2016-10-19 14:38 ` Michal Marek 2016-10-20 3:52 ` Nicholas Piggin 2016-11-01 15:48 ` Michal Marek 1 sibling, 1 reply; 58+ messages in thread From: Michal Marek @ 2016-10-19 14:38 UTC (permalink / raw) To: Nicholas Piggin, Adam Borowski Cc: Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): > We should probably just bring all these arch patches through the > kbuild tree. > > I'm sorry for the breakage: I didn't realize it broke the build with > some configs, otherwise I would have given Michal a heads up before > his pull request, and worked to get this stuff in first. It breaks with some binutils versions only (and only with CONFIG_MODVERSIONS=y, of course). Michal ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-19 14:38 ` Michal Marek @ 2016-10-20 3:52 ` Nicholas Piggin 2016-10-27 8:10 ` Kalle Valo 0 siblings, 1 reply; 58+ messages in thread From: Nicholas Piggin @ 2016-10-20 3:52 UTC (permalink / raw) To: Michal Marek Cc: Adam Borowski, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Wed, 19 Oct 2016 16:38:14 +0200 Michal Marek <mmarek@suse.com> wrote: > Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): > > We should probably just bring all these arch patches through the > > kbuild tree. > > > > I'm sorry for the breakage: I didn't realize it broke the build with > > some configs, otherwise I would have given Michal a heads up before > > his pull request, and worked to get this stuff in first. > > It breaks with some binutils versions only (and only with > CONFIG_MODVERSIONS=y, of course). Yeah this seems to be the issue, it apparently slipped past all the automated builds. It seems like the existing CRC warnings in the tree only trigger in rare circumstances too, so something could be a bit fragile there. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-20 3:52 ` Nicholas Piggin @ 2016-10-27 8:10 ` Kalle Valo 2016-10-27 11:15 ` Nicholas Piggin 2016-10-30 10:51 ` Thorsten Leemhuis 0 siblings, 2 replies; 58+ messages in thread From: Kalle Valo @ 2016-10-27 8:10 UTC (permalink / raw) To: Nicholas Piggin Cc: Michal Marek, Adam Borowski, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, Thorsten Leemhuis, Steven Rostedt (Adding Thorsten because of a serious regression and Steven because he tried to fix something in the same commit) Nicholas Piggin <npiggin@gmail.com> writes: > On Wed, 19 Oct 2016 16:38:14 +0200 > Michal Marek <mmarek@suse.com> wrote: > >> Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): >> > We should probably just bring all these arch patches through the >> > kbuild tree. >> > >> > I'm sorry for the breakage: I didn't realize it broke the build with >> > some configs, otherwise I would have given Michal a heads up before >> > his pull request, and worked to get this stuff in first. >> >> It breaks with some binutils versions only (and only with >> CONFIG_MODVERSIONS=y, of course). > > Yeah this seems to be the issue, it apparently slipped past all the > automated builds. It seems like the existing CRC warnings in the tree > only trigger in rare circumstances too, so something could be a bit > fragile there. I upgraded from 4.8 to 4.9-rc2 and noticed that kernel modules fail to load (log below). After investigating for some time I found this thread and apparently this is not still fixed, at least not in Linus' tree. Reverting 784d5699eddc5 fixed the issue for me. As I don't see any fix available (please correct me if I'm wrong) we should just revert that commit until it's properly fixed. Also note that there's a related fix from Steven: [PATCH] x86: Fix export for mcount and __fentry__ https://marc.info/?l=linux-kernel&m=147733572502413 For compiling the kernel I'm using Ubuntu 12.04: ii binutils 2.22-6ubuntu1.4 GNU assembler, linker and binary utilities ii gcc 4:4.6.3-1ubuntu5 GNU C compiler The kernel is running on a separate machine with Ubuntu 14.04. [ 110.703414] bluetooth: disagrees about version of symbol __get_user_2 [ 110.703416] bluetooth: Unknown symbol __get_user_2 (err -22) [ 110.703429] bluetooth: disagrees about version of symbol __put_user_2 [ 110.703430] bluetooth: Unknown symbol __put_user_2 (err -22) [ 110.703579] bluetooth: disagrees about version of symbol __put_user_4 [ 110.703580] bluetooth: Unknown symbol __put_user_4 (err -22) [ 110.703669] bluetooth: disagrees about version of symbol __put_user_1 [ 110.703670] bluetooth: Unknown symbol __put_user_1 (err -22) [ 110.703688] bluetooth: disagrees about version of symbol mcount [ 110.703689] bluetooth: Unknown symbol mcount (err -22) -- Kalle Valo ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-27 8:10 ` Kalle Valo @ 2016-10-27 11:15 ` Nicholas Piggin 2016-10-27 13:14 ` Kalle Valo 2016-10-30 10:51 ` Thorsten Leemhuis 1 sibling, 1 reply; 58+ messages in thread From: Nicholas Piggin @ 2016-10-27 11:15 UTC (permalink / raw) To: Kalle Valo Cc: Michal Marek, Adam Borowski, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, Thorsten Leemhuis, Steven Rostedt On Thu, 27 Oct 2016 11:10:03 +0300 Kalle Valo <kvalo@codeaurora.org> wrote: > (Adding Thorsten because of a serious regression and Steven because he > tried to fix something in the same commit) > > Nicholas Piggin <npiggin@gmail.com> writes: > > > On Wed, 19 Oct 2016 16:38:14 +0200 > > Michal Marek <mmarek@suse.com> wrote: > > > >> Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): > >> > We should probably just bring all these arch patches through the > >> > kbuild tree. > >> > > >> > I'm sorry for the breakage: I didn't realize it broke the build with > >> > some configs, otherwise I would have given Michal a heads up before > >> > his pull request, and worked to get this stuff in first. > >> > >> It breaks with some binutils versions only (and only with > >> CONFIG_MODVERSIONS=y, of course). > > > > Yeah this seems to be the issue, it apparently slipped past all the > > automated builds. It seems like the existing CRC warnings in the tree > > only trigger in rare circumstances too, so something could be a bit > > fragile there. > > I upgraded from 4.8 to 4.9-rc2 and noticed that kernel modules fail to > load (log below). After investigating for some time I found this thread > and apparently this is not still fixed, at least not in Linus' tree. > > Reverting 784d5699eddc5 fixed the issue for me. As I don't see any fix > available (please correct me if I'm wrong) we should just revert that > commit until it's properly fixed. With these two patches together, does it work for you? http://marc.info/?l=linux-arch&m=147653546809512&w=2 http://marc.info/?l=linux-kernel&m=147669851906489&w=2 It would be helpful if you could test and let us know, because there seems to be a very tiny number of configs and toolchains that causes problems. > > Also note that there's a related fix from Steven: > > [PATCH] x86: Fix export for mcount and __fentry__ > https://marc.info/?l=linux-kernel&m=147733572502413 > > For compiling the kernel I'm using Ubuntu 12.04: > > ii binutils 2.22-6ubuntu1.4 GNU assembler, linker and binary utilities > ii gcc 4:4.6.3-1ubuntu5 GNU C compiler > > The kernel is running on a separate machine with Ubuntu 14.04. > > [ 110.703414] bluetooth: disagrees about version of symbol __get_user_2 > [ 110.703416] bluetooth: Unknown symbol __get_user_2 (err -22) > [ 110.703429] bluetooth: disagrees about version of symbol __put_user_2 > [ 110.703430] bluetooth: Unknown symbol __put_user_2 (err -22) > [ 110.703579] bluetooth: disagrees about version of symbol __put_user_4 > [ 110.703580] bluetooth: Unknown symbol __put_user_4 (err -22) > [ 110.703669] bluetooth: disagrees about version of symbol __put_user_1 > [ 110.703670] bluetooth: Unknown symbol __put_user_1 (err -22) > [ 110.703688] bluetooth: disagrees about version of symbol mcount > [ 110.703689] bluetooth: Unknown symbol mcount (err -22) > I haven't seen that one before. Did you definitely make and install new modules? Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-27 11:15 ` Nicholas Piggin @ 2016-10-27 13:14 ` Kalle Valo 2016-10-27 13:25 ` Nicholas Piggin 0 siblings, 1 reply; 58+ messages in thread From: Kalle Valo @ 2016-10-27 13:14 UTC (permalink / raw) To: Nicholas Piggin Cc: Michal Marek, Adam Borowski, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, Thorsten Leemhuis, Steven Rostedt Nicholas Piggin <npiggin@gmail.com> writes: > On Thu, 27 Oct 2016 11:10:03 +0300 > Kalle Valo <kvalo@codeaurora.org> wrote: > >> (Adding Thorsten because of a serious regression and Steven because he >> tried to fix something in the same commit) >> >> Nicholas Piggin <npiggin@gmail.com> writes: >> >> > On Wed, 19 Oct 2016 16:38:14 +0200 >> > Michal Marek <mmarek@suse.com> wrote: >> > >> >> Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): >> >> > We should probably just bring all these arch patches through the >> >> > kbuild tree. >> >> > >> >> > I'm sorry for the breakage: I didn't realize it broke the build with >> >> > some configs, otherwise I would have given Michal a heads up before >> >> > his pull request, and worked to get this stuff in first. >> >> >> >> It breaks with some binutils versions only (and only with >> >> CONFIG_MODVERSIONS=y, of course). >> > >> > Yeah this seems to be the issue, it apparently slipped past all the >> > automated builds. It seems like the existing CRC warnings in the tree >> > only trigger in rare circumstances too, so something could be a bit >> > fragile there. >> >> I upgraded from 4.8 to 4.9-rc2 and noticed that kernel modules fail to >> load (log below). After investigating for some time I found this thread >> and apparently this is not still fixed, at least not in Linus' tree. >> >> Reverting 784d5699eddc5 fixed the issue for me. As I don't see any fix >> available (please correct me if I'm wrong) we should just revert that >> commit until it's properly fixed. > > With these two patches together, does it work for you? > > http://marc.info/?l=linux-arch&m=147653546809512&w=2 > http://marc.info/?l=linux-kernel&m=147669851906489&w=2 > > It would be helpful if you could test and let us know, because there seems > to be a very tiny number of configs and toolchains that causes > problems. With these two patches (on top of ath-201610251249 from ath.git, in practice 4.9-rc2 + latest wireless patches) module loading works again. If you want you can add my Tested-by: Tested-by: Kalle Valo <kvalo@codeaurora.org> Can we get these patches to Linus' tree soon? It's annoying to revert 784d5699eddc5 everytime I update my tree. >> Also note that there's a related fix from Steven: >> >> [PATCH] x86: Fix export for mcount and __fentry__ >> https://marc.info/?l=linux-kernel&m=147733572502413 >> >> For compiling the kernel I'm using Ubuntu 12.04: >> >> ii binutils 2.22-6ubuntu1.4 GNU assembler, linker and binary utilities >> ii gcc 4:4.6.3-1ubuntu5 GNU C compiler >> >> The kernel is running on a separate machine with Ubuntu 14.04. >> >> [ 110.703414] bluetooth: disagrees about version of symbol __get_user_2 >> [ 110.703416] bluetooth: Unknown symbol __get_user_2 (err -22) >> [ 110.703429] bluetooth: disagrees about version of symbol __put_user_2 >> [ 110.703430] bluetooth: Unknown symbol __put_user_2 (err -22) >> [ 110.703579] bluetooth: disagrees about version of symbol __put_user_4 >> [ 110.703580] bluetooth: Unknown symbol __put_user_4 (err -22) >> [ 110.703669] bluetooth: disagrees about version of symbol __put_user_1 >> [ 110.703670] bluetooth: Unknown symbol __put_user_1 (err -22) >> [ 110.703688] bluetooth: disagrees about version of symbol mcount >> [ 110.703689] bluetooth: Unknown symbol mcount (err -22) >> > > I haven't seen that one before. Did you definitely make and install new > modules? I'm pretty sure modules are correctly installed as I have used the same procedure for years: on my workstation I do 'make bindeb-pkg', copy the .deb to the test laptop and install the deb there. Also once I revert 784d5699eddc5 it starts immeadiately working. -- Kalle Valo ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-27 13:14 ` Kalle Valo @ 2016-10-27 13:25 ` Nicholas Piggin 0 siblings, 0 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-10-27 13:25 UTC (permalink / raw) To: Kalle Valo Cc: Michal Marek, Adam Borowski, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, Thorsten Leemhuis, Steven Rostedt On Thu, 27 Oct 2016 16:14:28 +0300 Kalle Valo <kvalo@codeaurora.org> wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > > > On Thu, 27 Oct 2016 11:10:03 +0300 > > Kalle Valo <kvalo@codeaurora.org> wrote: > > > >> (Adding Thorsten because of a serious regression and Steven because he > >> tried to fix something in the same commit) > >> > >> Nicholas Piggin <npiggin@gmail.com> writes: > >> > >> > On Wed, 19 Oct 2016 16:38:14 +0200 > >> > Michal Marek <mmarek@suse.com> wrote: > >> > > >> >> Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): > >> >> > We should probably just bring all these arch patches through the > >> >> > kbuild tree. > >> >> > > >> >> > I'm sorry for the breakage: I didn't realize it broke the build with > >> >> > some configs, otherwise I would have given Michal a heads up before > >> >> > his pull request, and worked to get this stuff in first. > >> >> > >> >> It breaks with some binutils versions only (and only with > >> >> CONFIG_MODVERSIONS=y, of course). > >> > > >> > Yeah this seems to be the issue, it apparently slipped past all the > >> > automated builds. It seems like the existing CRC warnings in the tree > >> > only trigger in rare circumstances too, so something could be a bit > >> > fragile there. > >> > >> I upgraded from 4.8 to 4.9-rc2 and noticed that kernel modules fail to > >> load (log below). After investigating for some time I found this thread > >> and apparently this is not still fixed, at least not in Linus' tree. > >> > >> Reverting 784d5699eddc5 fixed the issue for me. As I don't see any fix > >> available (please correct me if I'm wrong) we should just revert that > >> commit until it's properly fixed. > > > > With these two patches together, does it work for you? > > > > http://marc.info/?l=linux-arch&m=147653546809512&w=2 > > http://marc.info/?l=linux-kernel&m=147669851906489&w=2 > > > > It would be helpful if you could test and let us know, because there seems > > to be a very tiny number of configs and toolchains that causes > > problems. > > With these two patches (on top of ath-201610251249 from ath.git, in > practice 4.9-rc2 + latest wireless patches) module loading works again. > If you want you can add my Tested-by: > > Tested-by: Kalle Valo <kvalo@codeaurora.org> Great, thanks for testing it. > Can we get these patches to Linus' tree soon? It's annoying to revert > 784d5699eddc5 everytime I update my tree. Yes I think it's about ready to merge. Michal is returning from vacation next week so we should get some progress soon. > >> Also note that there's a related fix from Steven: > >> > >> [PATCH] x86: Fix export for mcount and __fentry__ > >> https://marc.info/?l=linux-kernel&m=147733572502413 > >> > >> For compiling the kernel I'm using Ubuntu 12.04: > >> > >> ii binutils 2.22-6ubuntu1.4 GNU assembler, linker and binary utilities > >> ii gcc 4:4.6.3-1ubuntu5 GNU C compiler > >> > >> The kernel is running on a separate machine with Ubuntu 14.04. > >> > >> [ 110.703414] bluetooth: disagrees about version of symbol __get_user_2 > >> [ 110.703416] bluetooth: Unknown symbol __get_user_2 (err -22) > >> [ 110.703429] bluetooth: disagrees about version of symbol __put_user_2 > >> [ 110.703430] bluetooth: Unknown symbol __put_user_2 (err -22) > >> [ 110.703579] bluetooth: disagrees about version of symbol __put_user_4 > >> [ 110.703580] bluetooth: Unknown symbol __put_user_4 (err -22) > >> [ 110.703669] bluetooth: disagrees about version of symbol __put_user_1 > >> [ 110.703670] bluetooth: Unknown symbol __put_user_1 (err -22) > >> [ 110.703688] bluetooth: disagrees about version of symbol mcount > >> [ 110.703689] bluetooth: Unknown symbol mcount (err -22) > >> > > > > I haven't seen that one before. Did you definitely make and install new > > modules? > > I'm pretty sure modules are correctly installed as I have used the same > procedure for years: on my workstation I do 'make bindeb-pkg', copy the > .deb to the test laptop and install the deb there. Also once I revert > 784d5699eddc5 it starts immeadiately working. > Sure, I was just checking because I've seen several types of failure but not this one before. Thanks for reporting and testing. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-27 8:10 ` Kalle Valo 2016-10-27 11:15 ` Nicholas Piggin @ 2016-10-30 10:51 ` Thorsten Leemhuis 1 sibling, 0 replies; 58+ messages in thread From: Thorsten Leemhuis @ 2016-10-30 10:51 UTC (permalink / raw) To: Kalle Valo, Nicholas Piggin Cc: Michal Marek, Adam Borowski, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, Steven Rostedt On 27.10.2016 10:10, Kalle Valo wrote: > (Adding Thorsten because of a serious regression and Steven because he > tried to fix something in the same commit) Many thx. I added this report to the list of regressions for Linux 4.9. I'll watch this thread for further updates on this issue to document progress in my weekly reports. Please let me know via regressions@leemhuis.info in case the discussion moves to a different place (bugzilla or another mail thread for example). Current status (afaics): Fix available, waiting for Michal to get back from vacation. tia! Ciao, Thorsten > Nicholas Piggin <npiggin@gmail.com> writes: > >> On Wed, 19 Oct 2016 16:38:14 +0200 >> Michal Marek <mmarek@suse.com> wrote: >> >>> Dne 18.10.2016 v 03:34 Nicholas Piggin napsal(a): >>>> We should probably just bring all these arch patches through the >>>> kbuild tree. >>>> >>>> I'm sorry for the breakage: I didn't realize it broke the build with >>>> some configs, otherwise I would have given Michal a heads up before >>>> his pull request, and worked to get this stuff in first. >>> >>> It breaks with some binutils versions only (and only with >>> CONFIG_MODVERSIONS=y, of course). >> >> Yeah this seems to be the issue, it apparently slipped past all the >> automated builds. It seems like the existing CRC warnings in the tree >> only trigger in rare circumstances too, so something could be a bit >> fragile there. > > I upgraded from 4.8 to 4.9-rc2 and noticed that kernel modules fail to > load (log below). After investigating for some time I found this thread > and apparently this is not still fixed, at least not in Linus' tree. > > Reverting 784d5699eddc5 fixed the issue for me. As I don't see any fix > available (please correct me if I'm wrong) we should just revert that > commit until it's properly fixed. > > Also note that there's a related fix from Steven: > > [PATCH] x86: Fix export for mcount and __fentry__ > https://marc.info/?l=linux-kernel&m=147733572502413 > > For compiling the kernel I'm using Ubuntu 12.04: > > ii binutils 2.22-6ubuntu1.4 GNU assembler, linker and binary utilities > ii gcc 4:4.6.3-1ubuntu5 GNU C compiler > > The kernel is running on a separate machine with Ubuntu 14.04. > > [ 110.703414] bluetooth: disagrees about version of symbol __get_user_2 > [ 110.703416] bluetooth: Unknown symbol __get_user_2 (err -22) > [ 110.703429] bluetooth: disagrees about version of symbol __put_user_2 > [ 110.703430] bluetooth: Unknown symbol __put_user_2 (err -22) > [ 110.703579] bluetooth: disagrees about version of symbol __put_user_4 > [ 110.703580] bluetooth: Unknown symbol __put_user_4 (err -22) > [ 110.703669] bluetooth: disagrees about version of symbol __put_user_1 > [ 110.703670] bluetooth: Unknown symbol __put_user_1 (err -22) > [ 110.703688] bluetooth: disagrees about version of symbol mcount > [ 110.703689] bluetooth: Unknown symbol mcount (err -22) > ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-18 1:34 ` Nicholas Piggin 2016-10-19 14:38 ` Michal Marek @ 2016-11-01 15:48 ` Michal Marek 2016-11-02 12:11 ` Adam Borowski 1 sibling, 1 reply; 58+ messages in thread From: Michal Marek @ 2016-11-01 15:48 UTC (permalink / raw) To: Nicholas Piggin, Adam Borowski Cc: Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On 2016-10-18 03:34, Nicholas Piggin wrote: > Hi Adam, > > Thanks, this is looking good. powerpc will be able to use the generic > header. > > On Tue, 18 Oct 2016 02:16:26 +0200 > Adam Borowski <kilobyte@angband.pl> wrote: > >> On Mon, Oct 17, 2016 at 02:22:34PM +0200, Mathieu OTHACEHE wrote: >>>> +#include <asm/uaccess.h> >>>> +#include <asm/uaccess.h> >>> >>> Included twice. >> >> D'oh! >> >>>> +#include <asm/string.h> >>>> +#include <asm/page.h> >>>> +#include <asm/checksum.h> >>>> + >>>> +#include <asm-generic/asm-prototypes.h> >>>> + >>>> +#include <asm/page.h> >>>> +#include <asm/pgtable.h> >>>> +#include <asm/special_insns.h> >>>> +#include <asm/preempt.h> >>> >>> No <asm/arch_hweight.h> for __sw_hweight32 and __sw_hweight64 ? >> >> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h >> new file mode 100644 >> index 0000000..df13637 >> --- /dev/null >> +++ b/include/asm-generic/asm-prototypes.h >> @@ -0,0 +1,7 @@ >> +#include <linux/bitops.h> >> >> ... which has these. >> >> Alexey Dobriyan <adobriyan@gmail.com> wrote: >> } bitops.h is wrong header as well. >> } Why do you need bitops for bunch of function prototypes? >> >> Unless you guys prefer using low-level headers only, that is. > > Well you can't use asm/arch_hweight.h in a generic header of course. > I would suggest just including linux/ variants where practical for > the asm-generic/asm-prototypes.h header. > > We should probably just bring all these arch patches through the > kbuild tree. Adam, are you submitting a new version of your x86 asm-prototypes.h patch? Thanks, Michal ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-11-01 15:48 ` Michal Marek @ 2016-11-02 12:11 ` Adam Borowski 2016-11-02 12:14 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 Adam Borowski 0 siblings, 1 reply; 58+ messages in thread From: Adam Borowski @ 2016-11-02 12:11 UTC (permalink / raw) To: Michal Marek Cc: Nicholas Piggin, Mathieu OTHACEHE, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Tue, Nov 01, 2016 at 04:48:48PM +0100, Michal Marek wrote: > > Adam Borowski <kilobyte@angband.pl> wrote: > > > >> On Mon, Oct 17, 2016 at 02:22:34PM +0200, Mathieu OTHACEHE wrote: > >>>> +#include <asm/uaccess.h> > >>>> +#include <asm/uaccess.h> > >>> > >>> Included twice. > >> > >> D'oh! This appears to be the only thing to fix, right? > >>>> +#include <asm/string.h> > >>>> +#include <asm/page.h> > >>>> +#include <asm/checksum.h> > >>>> + > >>>> +#include <asm-generic/asm-prototypes.h> > >>>> + > >>>> +#include <asm/page.h> > >>>> +#include <asm/pgtable.h> > >>>> +#include <asm/special_insns.h> > >>>> +#include <asm/preempt.h> > >>> > >>> No <asm/arch_hweight.h> for __sw_hweight32 and __sw_hweight64 ? > >> > >> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h > >> new file mode 100644 > >> index 0000000..df13637 > >> --- /dev/null > >> +++ b/include/asm-generic/asm-prototypes.h > >> @@ -0,0 +1,7 @@ > >> +#include <linux/bitops.h> > >> > >> ... which has these. > >> > >> Alexey Dobriyan <adobriyan@gmail.com> wrote: > >> } bitops.h is wrong header as well. > >> } Why do you need bitops for bunch of function prototypes? > >> > >> Unless you guys prefer using low-level headers only, that is. > > > > Well you can't use asm/arch_hweight.h in a generic header of course. > > I would suggest just including linux/ variants where practical for > > the asm-generic/asm-prototypes.h header. > > > > We should probably just bring all these arch patches through the > > kbuild tree. I believe inclusion of <linux/bitops.h> is the right thing to do, but if not, the patch would also need: extern unsigned int __sw_hweight8(unsigned int w); extern unsigned int __sw_hweight16(unsigned int w); extern unsigned int __sw_hweight32(unsigned int w); extern unsigned long __sw_hweight64(__u64 w); There's also the issue of mcount/__fentry__, but that's apparently already dealt with in 5de0a8c, already in mainline. > are you submitting a new version of your x86 asm-prototypes.h patch? The update is trivial, but yeah, I can resubmit. If I'm wrong about where __sw_hweight{8,16,32,64} should come from, please say so. Meow! -- A MAP07 (Dead Simple) raspberry tincture recipe: 0.5l 95% alcohol, 1kg raspberries, 0.4kg sugar; put into a big jar for 1 month. Filter out and throw away the fruits (can dump them into a cake, etc), let the drink age at least 3-6 months. ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 2016-11-02 12:11 ` Adam Borowski @ 2016-11-02 12:14 ` Adam Borowski 0 siblings, 0 replies; 58+ messages in thread From: Adam Borowski @ 2016-11-02 12:14 UTC (permalink / raw) To: Michal Marek, Nicholas Piggin, Mathieu OTHACEHE, Omar, Sandoval, osandov, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch Cc: Adam Borowski Nicholas Piggin wrote: > Architectures will need to have an include/asm/asm-prototypes.h that > defines or #include<>s C-style prototypes for exported asm functions. > We can do an asm-generic version for the common ones like memset so > there's not a lot of pointless duplication there. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- arch/x86/include/asm/asm-prototypes.h | 12 ++++++++++++ include/asm-generic/asm-prototypes.h | 7 +++++++ 2 files changed, 19 insertions(+) create mode 100644 arch/x86/include/asm/asm-prototypes.h create mode 100644 include/asm-generic/asm-prototypes.h diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h new file mode 100644 index 0000000..ae87224 --- /dev/null +++ b/arch/x86/include/asm/asm-prototypes.h @@ -0,0 +1,12 @@ +#include <asm/ftrace.h> +#include <asm/uaccess.h> +#include <asm/string.h> +#include <asm/page.h> +#include <asm/checksum.h> + +#include <asm-generic/asm-prototypes.h> + +#include <asm/page.h> +#include <asm/pgtable.h> +#include <asm/special_insns.h> +#include <asm/preempt.h> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h new file mode 100644 index 0000000..df13637 --- /dev/null +++ b/include/asm-generic/asm-prototypes.h @@ -0,0 +1,7 @@ +#include <linux/bitops.h> +extern void *__memset(void *, int, __kernel_size_t); +extern void *__memcpy(void *, const void *, __kernel_size_t); +extern void *__memmove(void *, const void *, __kernel_size_t); +extern void *memset(void *, int, __kernel_size_t); +extern void *memcpy(void *, const void *, __kernel_size_t); +extern void *memmove(void *, const void *, __kernel_size_t); -- 2.10.2 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-10-17 10:01 ` Adam Borowski ` (3 preceding siblings ...) (?) @ 2016-12-16 19:55 ` Jiri Slaby 2016-12-16 19:57 ` Linus Torvalds -1 siblings, 1 reply; 58+ messages in thread From: Jiri Slaby @ 2016-12-16 19:55 UTC (permalink / raw) To: Adam Borowski, Nicholas Piggin Cc: Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On 10/17/2016, 12:01 PM, Adam Borowski wrote: > Anyway, here's my stab at x86: Hi, what happened to this? I had to apply this to fix 4.9-pae kernel here. > From db746df65b920591606398b4b244f5b6dc9eea04 Mon Sep 17 00:00:00 2001 > From: Adam Borowski <kilobyte@angband.pl> > Date: Mon, 17 Oct 2016 11:42:35 +0200 > Subject: [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 > > Nicholas Piggin wrote: >> Architectures will need to have an include/asm/asm-prototypes.h that >> defines or #include<>s C-style prototypes for exported asm functions. >> We can do an asm-generic version for the common ones like memset so >> there's not a lot of pointless duplication there. > > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > arch/x86/include/asm/asm-prototypes.h | 13 +++++++++++++ > include/asm-generic/asm-prototypes.h | 7 +++++++ > 2 files changed, 20 insertions(+) > create mode 100644 arch/x86/include/asm/asm-prototypes.h > create mode 100644 include/asm-generic/asm-prototypes.h > > diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h > new file mode 100644 > index 0000000..072c97c > --- /dev/null > +++ b/arch/x86/include/asm/asm-prototypes.h > @@ -0,0 +1,13 @@ > +#include <asm/ftrace.h> > +#include <asm/uaccess.h> > +#include <asm/uaccess.h> > +#include <asm/string.h> > +#include <asm/page.h> > +#include <asm/checksum.h> > + > +#include <asm-generic/asm-prototypes.h> > + > +#include <asm/page.h> > +#include <asm/pgtable.h> > +#include <asm/special_insns.h> > +#include <asm/preempt.h> > diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h > new file mode 100644 > index 0000000..df13637 > --- /dev/null > +++ b/include/asm-generic/asm-prototypes.h > @@ -0,0 +1,7 @@ > +#include <linux/bitops.h> > +extern void *__memset(void *, int, __kernel_size_t); > +extern void *__memcpy(void *, const void *, __kernel_size_t); > +extern void *__memmove(void *, const void *, __kernel_size_t); > +extern void *memset(void *, int, __kernel_size_t); > +extern void *memcpy(void *, const void *, __kernel_size_t); > +extern void *memmove(void *, const void *, __kernel_size_t); > thanks, -- js suse labs ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-16 19:55 ` [GIT PULL] kbuild changes for v4.9-rc1 Jiri Slaby @ 2016-12-16 19:57 ` Linus Torvalds 2016-12-17 8:57 ` Jiri Slaby 0 siblings, 1 reply; 58+ messages in thread From: Linus Torvalds @ 2016-12-16 19:57 UTC (permalink / raw) To: Jiri Slaby Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On Fri, Dec 16, 2016 at 11:55 AM, Jiri Slaby <jslaby@suse.cz> wrote: > > what happened to this? I had to apply this to fix 4.9-pae kernel here. Did you actually have to do that? Because a missing CRC shouldn't be fatal in 4.9. What was the failure mode? Linus ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-16 19:57 ` Linus Torvalds @ 2016-12-17 8:57 ` Jiri Slaby 2016-12-17 9:33 ` Adam Borowski 2016-12-17 23:59 ` Linus Torvalds 0 siblings, 2 replies; 58+ messages in thread From: Jiri Slaby @ 2016-12-17 8:57 UTC (permalink / raw) To: Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 12/16/2016, 08:57 PM, Linus Torvalds wrote: > On Fri, Dec 16, 2016 at 11:55 AM, Jiri Slaby <jslaby@suse.cz> wrote: >> >> what happened to this? I had to apply this to fix 4.9-pae kernel here. > > Did you actually have to do that? Yes, disk drivers won't load: [ 2.141973] virtio_pci: disagrees about version of symbol mcount [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) [ 2.164547] virtio_pci: disagrees about version of symbol mcount [ 2.166309] virtio_pci: Unknown symbol mcount (err -22) [ 2.180651] virtio_pci: disagrees about version of symbol mcount [ 2.182823] virtio_pci: Unknown symbol mcount (err -22) [ 2.210943] virtio_pci: disagrees about version of symbol mcount [ 2.220097] virtio_pci: Unknown symbol mcount (err -22) [ 2.220173] ata_piix: disagrees about version of symbol mcount [ 2.220174] ata_piix: Unknown symbol mcount (err -22) and whole machine gets stuck with systemd waiting for /dev/sd*. > Because a missing CRC shouldn't be fatal in 4.9. > > What was the failure mode? I am not sure what you mean? The kernel is rpm-ized 4.9 vanilla and this is the config: http://kernel.suse.com/cgit/kernel-source/tree/config/i386/default?h=stable thanks, -- js suse labs ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-17 8:57 ` Jiri Slaby @ 2016-12-17 9:33 ` Adam Borowski 2016-12-17 23:59 ` Linus Torvalds 1 sibling, 0 replies; 58+ messages in thread From: Adam Borowski @ 2016-12-17 9:33 UTC (permalink / raw) To: Jiri Slaby Cc: Linus Torvalds, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On Sat, Dec 17, 2016 at 09:57:47AM +0100, Jiri Slaby wrote: > On 12/16/2016, 08:57 PM, Linus Torvalds wrote: > > On Fri, Dec 16, 2016 at 11:55 AM, Jiri Slaby <jslaby@suse.cz> wrote: > >> > >> what happened to this? I had to apply this to fix 4.9-pae kernel here. > > > > Did you actually have to do that? > > Yes, disk drivers won't load: > [ 2.141973] virtio_pci: disagrees about version of symbol mcount > [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) > and whole machine gets stuck with systemd waiting for /dev/sd*. > > > Because a missing CRC shouldn't be fatal in 4.9. Most of us get just a scary-looking warning, but whatever the problem is for you, it's good to hear this patch works around it. Whatever the long-term solution will be, for 4.10 an updated[1] version of this fix is on kbuild/kbuild (and kbuild/for-next). I guess we'll bother stable@ once it is merged. Note that it handles only x86, there's a bunch of other architectures affected, alpha m68k s390 sparc ia64 might still need fixing. Meow! [1]. Turns out there was a missing symbol on 486; people build-test those but don't try to actually boot, and even when they do, they don't read warnings. -- Autotools hint: to do a zx-spectrum build on a pdp11 host, type: ./configure --host=zx-spectrum --build=pdp11 ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-17 8:57 ` Jiri Slaby 2016-12-17 9:33 ` Adam Borowski @ 2016-12-17 23:59 ` Linus Torvalds 2016-12-18 10:49 ` Jiri Slaby 1 sibling, 1 reply; 58+ messages in thread From: Linus Torvalds @ 2016-12-17 23:59 UTC (permalink / raw) To: Jiri Slaby Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: > > Yes, disk drivers won't load: > [ 2.141973] virtio_pci: disagrees about version of symbol mcount > [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) This makes no sense. mcount isn't even one of the symbols that the patch by Adam is touching. There's something else screwed up here. Not to mention that others don't have your issue. Do you have some other hacks in this area? Are you testing actual plain 4.9, or do you (for example) still carry Arnd's patch around that turned out to not work (reverted by f27c2f69cc8e in my tree)? Linus ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-17 23:59 ` Linus Torvalds @ 2016-12-18 10:49 ` Jiri Slaby 2016-12-18 11:03 ` Arend Van Spriel 0 siblings, 1 reply; 58+ messages in thread From: Jiri Slaby @ 2016-12-18 10:49 UTC (permalink / raw) To: Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 12/18/2016, 12:59 AM, Linus Torvalds wrote: > On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: >> >> Yes, disk drivers won't load: >> [ 2.141973] virtio_pci: disagrees about version of symbol mcount >> [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) > > This makes no sense. > > mcount isn't even one of the symbols that the patch by Adam is touching. asm-prototypes.h in his patch includes asm/ftrace.h, where the function is declared. That should be enough IIUC scripts/Makefile.build. > There's something else screwed up here. Not to mention that others > don't have your issue. I suppose people don't run i386 kernels or have different config. > Do you have some other hacks in this area? Are you testing actual > plain 4.9, or do you (for example) still carry Arnd's patch around > that turned out to not work (reverted by f27c2f69cc8e in my tree)? Not at all. This was plain 4.9 packaged by suse -- only rpm-related fixes. I tried plain 4.9 without rpm right now with the same output: # insmod soundcore.ko [ 31.582326] soundcore: disagrees about version of symbol mcount [ 31.586183] soundcore: Unknown symbol mcount (err -22) insmod: ERROR: could not insert module soundcore.ko: Invalid parameters $ git describe @ v4.9 $ git status HEAD detached at v4.9 thanks, -- js suse labs ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-18 10:49 ` Jiri Slaby @ 2016-12-18 11:03 ` Arend Van Spriel 2016-12-18 13:27 ` Nikolay Borisov 0 siblings, 1 reply; 58+ messages in thread From: Arend Van Spriel @ 2016-12-18 11:03 UTC (permalink / raw) To: Jiri Slaby, Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 18-12-2016 11:49, Jiri Slaby wrote: > On 12/18/2016, 12:59 AM, Linus Torvalds wrote: >> On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: >>> >>> Yes, disk drivers won't load: >>> [ 2.141973] virtio_pci: disagrees about version of symbol mcount >>> [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) >> >> This makes no sense. >> >> mcount isn't even one of the symbols that the patch by Adam is touching. > > asm-prototypes.h in his patch includes asm/ftrace.h, where the function > is declared. That should be enough IIUC scripts/Makefile.build. > >> There's something else screwed up here. Not to mention that others >> don't have your issue. > > I suppose people don't run i386 kernels or have different config. > >> Do you have some other hacks in this area? Are you testing actual >> plain 4.9, or do you (for example) still carry Arnd's patch around >> that turned out to not work (reverted by f27c2f69cc8e in my tree)? > > Not at all. This was plain 4.9 packaged by suse -- only rpm-related > fixes. I tried plain 4.9 without rpm right now with the same output: > # insmod soundcore.ko > [ 31.582326] soundcore: disagrees about version of symbol mcount > [ 31.586183] soundcore: Unknown symbol mcount (err -22) > insmod: ERROR: could not insert module soundcore.ko: Invalid parameters I hit an mcount issue a while back (years?) which was due to building a driver with gcc v4.x while kernel was built using gcc v4.y. Not claiming that is your issue though. Regards, Arend ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-18 11:03 ` Arend Van Spriel @ 2016-12-18 13:27 ` Nikolay Borisov 2016-12-18 14:45 ` Jiri Slaby 0 siblings, 1 reply; 58+ messages in thread From: Nikolay Borisov @ 2016-12-18 13:27 UTC (permalink / raw) To: Arend Van Spriel, Jiri Slaby, Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 18.12.2016 13:03, Arend Van Spriel wrote: > On 18-12-2016 11:49, Jiri Slaby wrote: >> On 12/18/2016, 12:59 AM, Linus Torvalds wrote: >>> On Sat, Dec 17, 2016 at 12:57 AM, Jiri Slaby <jslaby@suse.cz> wrote: >>>> >>>> Yes, disk drivers won't load: >>>> [ 2.141973] virtio_pci: disagrees about version of symbol mcount >>>> [ 2.144415] virtio_pci: Unknown symbol mcount (err -22) >>> >>> This makes no sense. >>> >>> mcount isn't even one of the symbols that the patch by Adam is touching. >> >> asm-prototypes.h in his patch includes asm/ftrace.h, where the function >> is declared. That should be enough IIUC scripts/Makefile.build. >> >>> There's something else screwed up here. Not to mention that others >>> don't have your issue. >> >> I suppose people don't run i386 kernels or have different config. >> >>> Do you have some other hacks in this area? Are you testing actual >>> plain 4.9, or do you (for example) still carry Arnd's patch around >>> that turned out to not work (reverted by f27c2f69cc8e in my tree)? >> >> Not at all. This was plain 4.9 packaged by suse -- only rpm-related >> fixes. I tried plain 4.9 without rpm right now with the same output: >> # insmod soundcore.ko >> [ 31.582326] soundcore: disagrees about version of symbol mcount >> [ 31.586183] soundcore: Unknown symbol mcount (err -22) >> insmod: ERROR: could not insert module soundcore.ko: Invalid parameters > > I hit an mcount issue a while back (years?) which was due to building a > driver with gcc v4.x while kernel was built using gcc v4.y. Not claiming > that is your issue though. I've usually had the same thing happen to me if things were compiled with different gcc versions . Essentially in newer gcc (starting with 4.6 I believe) CC_USING_FENTRY is defined, meaning that there is no mcount() symbol but rather __fentry__. This is the likely problem here. > > Regards, > Arend > ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-18 13:27 ` Nikolay Borisov @ 2016-12-18 14:45 ` Jiri Slaby 2016-12-18 14:54 ` Nikolay Borisov 0 siblings, 1 reply; 58+ messages in thread From: Jiri Slaby @ 2016-12-18 14:45 UTC (permalink / raw) To: Nikolay Borisov, Arend Van Spriel, Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 12/18/2016, 02:27 PM, Nikolay Borisov wrote: > This is the likely problem here. No, it is not. How could a rpm be built with two compilers? Moreover, with some modules, __put_user_1 and others are reported instead of mcount. -- js suse labs ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-18 14:45 ` Jiri Slaby @ 2016-12-18 14:54 ` Nikolay Borisov 2016-12-18 15:08 ` Jiri Slaby 0 siblings, 1 reply; 58+ messages in thread From: Nikolay Borisov @ 2016-12-18 14:54 UTC (permalink / raw) To: Jiri Slaby, Arend Van Spriel, Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 18.12.2016 16:45, Jiri Slaby wrote: > Moreover, with some modules, __put_user_1 and others are reported > instead of mcount. nm vmlinux | grep __fentry__ nm vmlinux | grep mcount What do these report ? I bet you that in your vmlinux the first one would return something like : ffffffff822f1810 T __fentry__ ffffffff827fdc20 r __kcrctab___fentry__ ffffffff82809461 r __kstrtab___fentry__ ffffffff827e6c20 R __ksymtab___fentry__ and nothing for the second. Whereas doing nm on the module in question would give nothing for __fentry__ and something like: U mcount ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [GIT PULL] kbuild changes for v4.9-rc1 2016-12-18 14:54 ` Nikolay Borisov @ 2016-12-18 15:08 ` Jiri Slaby 0 siblings, 0 replies; 58+ messages in thread From: Jiri Slaby @ 2016-12-18 15:08 UTC (permalink / raw) To: Nikolay Borisov, Arend Van Spriel, Linus Torvalds Cc: Adam Borowski, Nicholas Piggin, Omar Sandoval, Michal Marek, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On 12/18/2016, 03:54 PM, Nikolay Borisov wrote: > > > On 18.12.2016 16:45, Jiri Slaby wrote: >> Moreover, with some modules, __put_user_1 and others are reported >> instead of mcount. > > > nm vmlinux | grep __fentry__ > nm vmlinux | grep mcount > > What do these report ? I bet you that in your vmlinux the first one > would return something like : > > ffffffff822f1810 T __fentry__ > ffffffff827fdc20 r __kcrctab___fentry__ > ffffffff82809461 r __kstrtab___fentry__ > ffffffff827e6c20 R __ksymtab___fentry__ > and nothing for the second. Whereas doing nm on the module in question > would give nothing for __fentry__ and something like: U mcount Well, I have just won a beer: $ nm vmlinux | grep mcount w __crc_mcount c0b3bd34 r __kcrctab_mcount c0b41692 r __kstrtab_mcount c0b2dd04 R __ksymtab_mcount c0896130 T mcount c0c9ee20 T __start_mcount_loc c0cba89c T __stop_mcount_loc $ nm vmlinux | grep __fentry__ $ nm sound/soundcore.ko | grep mcount U mcount No, I am really not stupid. We compile the kernels like this for over a decade and it really broke with 4.9. Applying the patch fixes the problem. Reverting it, makes it recur. regards, -- js suse labs ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-17 6:59 ` Nicholas Piggin 2016-10-17 10:01 ` Adam Borowski @ 2016-10-17 12:26 ` Arnd Bergmann 2016-10-19 14:52 ` Michal Marek 1 sibling, 1 reply; 58+ messages in thread From: Arnd Bergmann @ 2016-10-17 12:26 UTC (permalink / raw) To: Nicholas Piggin Cc: Adam Borowski, Omar Sandoval, Michal Marek, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch This adds an asm/asm-prototypes.h header for ARM to fix the broken symbol versioning for symbols exported from assembler files. In addition to the header, we have to do these other small changes: - move the 'extern' declarations out of memset_io/memcpy_io to make them visible to the symbol version generator - move the exports from bitops.h to {change,clear,set,...}bit.S - move the exports from csumpartialgeneric.S into the files including it 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. Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/arm/include/asm/asm-prototypes.h b/arch/arm/include/asm/asm-prototypes.h new file mode 100644 index 000000000000..04e5616a7b15 --- /dev/null +++ b/arch/arm/include/asm/asm-prototypes.h @@ -0,0 +1,34 @@ +#include <linux/arm-smccc.h> +#include <linux/bitops.h> +#include <linux/ftrace.h> +#include <linux/io.h> +#include <linux/platform_data/asoc-imx-ssi.h> +#include <linux/string.h> +#include <linux/uaccess.h> + +#include <asm/checksum.h> +#include <asm/div64.h> +#include <asm/memory.h> + +extern void __aeabi_idivmod(void); +extern void __aeabi_idiv(void); +extern void __aeabi_lasr(void); +extern void __aeabi_llsl(void); +extern void __aeabi_llsr(void); +extern void __aeabi_lmul(void); +extern void __aeabi_uidivmod(void); +extern void __aeabi_uidiv(void); +extern void __aeabi_ulcmp(void); + +extern void __ashldi3(void); +extern void __ashrdi3(void); +extern void __bswapdi2(void); +extern void __bswapsi2(void); +extern void __divsi3(void); +extern void __do_div64(void); +extern void __lshrdi3(void); +extern void __modsi3(void); +extern void __muldi3(void); +extern void __ucmpdi2(void); +extern void __udivsi3(void); +extern void __umodsi3(void); diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 51458d8273ad..fbc3695293cf 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -316,11 +316,13 @@ extern void _memset_io(volatile void __iomem *, int, size_t); #define writesw(p,d,l) __raw_writesw(p,d,l) #define writesl(p,d,l) __raw_writesl(p,d,l) +extern void mmioset(void *, unsigned int, size_t); +extern void mmiocpy(void *, const void *, size_t); + #ifndef __ARMBE__ static inline void memset_io(volatile void __iomem *dst, unsigned c, size_t count) { - extern void mmioset(void *, unsigned int, size_t); mmioset((void __force *)dst, c, count); } #define memset_io(dst,c,count) memset_io(dst,c,count) @@ -328,7 +330,6 @@ static inline void memset_io(volatile void __iomem *dst, unsigned c, static inline void memcpy_fromio(void *to, const volatile void __iomem *from, size_t count) { - extern void mmiocpy(void *, const void *, size_t); mmiocpy(to, (const void __force *)from, count); } #define memcpy_fromio(to,from,count) memcpy_fromio(to,from,count) @@ -336,7 +337,6 @@ static inline void memcpy_fromio(void *to, const volatile void __iomem *from, static inline void memcpy_toio(volatile void __iomem *to, const void *from, size_t count) { - extern void mmiocpy(void *, const void *, size_t); mmiocpy((void __force *)to, from, count); } #define memcpy_toio(to,from,count) memcpy_toio(to,from,count) diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h index df06638b327c..afaef2a7faec 100644 --- a/arch/arm/lib/bitops.h +++ b/arch/arm/lib/bitops.h @@ -26,7 +26,6 @@ UNWIND( .fnstart ) bx lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm .macro testop, name, instr, store @@ -57,7 +56,6 @@ UNWIND( .fnstart ) 2: bx lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm #else .macro bitop, name, instr @@ -77,7 +75,6 @@ UNWIND( .fnstart ) ret lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm /** @@ -106,6 +103,5 @@ UNWIND( .fnstart ) ret lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm #endif diff --git a/arch/arm/lib/changebit.S b/arch/arm/lib/changebit.S index f4027862172f..005fdd18c509 100644 --- a/arch/arm/lib/changebit.S +++ b/arch/arm/lib/changebit.S @@ -13,3 +13,4 @@ .text bitop _change_bit, eor +EXPORT_SYMBOL(_change_bit) diff --git a/arch/arm/lib/clearbit.S b/arch/arm/lib/clearbit.S index f6b75fb64d30..501eff09968d 100644 --- a/arch/arm/lib/clearbit.S +++ b/arch/arm/lib/clearbit.S @@ -13,3 +13,4 @@ .text bitop _clear_bit, bic +EXPORT_SYMBOL(_clear_bit) diff --git a/arch/arm/lib/csumpartialcopy.S b/arch/arm/lib/csumpartialcopy.S index 9c3383fed129..bdcc2eea4e5c 100644 --- a/arch/arm/lib/csumpartialcopy.S +++ b/arch/arm/lib/csumpartialcopy.S @@ -49,6 +49,7 @@ #define FN_ENTRY ENTRY(csum_partial_copy_nocheck) #define FN_EXIT ENDPROC(csum_partial_copy_nocheck) -#define FN_EXPORT EXPORT_SYMBOL(csum_partial_copy_nocheck) #include "csumpartialcopygeneric.S" + +EXPORT_SYMBOL(csum_partial_copy_nocheck) diff --git a/arch/arm/lib/csumpartialcopygeneric.S b/arch/arm/lib/csumpartialcopygeneric.S index 8b94d20e51d1..06825566c0f7 100644 --- a/arch/arm/lib/csumpartialcopygeneric.S +++ b/arch/arm/lib/csumpartialcopygeneric.S @@ -332,4 +332,3 @@ FN_ENTRY mov r5, r4, get_byte_1 b .Lexit FN_EXIT -FN_EXPORT diff --git a/arch/arm/lib/csumpartialcopyuser.S b/arch/arm/lib/csumpartialcopyuser.S index 5d495edf3d83..d5522c94f58c 100644 --- a/arch/arm/lib/csumpartialcopyuser.S +++ b/arch/arm/lib/csumpartialcopyuser.S @@ -73,9 +73,9 @@ #define FN_ENTRY ENTRY(csum_partial_copy_from_user) #define FN_EXIT ENDPROC(csum_partial_copy_from_user) -#define FN_EXPORT EXPORT_SYMBOL(csum_partial_copy_from_user) #include "csumpartialcopygeneric.S" +EXPORT_SYMBOL(csum_partial_copy_from_user) /* * FIXME: minor buglet here diff --git a/arch/arm/lib/setbit.S b/arch/arm/lib/setbit.S index 618fedae4b37..d748b8d1326f 100644 --- a/arch/arm/lib/setbit.S +++ b/arch/arm/lib/setbit.S @@ -13,3 +13,4 @@ .text bitop _set_bit, orr +EXPORT_SYMBOL(_set_bit) diff --git a/arch/arm/lib/testchangebit.S b/arch/arm/lib/testchangebit.S index 4becdc3a59cb..4d2dafa9b787 100644 --- a/arch/arm/lib/testchangebit.S +++ b/arch/arm/lib/testchangebit.S @@ -13,3 +13,4 @@ .text testop _test_and_change_bit, eor, str +EXPORT_SYMBOL(_test_and_change_bit) diff --git a/arch/arm/lib/testclearbit.S b/arch/arm/lib/testclearbit.S index 918841dcce7a..fe5cae2e480a 100644 --- a/arch/arm/lib/testclearbit.S +++ b/arch/arm/lib/testclearbit.S @@ -13,3 +13,4 @@ .text testop _test_and_clear_bit, bicne, strne +EXPORT_SYMBOL(_test_and_clear_bit) diff --git a/arch/arm/lib/testsetbit.S b/arch/arm/lib/testsetbit.S index 8d1b2fe9e487..25fed837edb3 100644 --- a/arch/arm/lib/testsetbit.S +++ b/arch/arm/lib/testsetbit.S @@ -13,3 +13,4 @@ .text testop _test_and_set_bit, orreq, streq +EXPORT_SYMBOL(_test_and_set_bit) ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-17 12:26 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann @ 2016-10-19 14:52 ` Michal Marek 2016-10-19 15:02 ` Arnd Bergmann 0 siblings, 1 reply; 58+ messages in thread From: Michal Marek @ 2016-10-19 14:52 UTC (permalink / raw) To: Arnd Bergmann, Nicholas Piggin Cc: Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch Dne 17.10.2016 v 14:26 Arnd Bergmann napsal(a): > This adds an asm/asm-prototypes.h header for ARM to fix the > broken symbol versioning for symbols exported from assembler > files. > > In addition to the header, we have to do these other small > changes: > > - move the 'extern' declarations out of memset_io/memcpy_io > to make them visible to the symbol version generator > - move the exports from bitops.h to {change,clear,set,...}bit.S > - move the exports from csumpartialgeneric.S into the files > including it > > 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. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Hi Arnd, just to make sure I'm looking at the right code - is this based on the patch by Nick here: https://patchwork.kernel.org/patch/9377783/? Thanks, Michal ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-19 14:52 ` Michal Marek @ 2016-10-19 15:02 ` Arnd Bergmann 2016-10-19 15:32 ` Russell King - ARM Linux 0 siblings, 1 reply; 58+ messages in thread From: Arnd Bergmann @ 2016-10-19 15:02 UTC (permalink / raw) To: Michal Marek, Russell King Cc: Nicholas Piggin, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Wednesday, October 19, 2016 4:52:06 PM CEST Michal Marek wrote: > Dne 17.10.2016 v 14:26 Arnd Bergmann napsal(a): > > This adds an asm/asm-prototypes.h header for ARM to fix the > > broken symbol versioning for symbols exported from assembler > > files. > > > > In addition to the header, we have to do these other small > > changes: > > > > - move the 'extern' declarations out of memset_io/memcpy_io > > to make them visible to the symbol version generator > > - move the exports from bitops.h to {change,clear,set,...}bit.S > > - move the exports from csumpartialgeneric.S into the files > > including it > > > > 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. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Hi Arnd, > > just to make sure I'm looking at the right code - is this based on the > patch by Nick here: https://patchwork.kernel.org/patch/9377783/? > (adding Russell to Cc, I missed him during my earlier mail, which is now archived at https://lkml.org/lkml/2016/10/17/356) Correct. I had imported Nick's patch into my randconfig tree and this is what I needed to build all configurations cleanly with it. Arnd ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-19 15:02 ` Arnd Bergmann @ 2016-10-19 15:32 ` Russell King - ARM Linux 2016-10-20 4:08 ` Nicholas Piggin 2016-10-20 7:37 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven 0 siblings, 2 replies; 58+ messages in thread From: Russell King - ARM Linux @ 2016-10-19 15:32 UTC (permalink / raw) To: Arnd Bergmann Cc: Michal Marek, Nicholas Piggin, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Wed, Oct 19, 2016 at 05:02:55PM +0200, Arnd Bergmann wrote: > On Wednesday, October 19, 2016 4:52:06 PM CEST Michal Marek wrote: > > Dne 17.10.2016 v 14:26 Arnd Bergmann napsal(a): > > > This adds an asm/asm-prototypes.h header for ARM to fix the > > > broken symbol versioning for symbols exported from assembler > > > files. > > > > > > In addition to the header, we have to do these other small > > > changes: > > > > > > - move the 'extern' declarations out of memset_io/memcpy_io > > > to make them visible to the symbol version generator > > > - move the exports from bitops.h to {change,clear,set,...}bit.S > > > - move the exports from csumpartialgeneric.S into the files > > > including it > > > > > > 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. > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > Hi Arnd, > > > > just to make sure I'm looking at the right code - is this based on the > > patch by Nick here: https://patchwork.kernel.org/patch/9377783/? > > > > (adding Russell to Cc, I missed him during my earlier mail, which > is now archived at https://lkml.org/lkml/2016/10/17/356) I'm not in favour of this. +extern void mmioset(void *, unsigned int, size_t); +extern void mmiocpy(void *, const void *, size_t); + #ifndef __ARMBE__ static inline void memset_io(volatile void __iomem *dst, unsigned c, size_t count) { - extern void mmioset(void *, unsigned int, size_t); mmioset((void __force *)dst, c, count); } The reason they're declared _within_ memset_io() is to prevent people from using them by hiding their declaration. Moving them outside is an open invitation to stupid people starting to use them as an "oh it must be an official API". We know this happens, there's been a long history of this kind of stupid in the ARM community, not only with cache flushing APIs, but also the DMA APIs. The way the existing code is written is a completely valid way to hide declarations from outside the intended caller's scope. We've been here many times, we've had many people doing this crap, so I'm now at the point of NAKing changes which result in an increased visibility to the rest of the kernel of symbols that should not be used by stupid driver authors. Now, why do we have these extra functions when they're just aliased to memset()/memcpy() - to avoid GCC optimising them because it thinks that they're standard memset()/memcpy(). So overall this gets a NAK from me. Now, it would have _ALSO_ been nice to have been at least COPIED on the original set of changes that caused the need for this change. I wasn't. So I want to see the original set of changes reverted, because they're clearly causing breakage. Let's revert them and then go through the proper process of maintainer review, rather than bypassing maintainers and screwing up architectures in the process. There really is no excuse for this crap. -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-19 15:32 ` Russell King - ARM Linux @ 2016-10-20 4:08 ` Nicholas Piggin 2016-10-20 13:17 ` Russell King - ARM Linux 2016-10-24 15:04 ` Arnd Bergmann 2016-10-20 7:37 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven 1 sibling, 2 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-10-20 4:08 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Wed, 19 Oct 2016 16:32:00 +0100 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Wed, Oct 19, 2016 at 05:02:55PM +0200, Arnd Bergmann wrote: > > On Wednesday, October 19, 2016 4:52:06 PM CEST Michal Marek wrote: > > > Dne 17.10.2016 v 14:26 Arnd Bergmann napsal(a): > > > > This adds an asm/asm-prototypes.h header for ARM to fix the > > > > broken symbol versioning for symbols exported from assembler > > > > files. > > > > > > > > In addition to the header, we have to do these other small > > > > changes: > > > > > > > > - move the 'extern' declarations out of memset_io/memcpy_io > > > > to make them visible to the symbol version generator > > > > - move the exports from bitops.h to {change,clear,set,...}bit.S > > > > - move the exports from csumpartialgeneric.S into the files > > > > including it > > > > > > > > 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. > > > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > > > Hi Arnd, > > > > > > just to make sure I'm looking at the right code - is this based on the > > > patch by Nick here: https://patchwork.kernel.org/patch/9377783/? > > > > > > > (adding Russell to Cc, I missed him during my earlier mail, which > > is now archived at https://lkml.org/lkml/2016/10/17/356) > > I'm not in favour of this. > > +extern void mmioset(void *, unsigned int, size_t); > +extern void mmiocpy(void *, const void *, size_t); > + > #ifndef __ARMBE__ > static inline void memset_io(volatile void __iomem *dst, unsigned c, > size_t count) > { > - extern void mmioset(void *, unsigned int, size_t); > mmioset((void __force *)dst, c, count); > } > > The reason they're declared _within_ memset_io() is to prevent people > from using them by hiding their declaration. Moving them outside is > an open invitation to stupid people starting to use them as an "oh it > must be an official API". > > We know this happens, there's been a long history of this kind of stupid > in the ARM community, not only with cache flushing APIs, but also the > DMA APIs. > > The way the existing code is written is a completely valid way to hide > declarations from outside the intended caller's scope. > > We've been here many times, we've had many people doing this crap, so > I'm now at the point of NAKing changes which result in an increased > visibility to the rest of the kernel of symbols that should not be > used by stupid driver authors. > > Now, why do we have these extra functions when they're just aliased to > memset()/memcpy() - to avoid GCC optimising them because it thinks that > they're standard memset()/memcpy(). > > So overall this gets a NAK from me. Fair point, what about leaving those as they are, and also adding them to asm-prototypes.h protected with GENKSYMS ifdef? It's not beautiful, but still better than armksyms.c before Al's patches (or at least no worse). > Now, it would have _ALSO_ been nice to have been at least COPIED on the > original set of changes that caused the need for this change. I wasn't. > So I want to see the original set of changes reverted, because they're > clearly causing breakage. Let's revert them and then go through the > proper process of maintainer review, rather than bypassing maintainers > and screwing up architectures in the process. There really is no > excuse for this crap. You may have a point about improvement of the process. I wasn't involved in the original patches, but we did cc linux-arch when the .S CRC issue became known. However let's work on the assumption that they won't be reverted at this stage, and try to come up with something to fix it that you're happy with. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 4:08 ` Nicholas Piggin @ 2016-10-20 13:17 ` Russell King - ARM Linux 2016-10-20 14:20 ` Nicholas Piggin 2016-10-24 15:04 ` Arnd Bergmann 1 sibling, 1 reply; 58+ messages in thread From: Russell King - ARM Linux @ 2016-10-20 13:17 UTC (permalink / raw) To: Nicholas Piggin Cc: Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Thu, Oct 20, 2016 at 03:08:14PM +1100, Nicholas Piggin wrote: > Fair point, what about leaving those as they are, and also adding > them to asm-prototypes.h protected with GENKSYMS ifdef? It's not > beautiful, but still better than armksyms.c before Al's patches (or > at least no worse). I disagree (also see below). The armksyms way was understandable. The new way... I've no idea yet, because I wasn't even copied on any of the patches. I've no idea how the exports are now handled. I'm in a black hole with respect to that, and that's now a problem. > > Now, it would have _ALSO_ been nice to have been at least COPIED on the > > original set of changes that caused the need for this change. I wasn't. > > So I want to see the original set of changes reverted, because they're > > clearly causing breakage. Let's revert them and then go through the > > proper process of maintainer review, rather than bypassing maintainers > > and screwing up architectures in the process. There really is no > > excuse for this crap. > > You may have a point about improvement of the process. I wasn't > involved in the original patches, but we did cc linux-arch when the > .S CRC issue became known. Yes, but I'm not on linux-kernel-v2, and I've no desire to end up with another list I've no hope of keeping up with to my mailbox - I'll just ignore it. 99% of the messages on it at the time when vger kicked me off the list was x86 related discussion, and not really cross-arch issues. As I say, it just became another linux-kernel list. > However let's work on the assumption that they won't be reverted at this > stage, and try to come up with something to fix it that you're happy with. Well, there's more problems with this new KSYMS approach than just the CRCs. It forces a rebuild of the ksyms files every single time, which then causes a relink of the kernel: CHK include/config/kernel.release GEN ./Makefile CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h Using /home/rmk/git/linux-rmk as source for kernel CHK include/generated/timeconst.h CHK include/generated/bounds.h CHK include/generated/asm-offsets.h CALL /home/rmk/git/linux-rmk/scripts/checksyscalls.sh - due to target missing CHK include/generated/compile.h EXPORTS arch/arm/lib/lib-ksyms.o - due to lib-ksyms.o not in $(targets) LD arch/arm/lib/built-in.o - due to: arch/arm/lib/lib-ksyms.o EXPORTS lib/lib-ksyms.o - due to lib-ksyms.o not in $(targets) LD lib/built-in.o - due to: lib/lib-ksyms.o LD vmlinux.o MODPOST vmlinux.o - due to vmlinux.o not in $(targets) GEN .version CHK include/generated/compile.h UPD include/generated/compile.h CC init/version.o - due to: include/generated/compile.h LD init/built-in.o - due to: init/version.o KSYM .tmp_kallsyms1.o KSYM .tmp_kallsyms2.o LD vmlinux SORTEX vmlinux SYSMAP System.map OBJCOPY arch/arm/boot/Image - due to: vmlinux Building modules, stage 2. Kernel: arch/arm/boot/Image is ready LZO arch/arm/boot/compressed/piggy_data - due to: arch/arm/boot/compressed/../Image MODPOST 689 modules - due to target is PHONY AS arch/arm/boot/compressed/piggy.o - due to: arch/arm/boot/compressed/piggy_data LD arch/arm/boot/compressed/vmlinux - due to: arch/arm/boot/compressed/piggy.o OBJCOPY arch/arm/boot/zImage - due to: arch/arm/boot/compressed/vmlinux Kernel: arch/arm/boot/zImage is ready -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 13:17 ` Russell King - ARM Linux @ 2016-10-20 14:20 ` Nicholas Piggin 2016-10-20 14:33 ` Russell King - ARM Linux 0 siblings, 1 reply; 58+ messages in thread From: Nicholas Piggin @ 2016-10-20 14:20 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Thu, 20 Oct 2016 14:17:02 +0100 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Thu, Oct 20, 2016 at 03:08:14PM +1100, Nicholas Piggin wrote: > > Fair point, what about leaving those as they are, and also adding > > them to asm-prototypes.h protected with GENKSYMS ifdef? It's not > > beautiful, but still better than armksyms.c before Al's patches (or > > at least no worse). > > I disagree (also see below). The armksyms way was understandable. > The new way... I've no idea yet, because I wasn't even copied on New way is you put the EXPORT_SYMBOL in the .S file, and give it a C style prototype in asm-prototypes.h, and that's it. The build system will do the rest. Either way you require some file of C prototypes, but after Al's patches, at least the EXPORT goes with its definition. As far as rebuilding too often, that sounds like a bug, more below. > any of the patches. I've no idea how the exports are now handled. > I'm in a black hole with respect to that, and that's now a problem. > > > > Now, it would have _ALSO_ been nice to have been at least COPIED on the > > > original set of changes that caused the need for this change. I wasn't. > > > So I want to see the original set of changes reverted, because they're > > > clearly causing breakage. Let's revert them and then go through the > > > proper process of maintainer review, rather than bypassing maintainers > > > and screwing up architectures in the process. There really is no > > > excuse for this crap. > > > > You may have a point about improvement of the process. I wasn't > > involved in the original patches, but we did cc linux-arch when the > > .S CRC issue became known. > > Yes, but I'm not on linux-kernel-v2, and I've no desire to end up with > another list I've no hope of keeping up with to my mailbox - I'll just > ignore it. 99% of the messages on it at the time when vger kicked me > off the list was x86 related discussion, and not really cross-arch > issues. As I say, it just became another linux-kernel list. For the patches that touched arm code, I'd agree you should have been cc'ed. Not to dismiss that concern at all, but for issues of interest to arch code but not specific to any (such as discovery that the asm exports change would require this change to restore CRC generation), I was just saying linux-arch is most appropriate for better or worse. Ccing 30 arch lists is not workable. And again not to dismiss your concern, I'm just here trying to come up with a workable fix for the non-revert scenario. Revert is no less valid an option, it's just not one I'm in favour of myself. > > However let's work on the assumption that they won't be reverted at this > > stage, and try to come up with something to fix it that you're happy with. > > Well, there's more problems with this new KSYMS approach than just the > CRCs. It forces a rebuild of the ksyms files every single time, which > then causes a relink of the kernel: Good catch, I'm surprised you're the first one who reported it. This patch seems to do the trick for me: From: Nicholas Piggin <npiggin@gmail.com> Date: Fri, 21 Oct 2016 01:13:33 +1100 Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- scripts/Makefile.build | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index de46ab0..e1f25d6 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -430,6 +430,9 @@ cmd_export_list = $(OBJDUMP) -h $< | \ $(obj)/lib-ksyms.o: $(lib-target) FORCE $(call if_changed,export_list) + +targets += $(obj)/lib-ksyms.o + endif # -- 2.9.3 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 14:20 ` Nicholas Piggin @ 2016-10-20 14:33 ` Russell King - ARM Linux 2016-10-20 14:51 ` Nicholas Piggin 2016-10-22 19:51 ` Michal Marek 0 siblings, 2 replies; 58+ messages in thread From: Russell King - ARM Linux @ 2016-10-20 14:33 UTC (permalink / raw) To: Nicholas Piggin Cc: Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Fri, Oct 21, 2016 at 01:20:17AM +1100, Nicholas Piggin wrote: > Good catch, I'm surprised you're the first one who reported it. This patch > seems to do the trick for me: And me, thanks, so... > > From: Nicholas Piggin <npiggin@gmail.com> > Date: Fri, 21 Oct 2016 01:13:33 +1100 > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reported-by: Russell King <rmk+kernel@armlinux.org.uk> Tested-by: Russell King <rmk+kernel@armlinux.org.uk> > > --- > scripts/Makefile.build | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index de46ab0..e1f25d6 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -430,6 +430,9 @@ cmd_export_list = $(OBJDUMP) -h $< | \ > > $(obj)/lib-ksyms.o: $(lib-target) FORCE > $(call if_changed,export_list) > + > +targets += $(obj)/lib-ksyms.o > + > endif > > # -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 14:33 ` Russell King - ARM Linux @ 2016-10-20 14:51 ` Nicholas Piggin 2016-10-22 19:51 ` Michal Marek 1 sibling, 0 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-10-20 14:51 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Thu, 20 Oct 2016 15:33:27 +0100 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Fri, Oct 21, 2016 at 01:20:17AM +1100, Nicholas Piggin wrote: > > Good catch, I'm surprised you're the first one who reported it. This patch > > seems to do the trick for me: > > And me, thanks, so... > > > > > From: Nicholas Piggin <npiggin@gmail.com> > > Date: Fri, 21 Oct 2016 01:13:33 +1100 > > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Reported-by: Russell King <rmk+kernel@armlinux.org.uk> > Tested-by: Russell King <rmk+kernel@armlinux.org.uk> Thanks for testing it. Hopefully if Arnd is able to respin the ARM patch to something a bit more to your liking that doesn't expose prototypes, and no other problems arise, we can avoid reverting. You had a poor first impression, but we may yet win you over. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 14:33 ` Russell King - ARM Linux 2016-10-20 14:51 ` Nicholas Piggin @ 2016-10-22 19:51 ` Michal Marek 1 sibling, 0 replies; 58+ messages in thread From: Michal Marek @ 2016-10-22 19:51 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Nicholas Piggin, Arnd Bergmann, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Thu, Oct 20, 2016 at 03:33:27PM +0100, Russell King - ARM Linux wrote: > On Fri, Oct 21, 2016 at 01:20:17AM +1100, Nicholas Piggin wrote: > > Good catch, I'm surprised you're the first one who reported it. This patch > > seems to do the trick for me: > > And me, thanks, so... > > > > > From: Nicholas Piggin <npiggin@gmail.com> > > Date: Fri, 21 Oct 2016 01:13:33 +1100 > > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Reported-by: Russell King <rmk+kernel@armlinux.org.uk> > Tested-by: Russell King <rmk+kernel@armlinux.org.uk> Thanks. Added to kbuild.git#rc-fixes. Michal ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 4:08 ` Nicholas Piggin 2016-10-20 13:17 ` Russell King - ARM Linux @ 2016-10-24 15:04 ` Arnd Bergmann 2016-10-24 15:05 ` [PATCH 1/2] " Arnd Bergmann ` (2 more replies) 1 sibling, 3 replies; 58+ messages in thread From: Arnd Bergmann @ 2016-10-24 15:04 UTC (permalink / raw) To: Nicholas Piggin Cc: Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Thursday, October 20, 2016 3:08:14 PM CEST Nicholas Piggin wrote: > On Wed, 19 Oct 2016 16:32:00 +0100 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > > I'm not in favour of this. > > > > +extern void mmioset(void *, unsigned int, size_t); > > +extern void mmiocpy(void *, const void *, size_t); > > + > > #ifndef __ARMBE__ > > static inline void memset_io(volatile void __iomem *dst, unsigned c, > > size_t count) > > { > > - extern void mmioset(void *, unsigned int, size_t); > > mmioset((void __force *)dst, c, count); > > } > > > > The reason they're declared _within_ memset_io() is to prevent people > > from using them by hiding their declaration. Moving them outside is > > an open invitation to stupid people starting to use them as an "oh it > > must be an official API". > > I've split out that change from the other ones now, and will follow up with the patch to address all the other ones first. > Fair point, what about leaving those as they are, and also adding > them to asm-prototypes.h protected with GENKSYMS ifdef? It's not > beautiful, but still better than armksyms.c before Al's patches (or > at least no worse). I'm trying this one, and an alternative patch that moves the export into arch/arm/kernel/io.h. Let's see if we can agree on one of these. Arnd ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-24 15:04 ` Arnd Bergmann @ 2016-10-24 15:05 ` Arnd Bergmann 2016-10-25 8:32 ` Nicholas Piggin 2016-11-21 18:46 ` Bug#844530: " Uwe Kleine-König 2016-10-24 15:06 ` [PATCH 2/2, variant A] ARM: add hidden mmioset/mmiocpy prototypes Arnd Bergmann 2016-10-24 15:06 ` [PATCH 2/2, variant B] ARM: move mmiocpy/mmioset exports to io.c Arnd Bergmann 2 siblings, 2 replies; 58+ messages in thread From: Arnd Bergmann @ 2016-10-24 15:05 UTC (permalink / raw) To: Nicholas Piggin Cc: Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch This adds an asm/asm-prototypes.h header for ARM to fix the broken symbol versioning for symbols exported from assembler files. In addition to the header, we have to do these other small changes: - move the exports from bitops.h to {change,clear,set,...}bit.S - move the exports from csumpartialgeneric.S into the files including it 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. This leaves the mmioset/mmiocpy function for now, as it's not obvious how to best handle them. Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/arm/include/asm/asm-prototypes.h b/arch/arm/include/asm/asm-prototypes.h new file mode 100644 index 000000000000..04e5616a7b15 --- /dev/null +++ b/arch/arm/include/asm/asm-prototypes.h @@ -0,0 +1,34 @@ +#include <linux/arm-smccc.h> +#include <linux/bitops.h> +#include <linux/ftrace.h> +#include <linux/io.h> +#include <linux/platform_data/asoc-imx-ssi.h> +#include <linux/string.h> +#include <linux/uaccess.h> + +#include <asm/checksum.h> +#include <asm/div64.h> +#include <asm/memory.h> + +extern void __aeabi_idivmod(void); +extern void __aeabi_idiv(void); +extern void __aeabi_lasr(void); +extern void __aeabi_llsl(void); +extern void __aeabi_llsr(void); +extern void __aeabi_lmul(void); +extern void __aeabi_uidivmod(void); +extern void __aeabi_uidiv(void); +extern void __aeabi_ulcmp(void); + +extern void __ashldi3(void); +extern void __ashrdi3(void); +extern void __bswapdi2(void); +extern void __bswapsi2(void); +extern void __divsi3(void); +extern void __do_div64(void); +extern void __lshrdi3(void); +extern void __modsi3(void); +extern void __muldi3(void); +extern void __ucmpdi2(void); +extern void __udivsi3(void); +extern void __umodsi3(void); diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h index df06638b327c..afaef2a7faec 100644 --- a/arch/arm/lib/bitops.h +++ b/arch/arm/lib/bitops.h @@ -26,7 +26,6 @@ UNWIND( .fnstart ) bx lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm .macro testop, name, instr, store @@ -57,7 +56,6 @@ UNWIND( .fnstart ) 2: bx lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm #else .macro bitop, name, instr @@ -77,7 +75,6 @@ UNWIND( .fnstart ) ret lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm /** @@ -106,6 +103,5 @@ UNWIND( .fnstart ) ret lr UNWIND( .fnend ) ENDPROC(\name ) -EXPORT_SYMBOL(\name ) .endm #endif diff --git a/arch/arm/lib/changebit.S b/arch/arm/lib/changebit.S index f4027862172f..005fdd18c509 100644 --- a/arch/arm/lib/changebit.S +++ b/arch/arm/lib/changebit.S @@ -13,3 +13,4 @@ .text bitop _change_bit, eor +EXPORT_SYMBOL(_change_bit) diff --git a/arch/arm/lib/clearbit.S b/arch/arm/lib/clearbit.S index f6b75fb64d30..501eff09968d 100644 --- a/arch/arm/lib/clearbit.S +++ b/arch/arm/lib/clearbit.S @@ -13,3 +13,4 @@ .text bitop _clear_bit, bic +EXPORT_SYMBOL(_clear_bit) diff --git a/arch/arm/lib/csumpartialcopy.S b/arch/arm/lib/csumpartialcopy.S index 9c3383fed129..bdcc2eea4e5c 100644 --- a/arch/arm/lib/csumpartialcopy.S +++ b/arch/arm/lib/csumpartialcopy.S @@ -49,6 +49,7 @@ #define FN_ENTRY ENTRY(csum_partial_copy_nocheck) #define FN_EXIT ENDPROC(csum_partial_copy_nocheck) -#define FN_EXPORT EXPORT_SYMBOL(csum_partial_copy_nocheck) #include "csumpartialcopygeneric.S" + +EXPORT_SYMBOL(csum_partial_copy_nocheck) diff --git a/arch/arm/lib/csumpartialcopygeneric.S b/arch/arm/lib/csumpartialcopygeneric.S index 8b94d20e51d1..06825566c0f7 100644 --- a/arch/arm/lib/csumpartialcopygeneric.S +++ b/arch/arm/lib/csumpartialcopygeneric.S @@ -332,4 +332,3 @@ FN_ENTRY mov r5, r4, get_byte_1 b .Lexit FN_EXIT -FN_EXPORT diff --git a/arch/arm/lib/csumpartialcopyuser.S b/arch/arm/lib/csumpartialcopyuser.S index 5d495edf3d83..d5522c94f58c 100644 --- a/arch/arm/lib/csumpartialcopyuser.S +++ b/arch/arm/lib/csumpartialcopyuser.S @@ -73,9 +73,9 @@ #define FN_ENTRY ENTRY(csum_partial_copy_from_user) #define FN_EXIT ENDPROC(csum_partial_copy_from_user) -#define FN_EXPORT EXPORT_SYMBOL(csum_partial_copy_from_user) #include "csumpartialcopygeneric.S" +EXPORT_SYMBOL(csum_partial_copy_from_user) /* * FIXME: minor buglet here diff --git a/arch/arm/lib/setbit.S b/arch/arm/lib/setbit.S index 618fedae4b37..d748b8d1326f 100644 --- a/arch/arm/lib/setbit.S +++ b/arch/arm/lib/setbit.S @@ -13,3 +13,4 @@ .text bitop _set_bit, orr +EXPORT_SYMBOL(_set_bit) diff --git a/arch/arm/lib/testchangebit.S b/arch/arm/lib/testchangebit.S index 4becdc3a59cb..4d2dafa9b787 100644 --- a/arch/arm/lib/testchangebit.S +++ b/arch/arm/lib/testchangebit.S @@ -13,3 +13,4 @@ .text testop _test_and_change_bit, eor, str +EXPORT_SYMBOL(_test_and_change_bit) diff --git a/arch/arm/lib/testclearbit.S b/arch/arm/lib/testclearbit.S index 918841dcce7a..fe5cae2e480a 100644 --- a/arch/arm/lib/testclearbit.S +++ b/arch/arm/lib/testclearbit.S @@ -13,3 +13,4 @@ .text testop _test_and_clear_bit, bicne, strne +EXPORT_SYMBOL(_test_and_clear_bit) diff --git a/arch/arm/lib/testsetbit.S b/arch/arm/lib/testsetbit.S index 8d1b2fe9e487..25fed837edb3 100644 --- a/arch/arm/lib/testsetbit.S +++ b/arch/arm/lib/testsetbit.S @@ -13,3 +13,4 @@ .text testop _test_and_set_bit, orreq, streq +EXPORT_SYMBOL(_test_and_set_bit) ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-24 15:05 ` [PATCH 1/2] " Arnd Bergmann @ 2016-10-25 8:32 ` Nicholas Piggin 2016-11-20 13:21 ` Russell King - ARM Linux 2016-11-21 18:46 ` Bug#844530: " Uwe Kleine-König 1 sibling, 1 reply; 58+ messages in thread From: Nicholas Piggin @ 2016-10-25 8:32 UTC (permalink / raw) To: Arnd Bergmann Cc: Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Mon, 24 Oct 2016 17:05:26 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > This adds an asm/asm-prototypes.h header for ARM to fix the > broken symbol versioning for symbols exported from assembler > files. > > In addition to the header, we have to do these other small > changes: > > - move the exports from bitops.h to {change,clear,set,...}bit.S > - move the exports from csumpartialgeneric.S into the files > including it > > 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. > > This leaves the mmioset/mmiocpy function for now, as it's not > obvious how to best handle them. This looks nicer. I like variant B because it keeps the GENKSYMS cruft to a single location, but either one isn't too bad. I'd like to get moving on this, so let's at least get the generic kbuild change merged. In the end, the kbuild code does not prevent a maintainer from putting their EXPORT_SYMBOL in whatever location they like, so there is no reason not to merge it (certainly there will be archs that do use it). Michal, what's your thoughts? If you merge my patch 2/2 and skip 1/2, it should not give any new build warnings or errors, so then arch patches can go via arch trees. 1/2 could go in after everyone is up to date. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-25 8:32 ` Nicholas Piggin @ 2016-11-20 13:21 ` Russell King - ARM Linux 2016-11-20 18:32 ` Linus Torvalds 0 siblings, 1 reply; 58+ messages in thread From: Russell King - ARM Linux @ 2016-11-20 13:21 UTC (permalink / raw) To: Nicholas Piggin Cc: Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch On Tue, Oct 25, 2016 at 07:32:00PM +1100, Nicholas Piggin wrote: > On Mon, 24 Oct 2016 17:05:26 +0200 > Arnd Bergmann <arnd@arndb.de> wrote: > > > This adds an asm/asm-prototypes.h header for ARM to fix the > > broken symbol versioning for symbols exported from assembler > > files. > > > > In addition to the header, we have to do these other small > > changes: > > > > - move the exports from bitops.h to {change,clear,set,...}bit.S > > - move the exports from csumpartialgeneric.S into the files > > including it > > > > 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. > > > > This leaves the mmioset/mmiocpy function for now, as it's not > > obvious how to best handle them. > > > This looks nicer. I like variant B because it keeps the GENKSYMS cruft to > a single location, but either one isn't too bad. > > I'd like to get moving on this, so let's at least get the generic kbuild > change merged. In the end, the kbuild code does not prevent a maintainer > from putting their EXPORT_SYMBOL in whatever location they like, so there > is no reason not to merge it (certainly there will be archs that do use > it). > > Michal, what's your thoughts? If you merge my patch 2/2 and skip 1/2, it > should not give any new build warnings or errors, so then arch patches can > go via arch trees. 1/2 could go in after everyone is up to date. So what's the conclusion on this? I've just had a failure due to CONFIG_TRIM_UNUSED_KSYMS reported on ARM, and it looks like (at least some of) patch 1 could resolve it. Do we need to split patch 1? Has any of these patches been committed yet? -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-11-20 13:21 ` Russell King - ARM Linux @ 2016-11-20 18:32 ` Linus Torvalds 2016-11-20 19:12 ` Russell King - ARM Linux 0 siblings, 1 reply; 58+ messages in thread From: Linus Torvalds @ 2016-11-20 18:32 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Nicholas Piggin, Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On Sun, Nov 20, 2016 at 5:21 AM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 25, 2016 at 07:32:00PM +1100, Nicholas Piggin wrote: >> >> Michal, what's your thoughts? If you merge my patch 2/2 and skip 1/2, it >> should not give any new build warnings or errors, so then arch patches can >> go via arch trees. 1/2 could go in after everyone is up to date. > > So what's the conclusion on this? I've just had a failure due to > CONFIG_TRIM_UNUSED_KSYMS reported on ARM, and it looks like (at > least some of) patch 1 could resolve it. Hmm. I've got cc6acc11cad1 kbuild: be more careful about matching preprocessed asm ___EXPORT_SYMBOL 4efca4ed05cb kbuild: modversions for EXPORT_SYMBOL() for asm in my tree. Is that sufficient, or do we still have issues? Linus ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-11-20 18:32 ` Linus Torvalds @ 2016-11-20 19:12 ` Russell King - ARM Linux 2016-11-21 6:10 ` Nicholas Piggin 0 siblings, 1 reply; 58+ messages in thread From: Russell King - ARM Linux @ 2016-11-20 19:12 UTC (permalink / raw) To: Linus Torvalds Cc: Nicholas Piggin, Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On Sun, Nov 20, 2016 at 10:32:50AM -0800, Linus Torvalds wrote: > On Sun, Nov 20, 2016 at 5:21 AM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > On Tue, Oct 25, 2016 at 07:32:00PM +1100, Nicholas Piggin wrote: > >> > >> Michal, what's your thoughts? If you merge my patch 2/2 and skip 1/2, it > >> should not give any new build warnings or errors, so then arch patches can > >> go via arch trees. 1/2 could go in after everyone is up to date. > > > > So what's the conclusion on this? I've just had a failure due to > > CONFIG_TRIM_UNUSED_KSYMS reported on ARM, and it looks like (at > > least some of) patch 1 could resolve it. > > Hmm. I've got > > cc6acc11cad1 kbuild: be more careful about matching preprocessed asm > ___EXPORT_SYMBOL > 4efca4ed05cb kbuild: modversions for EXPORT_SYMBOL() for asm > > in my tree. Is that sufficient, or do we still have issues? Hmm, those seem to have gone in during the last week, so I haven't tested it yet (build running, but it'll take a while). However, I don't think they'll solve _this_ problem. Some of the issue here is that we use a mixture of assembly macros and preprocessor for the ARM bitops - the ARM bitops are created with an assembly macro which contains some pre-processor expanded macros (eg, EXPORT_SYMBOL()). This means that the actual symbol being exported is not known to the preprocessor, so doing the "__is_defined(__KSYM_##sym)" inside "EXPORT_SYMBOL(\name)" becomes "__is_defined(__KSYM_\name)" to the preprocessor. As "__KSYM_\name" is never defined, it always comes out as zero, hence we always use __cond_export_sym_0, which omits the symbol export from the assembly macro definition: .macro bitop, name, instr .globl \name ; .align 0 ; \name: ... .type \name, %function; .size \name, .-\name .endm In other words, using preprocessor macros inside an assembly macro may not work as expected, and now leads to config-specific failures. -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-11-20 19:12 ` Russell King - ARM Linux @ 2016-11-21 6:10 ` Nicholas Piggin 0 siblings, 0 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-11-21 6:10 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Linus Torvalds, Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Alexey Dobriyan, Stephen Rothwell, Al Viro, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-arch On Sun, 20 Nov 2016 19:12:57 +0000 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Sun, Nov 20, 2016 at 10:32:50AM -0800, Linus Torvalds wrote: > > On Sun, Nov 20, 2016 at 5:21 AM, Russell King - ARM Linux > > <linux@armlinux.org.uk> wrote: > > > On Tue, Oct 25, 2016 at 07:32:00PM +1100, Nicholas Piggin wrote: > > >> > > >> Michal, what's your thoughts? If you merge my patch 2/2 and skip 1/2, it > > >> should not give any new build warnings or errors, so then arch patches can > > >> go via arch trees. 1/2 could go in after everyone is up to date. > > > > > > So what's the conclusion on this? I've just had a failure due to > > > CONFIG_TRIM_UNUSED_KSYMS reported on ARM, and it looks like (at > > > least some of) patch 1 could resolve it. > > > > Hmm. I've got > > > > cc6acc11cad1 kbuild: be more careful about matching preprocessed asm > > ___EXPORT_SYMBOL > > 4efca4ed05cb kbuild: modversions for EXPORT_SYMBOL() for asm > > > > in my tree. Is that sufficient, or do we still have issues? > > Hmm, those seem to have gone in during the last week, so I haven't > tested it yet (build running, but it'll take a while). However, I > don't think they'll solve _this_ problem. > > Some of the issue here is that we use a mixture of assembly macros > and preprocessor for the ARM bitops - the ARM bitops are created > with an assembly macro which contains some pre-processor expanded > macros (eg, EXPORT_SYMBOL()). > > This means that the actual symbol being exported is not known to > the preprocessor, so doing the "__is_defined(__KSYM_##sym)" inside > "EXPORT_SYMBOL(\name)" becomes "__is_defined(__KSYM_\name)" to the > preprocessor. As "__KSYM_\name" is never defined, it always comes > out as zero, hence we always use __cond_export_sym_0, which omits > the symbol export from the assembly macro definition: > > .macro bitop, name, instr > .globl \name ; .align 0 ; \name: > > ... > > .type \name, %function; .size \name, .-\name > > .endm > > In other words, using preprocessor macros inside an assembly macro > may not work as expected, and now leads to config-specific failures. > Yes, that's a limitation. cpp expansion we can handle, but not gas macros. You will need Arnd's patches for ARM. http://marc.info/?l=linux-kbuild&m=147732160529499&w=2 If that doesn't fix it for you, send me your .config offline and I'll set up a cross compile to work on it. Again, any arch always has the option of going back to doing asm exports in the old style of putting them into a .c file, but hopefully you'll find Arnd's reworked patches to be something you're willing to merge. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-24 15:05 ` [PATCH 1/2] " Arnd Bergmann @ 2016-11-21 18:46 ` Uwe Kleine-König 2016-11-21 18:46 ` Bug#844530: " Uwe Kleine-König 1 sibling, 0 replies; 58+ messages in thread From: Uwe Kleine-König @ 2016-11-21 18:46 UTC (permalink / raw) To: Arnd Bergmann Cc: Nicholas Piggin, Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, 844530 [-- Attachment #1: Type: text/plain, Size: 1616 bytes --] Hello, On Mon, Oct 24, 2016 at 05:05:26PM +0200, 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. > > In addition to the header, we have to do these other small > changes: > > - move the exports from bitops.h to {change,clear,set,...}bit.S > - move the exports from csumpartialgeneric.S into the files > including it > > 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. > > This leaves the mmioset/mmiocpy function for now, as it's not > obvious how to best handle them. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> In my test builds of 4.9-rc5 plus 4efca4ed05cb ("kbuild: modversions for EXPORT_SYMBOL() for asm") cc6acc11cad1 ("kbuild: be more careful about matching preprocessed asm ___EXPORT_SYMBOL") (which are in -rc6) I got many warnings à la: WARNING: "memset" [drivers/media/usb/airspy/airspy.ko] has no CRC! and booting the resulting kernel failed with messages of the type: [ 3.024126] usbcore: no symbol version for __memzero [ 3.029107] usbcore: Unknown symbol __memzero (err -22) so hardly any module could be loaded. modprobe -f works however, but that's not what my initramfs does. With this patch and https://patchwork.kernel.org/patch/9392291/ ("ARM: move mmiocpy/mmioset exports to io.c") I could compile a kernel without CRC warnings and it boots fine. So it would be great to get these two patches into 4.9. Thanks Uwe [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Bug#844530: [1/2] kbuild: provide include/asm/asm-prototypes.h for ARM @ 2016-11-21 18:46 ` Uwe Kleine-König 0 siblings, 0 replies; 58+ messages in thread From: Uwe Kleine-König @ 2016-11-21 18:46 UTC (permalink / raw) To: Arnd Bergmann Cc: Nicholas Piggin, Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, 844530 [-- Attachment #1: Type: text/plain, Size: 1616 bytes --] Hello, On Mon, Oct 24, 2016 at 05:05:26PM +0200, 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. > > In addition to the header, we have to do these other small > changes: > > - move the exports from bitops.h to {change,clear,set,...}bit.S > - move the exports from csumpartialgeneric.S into the files > including it > > 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. > > This leaves the mmioset/mmiocpy function for now, as it's not > obvious how to best handle them. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> In my test builds of 4.9-rc5 plus 4efca4ed05cb ("kbuild: modversions for EXPORT_SYMBOL() for asm") cc6acc11cad1 ("kbuild: be more careful about matching preprocessed asm ___EXPORT_SYMBOL") (which are in -rc6) I got many warnings à la: WARNING: "memset" [drivers/media/usb/airspy/airspy.ko] has no CRC! and booting the resulting kernel failed with messages of the type: [ 3.024126] usbcore: no symbol version for __memzero [ 3.029107] usbcore: Unknown symbol __memzero (err -22) so hardly any module could be loaded. modprobe -f works however, but that's not what my initramfs does. With this patch and https://patchwork.kernel.org/patch/9392291/ ("ARM: move mmiocpy/mmioset exports to io.c") I could compile a kernel without CRC warnings and it boots fine. So it would be great to get these two patches into 4.9. Thanks Uwe [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-11-21 18:46 ` Bug#844530: " Uwe Kleine-König (?) @ 2016-11-21 19:13 ` Russell King - ARM Linux 2016-11-22 1:01 ` Nicholas Piggin -1 siblings, 1 reply; 58+ messages in thread From: Russell King - ARM Linux @ 2016-11-21 19:13 UTC (permalink / raw) To: Uwe Kleine-König Cc: Arnd Bergmann, Nicholas Piggin, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, 844530 On Mon, Nov 21, 2016 at 07:46:44PM +0100, Uwe Kleine-König wrote: > Hello, > > On Mon, Oct 24, 2016 at 05:05:26PM +0200, 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. > > > > In addition to the header, we have to do these other small > > changes: > > > > - move the exports from bitops.h to {change,clear,set,...}bit.S > > - move the exports from csumpartialgeneric.S into the files > > including it > > > > 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. > > > > This leaves the mmioset/mmiocpy function for now, as it's not > > obvious how to best handle them. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > In my test builds of 4.9-rc5 plus > > 4efca4ed05cb ("kbuild: modversions for EXPORT_SYMBOL() for asm") > cc6acc11cad1 ("kbuild: be more careful about matching preprocessed asm ___EXPORT_SYMBOL") > > (which are in -rc6) I got many warnings à la: > > WARNING: "memset" [drivers/media/usb/airspy/airspy.ko] has no CRC! > > and booting the resulting kernel failed with messages of the type: > > [ 3.024126] usbcore: no symbol version for __memzero > [ 3.029107] usbcore: Unknown symbol __memzero (err -22) > > so hardly any module could be loaded. modprobe -f works however, but > that's not what my initramfs does. > > With this patch and https://patchwork.kernel.org/patch/9392291/ ("ARM: > move mmiocpy/mmioset exports to io.c") I could compile a kernel without > CRC warnings and it boots fine. So it would be great to get these two > patches into 4.9. Yea, many things would be nice, but I've been unable to track the issues here - it really didn't help _not_ being copied on the original set of patches which introduced this mess. I've merged Nicolas' patch, so now we need to work out what to do with the remaining bits - which I guess are the asm-prototypes.h and the mmio* bits. I'm not aware of what's happening with the patches that they depend on (which is why I recently asked the question - again, I seem to be completely out of the loop due to lack of Cc's). So I'm just throwing my hands up and saying "I don't know what to do" at this stage. -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [1/2] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-11-21 19:13 ` Russell King - ARM Linux @ 2016-11-22 1:01 ` Nicholas Piggin 0 siblings, 0 replies; 58+ messages in thread From: Nicholas Piggin @ 2016-11-22 1:01 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Uwe Kleine-König, Arnd Bergmann, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch, 844530 On Mon, 21 Nov 2016 19:13:55 +0000 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Mon, Nov 21, 2016 at 07:46:44PM +0100, Uwe Kleine-König wrote: > > Hello, > > > > On Mon, Oct 24, 2016 at 05:05:26PM +0200, 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. > > > > > > In addition to the header, we have to do these other small > > > changes: > > > > > > - move the exports from bitops.h to {change,clear,set,...}bit.S > > > - move the exports from csumpartialgeneric.S into the files > > > including it > > > > > > 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. > > > > > > This leaves the mmioset/mmiocpy function for now, as it's not > > > obvious how to best handle them. > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > In my test builds of 4.9-rc5 plus > > > > 4efca4ed05cb ("kbuild: modversions for EXPORT_SYMBOL() for asm") > > cc6acc11cad1 ("kbuild: be more careful about matching preprocessed asm ___EXPORT_SYMBOL") > > > > (which are in -rc6) I got many warnings à la: > > > > WARNING: "memset" [drivers/media/usb/airspy/airspy.ko] has no CRC! > > > > and booting the resulting kernel failed with messages of the type: > > > > [ 3.024126] usbcore: no symbol version for __memzero > > [ 3.029107] usbcore: Unknown symbol __memzero (err -22) > > > > so hardly any module could be loaded. modprobe -f works however, but > > that's not what my initramfs does. > > > > With this patch and https://patchwork.kernel.org/patch/9392291/ ("ARM: > > move mmiocpy/mmioset exports to io.c") I could compile a kernel without > > CRC warnings and it boots fine. So it would be great to get these two > > patches into 4.9. > > Yea, many things would be nice, but I've been unable to track the > issues here - it really didn't help _not_ being copied on the > original set of patches which introduced this mess. Quick overview: - asm exports changes allow EXPORT_SYMBOL to be moved into .S files, but they would not get modversion CRCs generated. - The core kbuild patches to add modversions support for asm exports is now merged in Linus's tree from the recent kbuild tree pull. asm/asm-prototypes.h must contain C style declarations of the symbol for this to work. - Architectures can now add their asm/asm-prototypes.h and things *should* start working. - Dependency is not a hard one. If you add asm-prototypes.h before merging the core patches then it should not introduce any problems. > I've merged Nicolas' patch, so now we need to work out what to do > with the remaining bits - which I guess are the asm-prototypes.h > and the mmio* bits. I'm not aware of what's happening with the > patches that they depend on (which is why I recently asked the > question - again, I seem to be completely out of the loop due to > lack of Cc's). > > So I'm just throwing my hands up and saying "I don't know what to > do" at this stage. > I don't think you have missed much since last it came up, it's just taken a bit of time to get the details right and get it merged. Not sure what your tree looks like, but if you merge this patch 1/2 plus 2a/2 or 2b/2 into upstream, then ARM should be working. Thanks, Nick ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 2/2, variant A] ARM: add hidden mmioset/mmiocpy prototypes 2016-10-24 15:04 ` Arnd Bergmann 2016-10-24 15:05 ` [PATCH 1/2] " Arnd Bergmann @ 2016-10-24 15:06 ` Arnd Bergmann 2016-10-24 15:06 ` [PATCH 2/2, variant B] ARM: move mmiocpy/mmioset exports to io.c Arnd Bergmann 2 siblings, 0 replies; 58+ messages in thread From: Arnd Bergmann @ 2016-10-24 15:06 UTC (permalink / raw) To: Nicholas Piggin Cc: Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch The prototypes for mmioset/mmiocpy are intentionally hidden inside of inline functions, which breaks the EXPORT_SYMBOL statements when symbol versioning is enabled. This adds a prototype to asm/asm-prototypes.h but hides it in an #ifdef so normal drivers don't see it and won't be able to abuse the interface. Suggested-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/arm/include/asm/asm-prototypes.h b/arch/arm/include/asm/asm-prototypes.h index 04e5616a7b15..e46b09536b14 100644 --- a/arch/arm/include/asm/asm-prototypes.h +++ b/arch/arm/include/asm/asm-prototypes.h @@ -32,3 +32,8 @@ extern void __muldi3(void); extern void __ucmpdi2(void); extern void __udivsi3(void); extern void __umodsi3(void); + +#ifdef __GENKSYMS__ +extern void mmioset(void *, unsigned int, size_t); +extern void mmiocpy(void *, const void *, size_t); +#endif ^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 2/2, variant B] ARM: move mmiocpy/mmioset exports to io.c 2016-10-24 15:04 ` Arnd Bergmann 2016-10-24 15:05 ` [PATCH 1/2] " Arnd Bergmann 2016-10-24 15:06 ` [PATCH 2/2, variant A] ARM: add hidden mmioset/mmiocpy prototypes Arnd Bergmann @ 2016-10-24 15:06 ` Arnd Bergmann 2 siblings, 0 replies; 58+ messages in thread From: Arnd Bergmann @ 2016-10-24 15:06 UTC (permalink / raw) To: Nicholas Piggin Cc: Russell King - ARM Linux, Michal Marek, Adam Borowski, Omar Sandoval, Linus Torvalds, adobriyan, sfr, viro, linux-kbuild, linux-kernel, linux-arch The prototypes for mmioset/mmiocpy are intentionally hidden inside of inline functions, which breaks the EXPORT_SYMBOL statements when symbol versioning is enabled. This moves the two exports from the files that implement the code into the kernel/io.c file, adding another local declaration there. Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/arm/kernel/io.c b/arch/arm/kernel/io.c index eedefe050022..c74746997626 100644 --- a/arch/arm/kernel/io.c +++ b/arch/arm/kernel/io.c @@ -82,3 +82,10 @@ void _memset_io(volatile void __iomem *dst, int c, size_t count) } } EXPORT_SYMBOL(_memset_io); + +/* can't export them from memcpy.S/memset.S because of hidden declaration */ +void mmioset(void __iomem *addr, unsigned int c, size_t n); +EXPORT_SYMBOL(mmioset); + +void mmiocpy(void *dest, const void __iomem *src, size_t n); +EXPORT_SYMBOL(mmiocpy); diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S index 1be5b6ddf37c..1f822fc52400 100644 --- a/arch/arm/lib/memcpy.S +++ b/arch/arm/lib/memcpy.S @@ -70,4 +70,3 @@ ENTRY(memcpy) ENDPROC(memcpy) ENDPROC(mmiocpy) EXPORT_SYMBOL(memcpy) -EXPORT_SYMBOL(mmiocpy) diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S index 7b72044cba62..6f075ca09abc 100644 --- a/arch/arm/lib/memset.S +++ b/arch/arm/lib/memset.S @@ -137,4 +137,3 @@ UNWIND( .fnend ) ENDPROC(memset) ENDPROC(mmioset) EXPORT_SYMBOL(memset) -EXPORT_SYMBOL(mmioset) ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-19 15:32 ` Russell King - ARM Linux 2016-10-20 4:08 ` Nicholas Piggin @ 2016-10-20 7:37 ` Geert Uytterhoeven 2016-10-20 8:20 ` Russell King - ARM Linux 1 sibling, 1 reply; 58+ messages in thread From: Geert Uytterhoeven @ 2016-10-20 7:37 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Michal Marek, Nicholas Piggin, Adam Borowski, Omar Sandoval, Linus Torvalds, Alexey Dobriyan, Stephen Rothwell, Al Viro, linux-kbuild, linux-kernel, Linux-Arch On Wed, Oct 19, 2016 at 5:32 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > I'm not in favour of this. > > +extern void mmioset(void *, unsigned int, size_t); > +extern void mmiocpy(void *, const void *, size_t); > + > #ifndef __ARMBE__ > static inline void memset_io(volatile void __iomem *dst, unsigned c, > size_t count) > { > - extern void mmioset(void *, unsigned int, size_t); > mmioset((void __force *)dst, c, count); > } > > The reason they're declared _within_ memset_io() is to prevent people > from using them by hiding their declaration. Moving them outside is > an open invitation to stupid people starting to use them as an "oh it > must be an official API". If they're not intended for public use, they should (also) be prefixed with "__" or even "____" to make this clear. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 7:37 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven @ 2016-10-20 8:20 ` Russell King - ARM Linux 2016-10-20 8:23 ` Geert Uytterhoeven 0 siblings, 1 reply; 58+ messages in thread From: Russell King - ARM Linux @ 2016-10-20 8:20 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Arnd Bergmann, Michal Marek, Nicholas Piggin, Adam Borowski, Omar Sandoval, Linus Torvalds, Alexey Dobriyan, Stephen Rothwell, Al Viro, linux-kbuild, linux-kernel, Linux-Arch On Thu, Oct 20, 2016 at 09:37:30AM +0200, Geert Uytterhoeven wrote: > On Wed, Oct 19, 2016 at 5:32 PM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > I'm not in favour of this. > > > > +extern void mmioset(void *, unsigned int, size_t); > > +extern void mmiocpy(void *, const void *, size_t); > > + > > #ifndef __ARMBE__ > > static inline void memset_io(volatile void __iomem *dst, unsigned c, > > size_t count) > > { > > - extern void mmioset(void *, unsigned int, size_t); > > mmioset((void __force *)dst, c, count); > > } > > > > The reason they're declared _within_ memset_io() is to prevent people > > from using them by hiding their declaration. Moving them outside is > > an open invitation to stupid people starting to use them as an "oh it > > must be an official API". > > If they're not intended for public use, they should (also) be prefixed > with "__" or even "____" to make this clear. Tried that with the __cpuc_* cache flushing interfaces. It doesn't have any effect what so ever. -- 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. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM 2016-10-20 8:20 ` Russell King - ARM Linux @ 2016-10-20 8:23 ` Geert Uytterhoeven 0 siblings, 0 replies; 58+ messages in thread From: Geert Uytterhoeven @ 2016-10-20 8:23 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Michal Marek, Nicholas Piggin, Adam Borowski, Omar Sandoval, Linus Torvalds, Alexey Dobriyan, Stephen Rothwell, Al Viro, linux-kbuild, linux-kernel, Linux-Arch Hi Russell, On Thu, Oct 20, 2016 at 10:20 AM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Thu, Oct 20, 2016 at 09:37:30AM +0200, Geert Uytterhoeven wrote: >> On Wed, Oct 19, 2016 at 5:32 PM, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > I'm not in favour of this. >> > >> > +extern void mmioset(void *, unsigned int, size_t); >> > +extern void mmiocpy(void *, const void *, size_t); >> > + >> > #ifndef __ARMBE__ >> > static inline void memset_io(volatile void __iomem *dst, unsigned c, >> > size_t count) >> > { >> > - extern void mmioset(void *, unsigned int, size_t); >> > mmioset((void __force *)dst, c, count); >> > } >> > >> > The reason they're declared _within_ memset_io() is to prevent people >> > from using them by hiding their declaration. Moving them outside is >> > an open invitation to stupid people starting to use them as an "oh it >> > must be an official API". >> >> If they're not intended for public use, they should (also) be prefixed >> with "__" or even "____" to make this clear. > > Tried that with the __cpuc_* cache flushing interfaces. It doesn't > have any effect what so ever. it may not stop the deliberate abuser, but it hints the casual reviewer. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 58+ messages in thread
end of thread, other threads:[~2016-12-18 15:08 UTC | newest] Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-17 6:51 [GIT PULL] kbuild changes for v4.9-rc1 Adam Borowski 2016-10-17 6:59 ` Nicholas Piggin 2016-10-17 10:01 ` Adam Borowski 2016-10-17 10:01 ` Adam Borowski 2016-10-17 10:01 ` Adam Borowski 2016-10-17 11:12 ` Alexey Dobriyan 2016-10-17 11:17 ` Geert Uytterhoeven 2016-10-17 11:32 ` Alexey Dobriyan 2016-10-17 12:22 ` Mathieu OTHACEHE 2016-10-18 0:16 ` Adam Borowski 2016-10-18 1:34 ` Nicholas Piggin 2016-10-19 14:38 ` Michal Marek 2016-10-20 3:52 ` Nicholas Piggin 2016-10-27 8:10 ` Kalle Valo 2016-10-27 11:15 ` Nicholas Piggin 2016-10-27 13:14 ` Kalle Valo 2016-10-27 13:25 ` Nicholas Piggin 2016-10-30 10:51 ` Thorsten Leemhuis 2016-11-01 15:48 ` Michal Marek 2016-11-02 12:11 ` Adam Borowski 2016-11-02 12:14 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 Adam Borowski 2016-12-16 19:55 ` [GIT PULL] kbuild changes for v4.9-rc1 Jiri Slaby 2016-12-16 19:57 ` Linus Torvalds 2016-12-17 8:57 ` Jiri Slaby 2016-12-17 9:33 ` Adam Borowski 2016-12-17 23:59 ` Linus Torvalds 2016-12-18 10:49 ` Jiri Slaby 2016-12-18 11:03 ` Arend Van Spriel 2016-12-18 13:27 ` Nikolay Borisov 2016-12-18 14:45 ` Jiri Slaby 2016-12-18 14:54 ` Nikolay Borisov 2016-12-18 15:08 ` Jiri Slaby 2016-10-17 12:26 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann 2016-10-19 14:52 ` Michal Marek 2016-10-19 15:02 ` Arnd Bergmann 2016-10-19 15:32 ` Russell King - ARM Linux 2016-10-20 4:08 ` Nicholas Piggin 2016-10-20 13:17 ` Russell King - ARM Linux 2016-10-20 14:20 ` Nicholas Piggin 2016-10-20 14:33 ` Russell King - ARM Linux 2016-10-20 14:51 ` Nicholas Piggin 2016-10-22 19:51 ` Michal Marek 2016-10-24 15:04 ` Arnd Bergmann 2016-10-24 15:05 ` [PATCH 1/2] " Arnd Bergmann 2016-10-25 8:32 ` Nicholas Piggin 2016-11-20 13:21 ` Russell King - ARM Linux 2016-11-20 18:32 ` Linus Torvalds 2016-11-20 19:12 ` Russell King - ARM Linux 2016-11-21 6:10 ` Nicholas Piggin 2016-11-21 18:46 ` [1/2] " Uwe Kleine-König 2016-11-21 18:46 ` Bug#844530: " Uwe Kleine-König 2016-11-21 19:13 ` Russell King - ARM Linux 2016-11-22 1:01 ` Nicholas Piggin 2016-10-24 15:06 ` [PATCH 2/2, variant A] ARM: add hidden mmioset/mmiocpy prototypes Arnd Bergmann 2016-10-24 15:06 ` [PATCH 2/2, variant B] ARM: move mmiocpy/mmioset exports to io.c Arnd Bergmann 2016-10-20 7:37 ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven 2016-10-20 8:20 ` Russell King - ARM Linux 2016-10-20 8:23 ` Geert Uytterhoeven
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.