Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754113AbcLLDux (ORCPT ); Sun, 11 Dec 2016 22:50:53 -0500 Received: from mail-pf0-f196.google.com ([209.85.192.196]:33770 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751488AbcLLDuu (ORCPT ); Sun, 11 Dec 2016 22:50:50 -0500 Date: Mon, 12 Dec 2016 13:50:06 +1000 From: Nicholas Piggin To: Greg Kroah-Hartman Cc: Dodji Seketeli , Ian Campbell , Michal Marek , Ben Hutchings , Linus Torvalds , Adam Borowski , Linux Kbuild mailing list , Debian kernel maintainers , "linux-arch@vger.kernel.org" , Arnd Bergmann , Ingo Molnar , Linux Kernel Mailing List , Stanislav Kozina Subject: Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Message-ID: <20161212135006.58fb9d8e@roar.ozlabs.ibm.com> In-Reply-To: <20161210124103.GD21421@kroah.com> References: <1480541581.16599.78.camel@decadent.org.uk> <20161201125545.406d092c@roar.ozlabs.ibm.com> <1480559754.16599.92.camel@decadent.org.uk> <20161201143928.07a08348@roar.ozlabs.ibm.com> <6e8cf20b-2d2f-ba1f-e02c-c757d5a25db7@suse.com> <20161209133308.0acbb57a@roar.ozlabs.ibm.com> <1481296893.4509.135.camel@hellion.org.uk> <20161210021529.4a6e684f@roar.ozlabs.ibm.com> <86vaus3eld.fsf@seketeli.org> <20161210124103.GD21421@kroah.com> Organization: IBM X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4516 Lines: 108 On Sat, 10 Dec 2016 13:41:03 +0100 Greg Kroah-Hartman wrote: > On Fri, Dec 09, 2016 at 11:46:54PM +0100, Dodji Seketeli wrote: > > Hello, > > > > Nicholas Piggin a écrit: > > > > [...] > > > > > That said, a dwarf based checker tool should be able to do as good a job > > > (maybe a bit better because report is very informative and it may pick up > > > compiler alignments or padding options). > > > > So, Nicholas was kind enough to send me the two Linux Kernel binaries > > that he built with the tiny little interface change that we were > > discussing earlier. Here is what the abidiff[1] tools says about that > > interface change: > > > > $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi > > Functions changes summary: 0 Removed, 1 Changed, 0 Added function > > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable > > > > 1 function with some indirect sub-type change: > > > > [C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes: > > parameter 1 of type 'blah*' has sub-type changes: > > in pointed to type 'struct blah' at memory.c:78:1: > > type size changed from 32 to 64 bits > > 1 data member insertion: > > 'int blah::y', at offset 0 (in bits) at memory.c:79:1 > > 1 data member change: > > 'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits) > > > > > > > > real 0m2.595s > > user 0m2.489s > > sys 0m0.108s > > $ > > > > I kept the timing information to give you an idea of the time it takes > > on a non-optimized build of abidiff. > > > > One could for instance want that types that are not defined in header > > files be kept out of the change report. In that case it's possible to > > write a little suppression specification file like this one: > > > > $ cat vmlinux.abignore > > [suppress_type] > > source_location_not_regexp = .*\\.h > > $ > > > > You can then pass that suppression file to the tool: > > > > $ ~/git/libabigail/kabidiff/build/tools/abidiff --suppr vmlinux.abignore vmlinux.abi1.abi vmlinux.abi2.abi > > Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function > > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable > > > > > > real 0m2.574s > > user 0m2.473s > > sys 0m0.102s > > $ > > > > So this is the kind of interface change analysis tool we are working on > > at the moment. > > > > One could also imagine a tool that would compute a CRC that takes the > > very same suppression specification files into account, letting people > > to decide that some interface changes are OK. That CRC would thus be > > added to the special ELF sections we already have today. We could keep > > the modversion machinery, but with a greater dose of flexibility. > > Whenever modversion detects a change, abidiff would tell people what the > > change is exactly. > > > > What do you guys think? > > YES YES YES!!! > > Now I don't work on a distro anymore, but I would think that something > like this would be really useful, pointing out exactly what changed is > very important for distro maintainers to determine what they want to do > (either fix up the abi change with strange hacks, or ignore it due to > the change being in an area they don't care at all about, i.e. a random > driver subsystem.) > > So yes, I think this is really good stuff. But if the distro > maintainers correct me and think it's useless, then I need to revisit my > view of exactly what they do for their customers :) Agree completely. BTW (for those who might be looking into these tools), we also have https://github.com/skozina/kabi-dw that Stanislav (cc'ed) mentioned earlier. It's true that the current modversions __crc_ matching infrastructure is "just" a symbol versioning system, and we could keep it and just populate it with something other than genksyms (e.g., a symbol version list provided by distros). But the starting point should be *no* versioning and simply using names to break linkage. Unless there's a compelling reason not to, symbols are simpler, easier, everyone knows how they work. The other question would be whether to pull a minimal tool into the kernel source or keep them out of tree (but possibly add some helper scripts etc). I guess we'll need to see what distros want. Thanks, Nick