Thread (13 messages) 13 messages, 3 authors, 2021-05-05

Re: [PATCH v8 0/3] ARM: Implement MODULE_PLT support in FTRACE

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2021-05-04 19:11:07
Also in: lkml

On 4/19/21 3:34 PM, Qais Yousef wrote:
On 04/19/21 14:54, Florian Fainelli wrote:
quoted

On 4/12/2021 4:08 AM, Qais Yousef wrote:
quoted
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:
quoted
Hi!

On 09/04/2021 17:33, Qais Yousef wrote:
quoted
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.
What is the plan to move forward with this patch series, should v8 be
submitted into RMK's patch tracker and improved upon from there, or do
you feel like your suggestion needs to be addressed right away?
There's no objection from my side to submitting this and improve later.
OK, thanks! Alexander, do you mind sending these patches to RMK's patch
tracker: https://www.armlinux.org.uk/developer/patches/?

Thank you!
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help