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
