Re: [RFC][PATCH v5 00/51] objtool: Make recordmcount a subcommand

2020-06-23 Thread Matt Helsley
On Thu, Jun 18, 2020 at 01:37:46PM -0700, Matt Helsley wrote:
> recordmcount has its own ELF wrapper code and could utilize
> objtool's ELF code to more-portably handle architecture variations.
> This series makes recordmcount a subcommand of objtool. It very
> gradually converts recordmcount to become a subcommand of objtool and
> then reuses parts of objtool's ELF code.
> 
> recordmcount maps the file in and collects simple information it needs to
> append a section to the object file. The only part of the original file it
> modifies is the address of new section tables -- interestingly enough this
> resembles RCU in that we don't really trim the old tables so
> much as unlink them via a critical offset and then rely on
> future tooling, in this case, to drop the unused bits.
> 
> Much of the recordmcount ELF code is only reading and walking the data
> structures to collect the mcount locations it records in a separate
> area of memory. This means it's safe to mix access to the mapped
> file with access to the objtool-style linked data
> structures as we gradually convert it to using only the linked data
> structures. Once the old ELF code is no longer in use we can drop it
> and use objtool to take over the task of writing the results without
> using the RCU-like trick any more.
> 
> After that we greatly simplify the mcount subcommand by adding a
> few flags to the ELF reading code in objtool. Overall the series
> removes about 600 lines of recordmcount while adding little to
> objtool's ELF code.
> 
> Testing so far:
> 
> I've been using scripts to test cross compilation and execution of
> objtool, and mcount on objects built for x86, ppc64le, arm64, s390, and
> sparc.
> 
> 
> Applies on top of:
>   objtool/core
> 
>   Peter Zijlstra's "x86/entry: noinstr fixes" [2]

Just thought I'd note for anyone reviewing/playing with this series:

Peter's patches are now in objtool/core

>   Sami Tolvanen's patch enabling support for more than 64k
>   sections in recordmcount, already going upstream. [3]

Sami's patch is now in Linus' master branch

Cheers,
-Matt


[RFC][PATCH v5 00/51] objtool: Make recordmcount a subcommand

2020-06-18 Thread Matt Helsley
recordmcount has its own ELF wrapper code and could utilize
objtool's ELF code to more-portably handle architecture variations.
This series makes recordmcount a subcommand of objtool. It very
gradually converts recordmcount to become a subcommand of objtool and
then reuses parts of objtool's ELF code.

recordmcount maps the file in and collects simple information it needs to
append a section to the object file. The only part of the original file it
modifies is the address of new section tables -- interestingly enough this
resembles RCU in that we don't really trim the old tables so
much as unlink them via a critical offset and then rely on
future tooling, in this case, to drop the unused bits.

Much of the recordmcount ELF code is only reading and walking the data
structures to collect the mcount locations it records in a separate
area of memory. This means it's safe to mix access to the mapped
file with access to the objtool-style linked data
structures as we gradually convert it to using only the linked data
structures. Once the old ELF code is no longer in use we can drop it
and use objtool to take over the task of writing the results without
using the RCU-like trick any more.

After that we greatly simplify the mcount subcommand by adding a
few flags to the ELF reading code in objtool. Overall the series
removes about 600 lines of recordmcount while adding little to
objtool's ELF code.

Testing so far:

I've been using scripts to test cross compilation and execution of
objtool, and mcount on objects built for x86, ppc64le, arm64, s390, and
sparc.


Applies on top of:
objtool/core

Peter Zijlstra's "x86/entry: noinstr fixes" [2]

Sami Tolvanen's patch enabling support for more than 64k
sections in recordmcount, already going upstream. [3]

Changes since v4[1]:
v5:
Updated the HAVE_C_RECORDMCOUNT Kconfig description.

Removed unused extern is_cmd_mcount_available() spotted by Julien 
Thierry.

Issue with symbol info handling reported and fixed by Kamalesh Babulal.

Removed the giant per-supported-arch series of ifdef blocks in
the objtool Makefile and replaced with new CONFIG_ variables
as suggested by Julien Thierry.

Moved the first pass through the sections into objtool's ELF
code as suggested by Peter Zijlstra.

Removed a redundant section lookup.

Removed old comments that no longer apply since conversion to objtool.

v4:
Split out recordmcount cleanups and upstreamed. [5]

Split out, iterated on objtool multi-arch support, and upstreamed. [6]

Split out expanded relocation support, renamed types, and functions
to reflect expanded relocation support, and upstreamed. [4]

This set is based on the patches sent upstream and posted above.

Adapted to renames by Ingo and Peter: s/elf_open/elf_open_read/

Added weak symbols for mcount subcommand
This nicely eliminated the need for the mcount.h header.

Added tools/objtool/Makefile per-arch SUBCMD_ blocks for each
arch recordmcount / mcount supports.

Moved ftrace/mcount/record.h from objtool_dep to recordmcount_dep
This keeps the dependencies better organized.

Fixed Makefile issue reported for PowerPC and a couple other archs
by kbuild test robot. The always-$(BUILD_C_RECORDMCOUNT)
line wasn't sufficiently replaced. Added to prepare-objtool
target in top level Makefile.

Split up dependencies to be independent of CONFIG_STACK_VALIDATION
and CONFIG_UNWINDER_ORC since these are x86-specific.
Now any arch which uses the C version of recordmcount
will build objtool if dynamic tracing is enabled.

Added a second rename at the end to be consistent with other
objtool subcommands.

v3:
Rebased on mainline. s/elf_open/elf_read/ in recordmcount.c

v2:
Fix whitespace before line continuation

Add ftrace/mcount/record.h to objtool_dep

Rename the Makefile variable BUILD_C_RECORDMCOUNT to
better reflect its purpose

Similar: rename recordmcount_source => recordmcount_dep
When using objtool we can just depend on the
binary rather than the source the binary is
built from. This should address Josh's feedback and
make the Makefile code a bit clearer

Add a comment to make reading the Makefile a little
easier

Rebased to latest mainline -rc

[1] https://lore.kernel.org/lkml/cover.1591125127.git.mhels...@vmware.com/
[2] https://lore.kernel.org/lkml/20200618144407.520952...@infradead.org
[3] https://lore.kernel.org/lkml/20200424193046.160744-1-samitolva...@google.com
[4] https://lore.kernel.org/lkml/cover.1590785960.git.mhels...@vmware.com/
[5] https://lore.kernel.org/lkml/20190802134712