Messages in this thread | | | Date | Mon, 12 Apr 2021 12:08:10 +0100 | From | Qais Yousef <> | Subject | Re: [PATCH v8 0/3] ARM: Implement MODULE_PLT support in FTRACE |
| |
Hi Alexander
Fixing Ard's email as the Linaro one keeps bouncing back. Please fix that in your future postings.
On 04/12/21 08:28, Alexander Sverdlin wrote: > Hi! > > On 09/04/2021 17:33, Qais Yousef wrote: > > I still think the ifdefery in patch 3 is ugly. Any reason my suggestion didn't > > work out for you? I struggle to see how this is better and why it was hard to > > incorporate my suggestion. > > > > For example > > > > - old = ftrace_call_replace(ip, adjust_address(rec, addr)); > > +#ifdef CONFIG_ARM_MODULE_PLTS > > + /* mod is only supplied during module loading */ > > + if (!mod) > > + mod = rec->arch.mod; > > + else > > + rec->arch.mod = mod; > > +#endif > > + > > + old = ftrace_call_replace(ip, aaddr, > > + !IS_ENABLED(CONFIG_ARM_MODULE_PLTS) || !mod); > > +#ifdef CONFIG_ARM_MODULE_PLTS > > + if (!old && mod) { > > + aaddr = get_module_plt(mod, ip, aaddr); > > + old = ftrace_call_replace(ip, aaddr, true); > > + } > > +#endif > > + > > > > There's an ifdef, followed by a code that embeds > > !IS_ENABLED(CONFIG_ARM_MODULE_PLTS) followed by another ifdef :-/ > > No, it's actually two small ifdefed blocks added before and after an original call, > which parameters have been modified as well. The issue with arch.mod was explained > by Steven Rostedt, maybe you've missed his email.
If you're referring to arch.mod having to be protected by the ifdef I did address that. Please look at my patch.
My comment here refers to the ugliness of this ifdefery. Introducing 2 simple wrapper functions would address that as I've demonstrated in my suggestion/patch.
Thanks
-- Qais Yousef
| |