Messages in this thread | | | From | Ard Biesheuvel <> | Date | Sat, 10 Nov 2018 12:58:08 +0100 | Subject | Re: [RFC PATCH 1/3] static_call: Add static call infrastructure |
| |
On 10 November 2018 at 06:10, Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 9 Nov 2018 14:34:59 -0600 > Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > I'm slowly massaging this to work with tracepoints. > > But I hit a snag on this patch. > >> On Fri, Nov 09, 2018 at 02:57:46PM -0500, Steven Rostedt wrote: >> > On Fri, 9 Nov 2018 13:35:05 -0600 >> > Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > >> > >> > > > > +#define DECLARE_STATIC_CALL(key, func) \ >> > > > > + extern struct static_call_key key; \ >> > > > > + extern typeof(func) STATIC_CALL_TRAMP(key); \ >> > > > > + /* Preserve the ELF symbol so objtool can access it: */ \ >> > > > > + __ADDRESSABLE(key) >> > > > >> > > > Does the __ADDRESSABLE(key) need to be in the DECLARE part? >> > > > If so, there needs to be more explanation than just the comment above >> > > > it. >> > > >> > > For each call site, objtool creates a struct in .static_call_sites: >> > > >> > > struct static_call_site { >> > > s32 addr; >> > > s32 key; >> > > }; >> > > >> > > In order to do that, it needs to create a relocation which references >> > > the key symbol. If the key is defined in another .o file, then the >> > > current .o will not have an ELF symbol associated with the key. The >> > > __ADDRESSABLE(key) thing tells GCC to leave the key symbol in the .o >> > > file, even though it's not referenced anywhere. That makes objtool's >> > > job easier, so it doesn't have to edit the symbol table. >> > > >> > > I could add a comment saying as much, though it's hard to explain it in >> > > fewer words than I just did :-) >> > >> > Does this have to do with adding the references by relative address? >> > >> > In record_mcount, I just picked an existing symbol and referenced that.. >> > But perhaps this is a cleaner way. >> >> I think recordmcount is different. It creates references (in >> __mcount_loc) to functions which are already in the object file, so they >> already have symbols associated with them. >> >> But in this case, when objtool is creating references, the symbol it >> needs to reference is outside the .o file, so there's no symbol to >> associate it with. >> > > The __ADDRESSABLE() appears to fail if you have a header with a > DECLARE_STATIC_CALL() that is included where the DEFINE_STATIC_CALL() > is, because I'm getting this: > > In file included from <command-line>: > /work/git/linux-trace.git/include/linux/compiler.h:285:11: error: redefinition of ‘__addressable___tp_func_sys_enter40’ > __PASTE(__addressable_##sym, __LINE__) = (void *)&sym; > ^~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/compiler_types.h:53:23: note: in definition of macro ‘___PASTE’ > #define ___PASTE(a,b) a##b > ^ > /work/git/linux-trace.git/include/linux/compiler.h:285:3: note: in expansion of macro ‘__PASTE’ > __PASTE(__addressable_##sym, __LINE__) = (void *)&sym; > ^~~~~~~ > /work/git/linux-trace.git/include/linux/static_call.h:112:2: note: in expansion of macro ‘__ADDRESSABLE’ > __ADDRESSABLE(key) > ^~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/static_call.h:115:2: note: in expansion of macro ‘DECLARE_STATIC_CALL’ > DECLARE_STATIC_CALL(key, _func); \ > ^~~~~~~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/tracepoint.h:310:2: note: in expansion of macro ‘DEFINE_STATIC_CALL’ > DEFINE_STATIC_CALL(__tp_func_##name, __tracepoint_iter_##name); > ^~~~~~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/trace/define_trace.h:42:2: note: in expansion of macro ‘DEFINE_TRACE_FN’ > DEFINE_TRACE_FN(name, reg, unreg, PARAMS(proto), PARAMS(args)) > ^~~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/trace/events/syscalls.h:18:1: note: in expansion of macro ‘TRACE_EVENT_FN’ > TRACE_EVENT_FN(sys_enter, > ^~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/compiler.h:285:11: note: previous definition of ‘__addressable___tp_func_sys_enter40’ was here > __PASTE(__addressable_##sym, __LINE__) = (void *)&sym; > ^~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/compiler_types.h:53:23: note: in definition of macro ‘___PASTE’ > #define ___PASTE(a,b) a##b > ^ > /work/git/linux-trace.git/include/linux/compiler.h:285:3: note: in expansion of macro ‘__PASTE’ > __PASTE(__addressable_##sym, __LINE__) = (void *)&sym; > ^~~~~~~ > /work/git/linux-trace.git/include/linux/static_call.h:112:2: note: in expansion of macro ‘__ADDRESSABLE’ > __ADDRESSABLE(key) > ^~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/tracepoint.h:234:2: note: in expansion of macro ‘DECLARE_STATIC_CALL’ > DECLARE_STATIC_CALL(__tp_func_##name, __tracepoint_iter_##name); \ > ^~~~~~~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/tracepoint.h:421:2: note: in expansion of macro ‘__DECLARE_TRACE’ > __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \ > ^~~~~~~~~~~~~~~ > /work/git/linux-trace.git/include/linux/tracepoint.h:560:2: note: in expansion of macro ‘DECLARE_TRACE’ > DECLARE_TRACE(name, PARAMS(proto), PARAMS(args)) > ^~~~~~~~~~~~~ > /work/git/linux-trace.git/include/trace/events/syscalls.h:18:1: note: in expansion of macro ‘TRACE_EVENT_FN’ > TRACE_EVENT_FN(sys_enter, > > The complaint is on: > > DEFINE_STATIC_CALL(__tp_func_##name, __tracepoint_iter_##name); > > And the previous definition is on: > > DECLARE_STATIC_CALL(__tp_func_##name, __tracepoint_iter_##name); \ >
Does the DECLARE really need the __ADDRESSABLE? Its purpose is to ensure that symbols with static linkage are not optimized away, but since the reference is from a header file, the symbol should have external linkage anyway.
| |