Messages in this thread | | | Date | Wed, 24 Mar 2021 16:10:54 +0000 | From | Qais Yousef <> | Subject | Re: [PATCH v7 2/2] ARM: ftrace: Add MODULE_PLTS support |
| |
Hi Florian
On 03/23/21 20:37, Florian Fainelli wrote: > Hi Qais, > > On 3/23/2021 3:22 PM, Qais Yousef wrote: > > Hi Alexander > > > > On 03/22/21 18:02, Alexander Sverdlin wrote: > >> Hi Qais, > >> > >> On 22/03/2021 17:32, Qais Yousef wrote: > >>> Yes you're right. I was a bit optimistic on CONFIG_DYNAMIC_FTRACE will imply > >>> CONFIG_ARM_MODULE_PLTS is enabled too. > >>> > >>> It only has an impact on reducing ifdefery when calling > >>> > >>> ftrace_call_replace_mod(rec->arch.mod, ...) > >>> > >>> Should be easy to wrap rec->arch.mod with its own accessor that will return > >>> NULL if !CONFIG_ARM_MODULE_PLTS or just ifdef the functions. > >>> > >>> Up to Alexander to pick what he prefers :-) > >> > >> well, I of course prefer v7 as-is, because this review is running longer than two > >> years and I actually hope these patches to be finally merged at some point. > >> But you are welcome to optimize them with follow up patches :) > > > > I appreciate that and thanks a lot for your effort. My attempt to review and > > test here is to help in getting this merged. > > > > FWIW my main concern is about duplicating the range check in > > ftrace_call_replace() and using magic values that already exist in > > __arm_gen_branch_{arm, thumb2}() and better remain encapsulated there. > > Your patch in addition to Alexander's patch work for me as well, so feel > free to add a: > > Tested-by: Florian Fainelli <f.fainelli@gmail.com> > > FWIW, what is nice about Alexander's original patch is that it applies > relatively cleanly to older kernels as well where this is equally
How old are we talking? Was the conflict that bad for the stable maintainers to deal with it? ie: would it require sending the backport separately?
> needed. There is not currently any Fixes: tag being provided but maybe > we should amend the second patch with one?
I'm not sure if this will be considered new feature or a bug fix. FWIW, tagging it for stable sounds reasonable to me.
Thanks!
-- Qais Yosuef
| |