Steven Rostedt wrote:
On Thu, 10 Feb 2022 13:58:29 +0000 "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote:diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index f9feb197b2daaf..68f20cf34b0c47 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -1510,6 +1510,7 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs) }+#ifndef ftrace_cmp_recsstatic int ftrace_cmp_recs(const void *a, const void *b) { const struct dyn_ftrace *key = a; @@ -1521,6 +1522,7 @@ static int ftrace_cmp_recs(const void *a, const void *b) return 1; return 0; } +#endifI don't really care for this part, as it seems somewhat ugly. But this patch does appear to solve your issue, and I can't think of a prettier way to do this.
Yes, this approach looks like it will solve a few different issues for us. We would also like to nop-out the instruction before the branch to _mcount(), so this helps ensure that kprobes won't also overwrite that instruction.
So, I will reluctantly ack it.
Since we don't want to change struct dyn_ftrace, we will need to contain changes within the architecture code, so I don't see a way to do this in a generic manner.
The other option is to mark ftrace_cmp_recs() as a __weak function, but I have a vague recollection of you suggesting #ifdef rather than a __weak function in the past. I might be mis-remembering, so if you think making this a __weak function is better, I can do that.
If anything, please add a comment above saying that architectures may need to override this function, and when doing so, they will define their own ftrace_cmp_recs.
Sure. - Naveen
