Messages in this thread | | | Subject | Re: [RFC][PATCH v4 02/32] objtool: Make recordmcount into mcount subcmd | From | Julien Thierry <> | Date | Tue, 9 Jun 2020 20:11:26 +0100 |
| |
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
| |