lkml.org 
[lkml]   [2020]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][PATCH v4 01/32] objtool: Prepare to merge recordmcount
    From
    Date
    Hi Matt,

    On 6/2/20 8:49 PM, Matt Helsley wrote:
    > Move recordmcount into the objtool directory. We keep this step separate
    > so changes which turn recordmcount into a subcommand of objtool don't
    > get obscured.
    >
    > Signed-off-by: Matt Helsley <mhelsley@vmware.com>
    > ---
    > Documentation/trace/ftrace-design.rst | 4 ++--
    > Documentation/trace/ftrace.rst | 2 +-
    > Makefile | 15 +++++++++------
    > scripts/.gitignore | 1 -
    > scripts/Makefile | 1 -
    > scripts/Makefile.build | 11 ++++++-----
    > tools/objtool/.gitignore | 1 +
    > tools/objtool/Build | 2 ++
    > tools/objtool/Makefile | 13 ++++++++++++-
    > {scripts => tools/objtool}/recordmcount.c | 0
    > {scripts => tools/objtool}/recordmcount.h | 0
    > {scripts => tools/objtool}/recordmcount.pl | 0
    > 12 files changed, 33 insertions(+), 17 deletions(-)
    > rename {scripts => tools/objtool}/recordmcount.c (100%)
    > rename {scripts => tools/objtool}/recordmcount.h (100%)
    > rename {scripts => tools/objtool}/recordmcount.pl (100%)
    >
    > diff --git a/Documentation/trace/ftrace-design.rst b/Documentation/trace/ftrace-design.rst
    > index a8e22e0db63c..dea8db5e79d0 100644
    > --- a/Documentation/trace/ftrace-design.rst
    > +++ b/Documentation/trace/ftrace-design.rst
    > @@ -261,7 +261,7 @@ You need very few things to get the syscalls tracing in an arch.
    > HAVE_FTRACE_MCOUNT_RECORD
    > -------------------------
    >
    > -See scripts/recordmcount.pl for more info. Just fill in the arch-specific
    > +See tools/objtool/recordmcount.pl for more info. Just fill in the arch-specific
    > details for how to locate the addresses of mcount call sites via objdump.
    > This option doesn't make much sense without also implementing dynamic ftrace.
    >
    > @@ -379,7 +379,7 @@ linux/ftrace.h for the functions::
    > ftrace_make_call()
    >
    > The rec->ip value is the address of the mcount call site that was collected
    > -by the scripts/recordmcount.pl during build time.
    > +by the tools/objtool/recordmcount.pl during build time.
    >
    > The last function is used to do runtime patching of the active tracer. This
    > will be modifying the assembly code at the location of the ftrace_call symbol
    > diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
    > index 3b5614b1d1a5..9adefcc3c7a8 100644
    > --- a/Documentation/trace/ftrace.rst
    > +++ b/Documentation/trace/ftrace.rst
    > @@ -2685,7 +2685,7 @@ starts of pointing to a simple return. (Enabling FTRACE will
    > include the -pg switch in the compiling of the kernel.)
    >
    > At compile time every C file object is run through the
    > -recordmcount program (located in the scripts directory). This
    > +recordmcount program (located in the tools/objtool directory). This
    > program will parse the ELF headers in the C object to find all
    > the locations in the .text section that call mcount. Starting
    > with gcc version 4.6, the -mfentry has been added for x86, which
    > diff --git a/Makefile b/Makefile
    > index 04f5662ae61a..d353a0a65a71 100644
    > --- a/Makefile
    > +++ b/Makefile
    > @@ -844,6 +844,7 @@ ifdef CONFIG_DYNAMIC_FTRACE
    > ifdef CONFIG_HAVE_C_RECORDMCOUNT
    > BUILD_C_RECORDMCOUNT := y
    > export BUILD_C_RECORDMCOUNT
    > + objtool_target := tools/objtool FORCE
    > endif
    > endif
    > endif
    > @@ -1023,10 +1024,10 @@ endif
    > export mod_sign_cmd
    >
    > HOST_LIBELF_LIBS = $(shell pkg-config libelf --libs 2>/dev/null || echo -lelf)
    > +has_libelf := $(call try-run,\
    > + echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
    >

    Maybe there could be some build dependency, e.g. CONFIG_OBJTOOL_SUBCMDS
    that sets the "objtool_target" and "has_libelf" when selected.

    Then the CONFIG_UNWINDER_ORC, RECORD_MCOUNT and STACK_VALIDATION would
    just had to select that config option.

    > ifdef CONFIG_STACK_VALIDATION
    > - has_libelf := $(call try-run,\
    > - echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
    > ifeq ($(has_libelf),1)
    > objtool_target := tools/objtool FORCE
    > else
    > @@ -1163,13 +1164,15 @@ uapi-asm-generic:
    >
    > PHONY += prepare-objtool
    > prepare-objtool: $(objtool_target)
    > -ifeq ($(SKIP_STACK_VALIDATION),1)
    > -ifdef CONFIG_UNWINDER_ORC
    > +ifneq ($(has_libelf),1)
    > + ifdef CONFIG_UNWINDER_ORC
    > @echo "error: Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
    > @false
    > -else
    > + else
    > + ifeq ($(SKIP_STACK_VALIDATION),1)
    > @echo "warning: Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2


    I think this would be more readable without the else branch:

    ifneq ($(has_libelf),1)
    ifdef <some objtool command config>
    <warn about unavailability>
    endif
    ifdef <another objtool command config>
    <warn ...>
    endif
    <...>
    endif


    Cheers,

    --
    Julien Thierry

    \
     
     \ /
      Last update: 2020-06-09 10:55    [W:2.490 / U:0.268 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site