On Fri, 8 Jul 2016 17:24:21 +0200
Petr Mladek [off-list ref] wrote:

On Fri 2016-07-08 17:07:09, Torsten Duwe wrote:

quoted

On Fri, Jul 08, 2016 at 04:58:00PM +0200, Petr Mladek wrote:  

quoted

On Mon 2016-06-27 17:17:17, Torsten Duwe wrote:  

quoted

Once gcc is enhanced to optionally generate NOPs at the beginning
of each function, like the concept proven in
https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01671.html
(sans the "fprintf (... pad_size);", which spoils the data structure
for kernel use), the generated pads can nicely be used to reroute
function calls for tracing/profiling, or live patching.

diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index ebecf9a..917065c 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c

@@ -39,6 +39,12 @@ static int ftrace_modify_code(unsigned long pc, u32 old, u32 new,
 		if (aarch64_insn_read((void *)pc, &replaced))
 			return -EFAULT;
 
+		/* If we already have what we'll finally want,
+		 * report success. This is needed on startup.
+		 */
+		if (replaced == new)
+			return 0;  

This looks strange. I wonder if it actually hides a real bug that we
modify the code twice or so.  

Not at all. All "profilers" we abused so far generate code that needs to
be disabled on boot first. prolog-pad generates nops, initially.  

Yeah, but I cannot find this kind of check in other architectures.
I checked arch/x86/kernel/ftrace.c, arch/s390/kernel/ftrace.c, and
arch/powerpc/kernel/ftrace.c. These all support ftrace with
regs and livepatching.

I guess the question is, with this approach, there's no call to mcount
or fentry at compile time? Just nops are added? In this case perhaps the
if statement should be more defined:

	/*
	 * On boot, with the prologue code, the code will already
	 * be a nop.
	 */
	if (replace == new && new == NOP)
		return 0;

And perhaps you can even pass in addr and check if it equals the nop
address. Maybe even not call this code then? That is, if addr ==
MCOUNT_ADDR passed in by ftrace_code_disable() have ftrace_make_nop()
simple return 0 without doing anything.

-- Steve