On Thu, 23 Apr 2020 17:41:52 +0200
Christophe Leroy [off-list ref] wrote:
  

quoted

diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 024f7aad1952..046485bb0a52 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c

@@ -139,52 +139,67 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
  	}
  }
  
+#define PATCH_INSN(addr, instr)						     \
+do {									     \
+	int rc = patch_instruction((unsigned int *)(addr), instr);	     \
+	if (rc) {							     \
+		pr_err("%s:%d Error patching instruction at 0x%pK (%pS): %d\n", \
+				__func__, __LINE__,			     \
+				(void *)(addr), (void *)(addr), rc);	     \
+		return rc;						     \
+	}								     \
+} while (0)
+  

I hate this kind of macro which hides the "return".

What about keeping the return action in the caller ?

Otherwise, what about implementing something based on the use of goto, 
on the same model as unsafe_put_user() for instance ?

#define PATCH_INSN(addr, instr) \
({
	int rc = patch_instruction((unsigned int *)(addr), instr);	     \
	if (rc)								     \
		pr_err("%s:%d Error patching instruction at 0x%pK (%pS): %d\n", \
				__func__, __LINE__,			     \
				(void *)(addr), (void *)(addr), rc);	     \
	rc;								     \
})


Then you can just do:

	ret = PATCH_INSN(...);
	if (ret)
		return ret;

in the code.

-- Steve