All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>, Michal Marek <mmarek@suse.com>,
	Adam Borowski <kilobyte@angband.pl>,
	Omar Sandoval <osandov@osandov.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	adobriyan@gmail.com, sfr@canb.auug.org.au,
	viro@zeniv.linux.org.uk, linux-kbuild@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Subject: Re: [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM
Date: Fri, 21 Oct 2016 01:20:17 +1100	[thread overview]
Message-ID: <20161021012017.67b75a90@roar.ozlabs.ibm.com> (raw)
In-Reply-To: <20161020131702.GX1041@n2100.armlinux.org.uk>

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

  reply	other threads:[~2016-10-20 14:21 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-17  6:51 [GIT PULL] kbuild changes for v4.9-rc1 Adam Borowski
2016-10-17  6:59 ` Nicholas Piggin
2016-10-17 10:01   ` Adam Borowski
2016-10-17 10:01     ` Adam Borowski
2016-10-17 10:01     ` Adam Borowski
2016-10-17 11:12     ` Alexey Dobriyan
2016-10-17 11:17       ` Geert Uytterhoeven
2016-10-17 11:32         ` Alexey Dobriyan
2016-10-17 12:22     ` Mathieu OTHACEHE
2016-10-18  0:16       ` Adam Borowski
2016-10-18  1:34         ` Nicholas Piggin
2016-10-19 14:38           ` Michal Marek
2016-10-20  3:52             ` Nicholas Piggin
2016-10-27  8:10               ` Kalle Valo
2016-10-27 11:15                 ` Nicholas Piggin
2016-10-27 13:14                   ` Kalle Valo
2016-10-27 13:25                     ` Nicholas Piggin
2016-10-30 10:51                 ` Thorsten Leemhuis
2016-11-01 15:48           ` Michal Marek
2016-11-02 12:11             ` Adam Borowski
2016-11-02 12:14               ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for x86 Adam Borowski
2016-12-16 19:55     ` [GIT PULL] kbuild changes for v4.9-rc1 Jiri Slaby
2016-12-16 19:57       ` Linus Torvalds
2016-12-17  8:57         ` Jiri Slaby
2016-12-17  9:33           ` Adam Borowski
2016-12-17 23:59           ` Linus Torvalds
2016-12-18 10:49             ` Jiri Slaby
2016-12-18 11:03               ` Arend Van Spriel
2016-12-18 13:27                 ` Nikolay Borisov
2016-12-18 14:45                   ` Jiri Slaby
2016-12-18 14:54                     ` Nikolay Borisov
2016-12-18 15:08                       ` Jiri Slaby
2016-10-17 12:26   ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Arnd Bergmann
2016-10-19 14:52     ` Michal Marek
2016-10-19 15:02       ` Arnd Bergmann
2016-10-19 15:32         ` Russell King - ARM Linux
2016-10-20  4:08           ` Nicholas Piggin
2016-10-20 13:17             ` Russell King - ARM Linux
2016-10-20 14:20               ` Nicholas Piggin [this message]
2016-10-20 14:33                 ` Russell King - ARM Linux
2016-10-20 14:51                   ` Nicholas Piggin
2016-10-22 19:51                   ` Michal Marek
2016-10-24 15:04             ` Arnd Bergmann
2016-10-24 15:05               ` [PATCH 1/2] " Arnd Bergmann
2016-10-25  8:32                 ` Nicholas Piggin
2016-11-20 13:21                   ` Russell King - ARM Linux
2016-11-20 18:32                     ` Linus Torvalds
2016-11-20 19:12                       ` Russell King - ARM Linux
2016-11-21  6:10                         ` Nicholas Piggin
2016-11-21 18:46                 ` [1/2] " Uwe Kleine-König
2016-11-21 18:46                   ` Bug#844530: " Uwe Kleine-König
2016-11-21 19:13                   ` Russell King - ARM Linux
2016-11-22  1:01                     ` Nicholas Piggin
2016-10-24 15:06               ` [PATCH 2/2, variant A] ARM: add hidden mmioset/mmiocpy prototypes Arnd Bergmann
2016-10-24 15:06               ` [PATCH 2/2, variant B] ARM: move mmiocpy/mmioset exports to io.c Arnd Bergmann
2016-10-20  7:37           ` [PATCH] kbuild: provide include/asm/asm-prototypes.h for ARM Geert Uytterhoeven
2016-10-20  8:20             ` Russell King - ARM Linux
2016-10-20  8:23               ` Geert Uytterhoeven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161021012017.67b75a90@roar.ozlabs.ibm.com \
    --to=npiggin@gmail.com \
    --cc=adobriyan@gmail.com \
    --cc=arnd@arndb.de \
    --cc=kilobyte@angband.pl \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mmarek@suse.com \
    --cc=osandov@osandov.com \
    --cc=sfr@canb.auug.org.au \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.