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

[Arnd Bergmann]

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 
>  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

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

[Arnd Bergmann]

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 
>  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

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

[Michal Marek]

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 
> > Date: Fri, 21 Oct 2016 01:13:33 +1100
> > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds
> > 
> > Signed-off-by: Nicholas Piggin 
> 
> Reported-by: Russell King 
> Tested-by: Russell King 

Thanks. Added to kbuild.git#rc-fixes.

Michal

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

[Michal Marek]

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 
> > Date: Fri, 21 Oct 2016 01:13:33 +1100
> > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds
> > 
> > Signed-off-by: Nicholas Piggin 
> 
> Reported-by: Russell King 
> Tested-by: Russell King 

Thanks. Added to kbuild.git#rc-fixes.

Michal

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

[Nicholas Piggin]

On Thu, 20 Oct 2016 15:33:27 +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 
> > Date: Fri, 21 Oct 2016 01:13:33 +1100
> > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds
> > 
> > Signed-off-by: Nicholas Piggin   
> 
> Reported-by: Russell King 
> Tested-by: Russell King 

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.

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

[Nicholas Piggin]

On Thu, 20 Oct 2016 15:33:27 +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 
> > Date: Fri, 21 Oct 2016 01:13:33 +1100
> > Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds
> > 
> > Signed-off-by: Nicholas Piggin   
> 
> Reported-by: Russell King 
> Tested-by: Russell King 

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.

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

[Russell King - ARM Linux]

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 
> Date: Fri, 21 Oct 2016 01:13:33 +1100
> Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds
> 
> Signed-off-by: Nicholas Piggin 

Reported-by: Russell King 
Tested-by: Russell King 

> 
> ---
>  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.

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

[Russell King - ARM Linux]

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 
> Date: Fri, 21 Oct 2016 01:13:33 +1100
> Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds
> 
> Signed-off-by: Nicholas Piggin 

Reported-by: Russell King 
Tested-by: Russell King 

> 
> ---
>  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.

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

[Nicholas Piggin]

On Thu, 20 Oct 2016 14:17:02 +0100
Russell King - ARM Linux  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 
Date: Fri, 21 Oct 2016 01:13:33 +1100
Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds

Signed-off-by: Nicholas Piggin 

---
 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

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

[Nicholas Piggin]

On Thu, 20 Oct 2016 14:17:02 +0100
Russell King - ARM Linux  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 
Date: Fri, 21 Oct 2016 01:13:33 +1100
Subject: [PATCH] kbuild: prevent lib-ksyms.o rebuilds

Signed-off-by: Nicholas Piggin 

---
 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

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

[Russell King - ARM Linux]

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.

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

[Russell King - ARM Linux]

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.

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

[Geert Uytterhoeven]

Hi Russell,

On Thu, Oct 20, 2016 at 10:20 AM, Russell King - ARM Linux
 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
>>  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 -- ge...@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

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

[Geert Uytterhoeven]

Hi Russell,

On Thu, Oct 20, 2016 at 10:20 AM, Russell King - ARM Linux
 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
>>  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 -- ge...@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

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

[Russell King - ARM Linux]

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
>  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.

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

[Russell King - ARM Linux]

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
>  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.

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

[Geert Uytterhoeven]

On Wed, Oct 19, 2016 at 5:32 PM, Russell King - ARM Linux
 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 -- ge...@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

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

[Geert Uytterhoeven]

On Wed, Oct 19, 2016 at 5:32 PM, Russell King - ARM Linux
 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 -- ge...@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

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

[Nicholas Piggin]

On Wed, 19 Oct 2016 16:32:00 +0100
Russell King - ARM Linux  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   
> > > 
> > > 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

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

[Nicholas Piggin]

On Wed, 19 Oct 2016 16:32:00 +0100
Russell King - ARM Linux  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   
> > > 
> > > 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

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

[Russell King - ARM Linux]

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 
> > 
> > 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.

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

[Russell King - ARM Linux]

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 
> > 
> > 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.

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

[Arnd Bergmann]

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 
> 
> 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

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

[Arnd Bergmann]

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 
> 
> 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

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

[Michal Marek]

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 

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

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

[Michal Marek]

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 

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

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

[Arnd Bergmann]

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 

diff --git a/arch/arm/include/asm/asm-prototypes.h 
b/arch/arm/include/asm/asm-prototypes.h
new file mode 100644
index ..04e5616a7b15
--- /dev/null
+++ b/arch/arm/include/asm/asm-prototypes.h
@@ -0,0 +1,34 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+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_EXITENDPROC(csum_partial_copy_nocheck)
-#define FN_EXPORT  EXPORT_SYMBOL(csum_partial_copy_nocheck)
 
 

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

[Arnd Bergmann]

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 

diff --git a/arch/arm/include/asm/asm-prototypes.h 
b/arch/arm/include/asm/asm-prototypes.h
new file mode 100644
index ..04e5616a7b15
--- /dev/null
+++ b/arch/arm/include/asm/asm-prototypes.h
@@ -0,0 +1,34 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+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_EXITENDPROC(csum_partial_copy_nocheck)
-#define FN_EXPORT  EXPORT_SYMBOL(csum_partial_copy_nocheck)
 
 #include