Re: [PATCH v2 00/13] Cleanup recordmcount and begin objtool conversion

[Steven Rostedt]

Josh,

Can you have a look at these? I can apply them if you think they are OK.

-- Steve


On Tue, 11 Jun 2019 15:21:42 -0700
Matt Helsley <mhelsley@xxxxxxxxxx> wrote:

> This series cleans up recordmcount and then makes it into
> an objtool subcommand.
> 
> The series starts with 8 cleanup patches which make recordmcount
> easier to review and integrate with objtool. The final 5 patches
> show the beginning steps of converting recordmcount to use objtool's
> ELF code rather than its own open-coded methods of accessing ELF
> files.
> 
> ---
> 
> 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
> 
> 	Collected some build time measurements
> 
> Build times measurements -- median of multiple runs in a VM measured
> with "time":
> 
> 	mainline (5.2.0-rc4) build times (median of 3 runs):
> 		real    2m58.379s
> 		user    2m29.621s
> 		sys     1m35.116s
> 
> 	Post recordmcount-cleanup build times (median of 5 runs):
> 		real    2m51.973s
> 		user    2m29.094s
> 		sys     1m33.688s
> 		
> 	objtool mcount build times (median of 7 runs):
> 		real	2m57.92s
> 		user	2m33.73s
> 		sys	1m37.06s
> 
> Note: I saw some significant variation especially in the "real" time
> 	measurements probably because it was in a VM on a machine with
> 	various "idle" GUI tasks running. This is why I took the median
> 	rather than the mean. Though I haven't run the statistics, my
> 	sense is the numbers don't support concluding that things really
> 	got any faster or slower.
> 
> I'm working on a separate, follow-on RFC set which implements the
> pseudo-pipe idea.
> 
> Matt Helsley (13):
>   recordmcount: Remove redundant strcmp
>   recordmcount: Remove uread()
>   recordmcount: Remove unused fd from uwrite() and ulseek()
>   recordmcount: Rewrite error/success handling
>   recordmcount: Kernel style function signature formatting
>   recordmcount: Kernel style formatting
>   recordmcount: Remove redundant cleanup() calls
>   recordmcount: Clarify what cleanup() does
>   objtool: Prepare to merge recordmcount
>   objtool: Make recordmcount into an objtool subcmd
>   objtool: recordmcount: Start using objtool's elf wrapper
>   objtool: recordmcount: Search for __mcount_loc before walking the
>     sections
>   objtool: recordmcount: Convert do_func() relhdrs
> 
>  Makefile                                   |   6 +-
>  scripts/.gitignore                         |   1 -
>  scripts/Makefile                           |   1 -
>  scripts/Makefile.build                     |  27 +-
>  tools/objtool/.gitignore                   |   1 +
>  tools/objtool/Build                        |   1 +
>  tools/objtool/Makefile                     |   1 +
>  tools/objtool/builtin-mcount.c             |  72 +++++
>  tools/objtool/builtin-mcount.h             |  23 ++
>  tools/objtool/builtin.h                    |   1 +
>  tools/objtool/objtool.c                    |   1 +
>  {scripts => tools/objtool}/recordmcount.c  | 350 ++++++++++-----------
>  {scripts => tools/objtool}/recordmcount.h  | 197 +++++++-----
>  {scripts => tools/objtool}/recordmcount.pl |   0
>  14 files changed, 407 insertions(+), 275 deletions(-)
>  create mode 100644 tools/objtool/builtin-mcount.c
>  create mode 100644 tools/objtool/builtin-mcount.h
>  rename {scripts => tools/objtool}/recordmcount.c (78%)
>  rename {scripts => tools/objtool}/recordmcount.h (78%)
>  rename {scripts => tools/objtool}/recordmcount.pl (100%)
>