Le 18/04/2022 à 21:44, Steven Rostedt a écrit : > On Mon, 18 Apr 2022 11:51:16 +0530 > "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> wrote: > >>> --- a/arch/powerpc/kernel/trace/ftrace.c >>> +++ b/arch/powerpc/kernel/trace/ftrace.c >>> @@ -78,10 +78,7 @@ ftrace_modify_code(unsigned long ip, ppc_inst_t old, >>> ppc_inst_t new) >>> } >>> >>> /* replace the text with the new text */ >>> - if (patch_instruction((u32 *)ip, new)) >>> - return -EPERM; >>> - >>> - return 0; >>> + return patch_instruction((u32 *)ip, new); >> >> I think the reason we were returning -EPERM is so that ftrace_bug() can > > That is correct. > >> throw the right error message. That will change due to this patch, >> though I'm not sure how much it matters. -EFAULT and -EPERM seem to >> print almost the same error message. > > In these cases it helps to know the type of failure, as the way to debug it > is different. > > -EFAULT: It failed to read it the location. This means that the memory is > likely not even mapped in, or the pointer is way off. > > -EINVAL: Means that what was read did not match what was expected (the code > was already updated, pointing to the wrong location, or simply the > calculation of what to expect is incorrect). > > -EPERM: Means the write failed. What was read was expected, but the > permissions to write have not been updated properly. > > Differentiating the three is crucial to looking at where the issue lies > when an ftrace_bug() triggers. >
Apparently no caller really care about the value returned by patch_instruction(), the ones who check the return value just check that it's not 0. So the most performant would be to have patch_instruction() return -EPERM instead of -EFAULT in case of failure. Christophe
