lkml.org 
[lkml]   [2021]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2021-04-12 13:11    [W:0.050 / U:0.980 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site