Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1501639 > unrolled thread

Re: [GIT PULL] kbuild changes for v4.9-rc1

Started byNicholas Piggin <npiggin@gmail.com>
First post2016-10-17 09:10 +0200
Last post2016-10-20 10:30 +0200
Articles 20 on this page of 28 — 8 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [GIT PULL] kbuild changes for v4.9-rc1 Nicholas Piggin <npiggin@gmail.com> - 2016-10-17 09:10 +0200
    Re: [GIT PULL] kbuild changes for v4.9-rc1 Adam Borowski <kilobyte@angband.pl> - 2016-10-17 12:10 +0200
      Re: [GIT PULL] kbuild changes for v4.9-rc1 Geert Uytterhoeven <geert@linux-m68k.org> - 2016-10-17 13:20 +0200
        Re: [GIT PULL] kbuild changes for v4.9-rc1 Alexey Dobriyan <adobriyan@gmail.com> - 2016-10-17 13:40 +0200
      Re: [GIT PULL] kbuild changes for v4.9-rc1 Alexey Dobriyan <adobriyan@gmail.com> - 2016-10-17 13:20 +0200
      Re: [GIT PULL] kbuild changes for v4.9-rc1 Mathieu OTHACEHE <m.othacehe@gmail.com> - 2016-10-17 14:30 +0200
        Re: [GIT PULL] kbuild changes for v4.9-rc1 Adam Borowski <kilobyte@angband.pl> - 2016-10-18 02:20 +0200
          Re: [GIT PULL] kbuild changes for v4.9-rc1 Nicholas Piggin <npiggin@gmail.com> - 2016-10-18 03:40 +0200
            Re: [GIT PULL] kbuild changes for v4.9-rc1 Michal Marek <mmarek@suse.com> - 2016-10-19 16:40 +0200
              Re: [GIT PULL] kbuild changes for v4.9-rc1 Nicholas Piggin <npiggin@gmail.com> - 2016-10-20 06:00 +0200
    [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann <arnd@arndb.de> - 2016-10-17 14:30 +0200
      Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Michal Marek <mmarek@suse.com> - 2016-10-19 17:00 +0200
        Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann <arnd@arndb.de> - 2016-10-19 17:10 +0200
          Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-10-19 17:40 +0200
            Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Nicholas Piggin <npiggin@gmail.com> - 2016-10-20 06:20 +0200
              Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-10-20 15:30 +0200
                Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Nicholas Piggin <npiggin@gmail.com> - 2016-10-20 16:30 +0200
                  Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-10-20 16:40 +0200
                    Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Nicholas Piggin <npiggin@gmail.com> - 2016-10-20 17:00 +0200
                    Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Michal Marek <mmarek@suse.com> - 2016-10-22 22:00 +0200
              [PATCH 2/2, variant A] ARM: add hidden mmioset/mmiocpy prototypes Arnd Bergmann <arnd@arndb.de> - 2016-10-24 17:10 +0200
              [PATCH 2/2, variant B] ARM: move mmiocpy/mmioset exports to io.c Arnd Bergmann <arnd@arndb.de> - 2016-10-24 17:10 +0200
              [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann <arnd@arndb.de> - 2016-10-24 17:10 +0200
                Re: [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for  ARM Nicholas Piggin <npiggin@gmail.com> - 2016-10-25 10:40 +0200
              Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann <arnd@arndb.de> - 2016-10-24 17:10 +0200
            Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven <geert@linux-m68k.org> - 2016-10-20 09:40 +0200
              Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-10-20 10:30 +0200
                Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven <geert@linux-m68k.org> - 2016-10-20 10:30 +0200

Page 1 of 2  [1] 2  Next page →


#1501639 — Re: [GIT PULL] kbuild changes for v4.9-rc1

FromNicholas Piggin <npiggin@gmail.com>
Date2016-10-17 09:10 +0200
SubjectRe: [GIT PULL] kbuild changes for v4.9-rc1
Message-ID<stadA-j1-5@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1501840

FromAdam Borowski <kilobyte@angband.pl>
Date2016-10-17 12:10 +0200
Message-ID<std1M-26b-23@gated-at.bofh.it>
In reply to#1501639
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

[toc] | [prev] | [next] | [standalone]


#1501888

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-10-17 13:20 +0200
Message-ID<ste7v-2Kh-23@gated-at.bofh.it>
In reply to#1501840
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

[toc] | [prev] | [next] | [standalone]


#1501898

FromAlexey Dobriyan <adobriyan@gmail.com>
Date2016-10-17 13:40 +0200
Message-ID<steqS-2QN-13@gated-at.bofh.it>
In reply to#1501888
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?

[toc] | [prev] | [next] | [standalone]


#1501890

FromAlexey Dobriyan <adobriyan@gmail.com>
Date2016-10-17 13:20 +0200
Message-ID<ste7v-2Kh-25@gated-at.bofh.it>
In reply to#1501840
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.

[toc] | [prev] | [next] | [standalone]


#1501927

FromMathieu OTHACEHE <m.othacehe@gmail.com>
Date2016-10-17 14:30 +0200
Message-ID<stfdf-3r0-7@gated-at.bofh.it>
In reply to#1501840
> +#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 ? 

[toc] | [prev] | [next] | [standalone]


#1502602

FromAdam Borowski <kilobyte@angband.pl>
Date2016-10-18 02:20 +0200
Message-ID<stqim-2Hw-19@gated-at.bofh.it>
In reply to#1501927
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.

[toc] | [prev] | [next] | [standalone]


#1502626

FromNicholas Piggin <npiggin@gmail.com>
Date2016-10-18 03:40 +0200
Message-ID<strxL-3nR-3@gated-at.bofh.it>
In reply to#1502602
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

[toc] | [prev] | [next] | [standalone]


#1503702

FromMichal Marek <mmarek@suse.com>
Date2016-10-19 16:40 +0200
Message-ID<su0c9-2v4-15@gated-at.bofh.it>
In reply to#1502626
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

[toc] | [prev] | [next] | [standalone]


#1504508

FromNicholas Piggin <npiggin@gmail.com>
Date2016-10-20 06:00 +0200
Message-ID<sucGl-28N-9@gated-at.bofh.it>
In reply to#1503702
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

[toc] | [prev] | [next] | [standalone]


#1501926 — [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromArnd Bergmann <arnd@arndb.de>
Date2016-10-17 14:30 +0200
Subject[PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<stfdf-3r0-3@gated-at.bofh.it>
In reply to#1501639
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)

[toc] | [prev] | [next] | [standalone]


#1503778 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromMichal Marek <mmarek@suse.com>
Date2016-10-19 17:00 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<su0vw-2D0-35@gated-at.bofh.it>
In reply to#1501926
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

[toc] | [prev] | [next] | [standalone]


#1503846 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromArnd Bergmann <arnd@arndb.de>
Date2016-10-19 17:10 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<su0Fe-2Wq-99@gated-at.bofh.it>
In reply to#1503778
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

[toc] | [prev] | [next] | [standalone]


#1503941 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2016-10-19 17:40 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<su18f-37s-77@gated-at.bofh.it>
In reply to#1503846
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.

[toc] | [prev] | [next] | [standalone]


#1504514 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromNicholas Piggin <npiggin@gmail.com>
Date2016-10-20 06:20 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<sucZI-2zP-11@gated-at.bofh.it>
In reply to#1503941
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

[toc] | [prev] | [next] | [standalone]


#1504887 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2016-10-20 15:30 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<sulzX-84O-15@gated-at.bofh.it>
In reply to#1504514
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.

[toc] | [prev] | [next] | [standalone]


#1504928 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromNicholas Piggin <npiggin@gmail.com>
Date2016-10-20 16:30 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<sumw1-mk-1@gated-at.bofh.it>
In reply to#1504887
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

[toc] | [prev] | [next] | [standalone]


#1504938 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2016-10-20 16:40 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<sumFH-pD-3@gated-at.bofh.it>
In reply to#1504928
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.

[toc] | [prev] | [next] | [standalone]


#1504954 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromNicholas Piggin <npiggin@gmail.com>
Date2016-10-20 17:00 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<sumZ3-wo-11@gated-at.bofh.it>
In reply to#1504938
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.

[toc] | [prev] | [next] | [standalone]


#1506594 — Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM

FromMichal Marek <mmarek@suse.com>
Date2016-10-22 22:00 +0200
SubjectRe: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Message-ID<svaCt-7IO-13@gated-at.bofh.it>
In reply to#1504938
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

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web