lkml.org 
[lkml]   [2020]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC][PATCH v4 02/32] objtool: Make recordmcount into mcount subcmd
From
Date


On 6/9/20 7:39 PM, Matt Helsley wrote:
> On Tue, Jun 09, 2020 at 10:00:59AM +0100, Julien Thierry wrote:
>> Hi Matt,
>>
>> On 6/2/20 8:49 PM, Matt Helsley wrote:
>>> Rather than a standalone executable merge recordmcount as a sub command
>>> of objtool. This is a small step towards cleaning up recordmcount and
>>> eventually sharing ELF code with objtool.
>>>
>>> For the initial step all that's required is a bit of Makefile changes
>>> and invoking the former main() function from recordmcount.c because the
>>> subcommand code uses similar function arguments as main when dispatching.
>>>
>>> objtool ignores some object files that tracing does not, specifically
>>> those with OBJECT_FILES_NON_STANDARD Makefile variables. For this reason
>>> we keep the recordmcount_dep separate from the objtool_dep. When using
>>> objtool mcount we can also, like the other objtool invocations, just
>>> depend on the binary rather than the source the binary is built from.
>>>
>>> Subsequent patches will gradually convert recordmcount to use
>>> more and more of libelf/objtool's ELF accessor code. This will both
>>> clean up recordmcount to be more easily readable and remove
>>> recordmcount's crude accessor wrapping code.
>>>
>>> Signed-off-by: Matt Helsley <mhelsley@vmware.com>
>>> ---
> ...
>>> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
>>> index 743647005f64..ae74647b06fa 100644
>>> --- a/kernel/trace/Kconfig
>>> +++ b/kernel/trace/Kconfig
>>> @@ -59,7 +59,7 @@ config HAVE_NOP_MCOUNT
>>> config HAVE_C_RECORDMCOUNT
>>> bool
>>> help
>>> - C version of recordmcount available?
>>> + C version of objtool mcount available?
>>
>> The "C version" doesn't make much sense here. "Objtool mcount available?" or
>> "mcount subcommand of objtool available?" perhaps?
>
> Agreed, "C version" is nonsense at this point.
>
> Looking at the other HAVE_* help messages in that Kconfig suggests:
>
> Arch supports objtool mcount subcommand
>
> So I've changed it to that.
>

Yes, that seems good.

>>> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
>>> index 285474a77fe9..ffef73f7f47e 100644
>>> --- a/tools/objtool/Makefile
>>> +++ b/tools/objtool/Makefile
>>> @@ -31,12 +31,6 @@ OBJTOOL_IN := $(OBJTOOL)-in.o
>>> LIBELF_FLAGS := $(shell pkg-config libelf --cflags 2>/dev/null)
>>> LIBELF_LIBS := $(shell pkg-config libelf --libs 2>/dev/null || echo -lelf)
>>> -RECORDMCOUNT := $(OUTPUT)recordmcount
>>> -RECORDMCOUNT_IN := $(RECORDMCOUNT)-in.o
>>> -ifeq ($(BUILD_C_RECORDMCOUNT),y)
>>> -all: $(RECORDMCOUNT)
>>> -endif
>>> -
>>> all: $(OBJTOOL)
>>> INCLUDES := -I$(srctree)/tools/include \
>>> @@ -55,13 +49,47 @@ AWK = awk
>>> SUBCMD_CHECK := n
>>> SUBCMD_ORC := n
>>> +SUBCMD_MCOUNT := n
>>> ifeq ($(SRCARCH),x86)
>>> SUBCMD_CHECK := y
>>> SUBCMD_ORC := y
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),arm)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),arm64)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),ia64)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),mips)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),powerpc)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),s390)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),sh)
>>> + SUBCMD_MCOUNT := y
>>> +endif
>>> +
>>> +ifeq ($(SRCARCH),sparc)
>>> + SUBCMD_MCOUNT := y
>>
>> Is there some arch for which MCOUNT is not supported? If not you could just
>> have MCOUNT default to 'y' and avoid adding all those tests (or maybe reduce
>> the numbers and set to 'n' only for arches not supporting it).
>
> Yes, there are some which it does not support. For those architectures
> we keep recordmcount.pl around.
>
> It occured to me that with your suggestion to use more CONFIG_ variables
> we could eliminate this pattern and replace it with these pseudo-patches:
>
> +++ b/kernel/trace/Kconfig
>
> +config OBJTOOL_SUBCMD_MCOUNT
> + bool
> + depends on HAVE_C_RECORDMCOUNT
> + select OBJTOOL_SUBCMDS
> + help
> + Record mcount call locations using objtool
>
> and then change the Makefiles to use the CONFIG_ variables
> rather than have one ifeq block per arch:
>
> +++ b/tools/objtool/Makefile
>
> +SUBCMD_MCOUNT := $(CONFIG_OBJTOOL_SUBCMD_MCOUNT)
>
> Does this seem like a good use of CONFIG_ variables or is it going too
> far?
>

Definitely seems like a good idea to me! Will be a nice improvement.

Cheers,

--
Julien Thierry

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