Re: [RFC V2] mm: Generalize notify_page_fault()
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-06-05 11:19:31
Also in:
linux-arm-kernel, linux-mm, linux-s390, linux-sh, lkml, sparclinux
Anshuman Khandual [off-list ref] writes:
Similar notify_page_fault() definitions are being used by architectures duplicating much of the same code. This attempts to unify them into a single implementation, generalize it and then move it to a common place. kprobes_built_in() can detect CONFIG_KPROBES, hence notify_page_fault() need not be wrapped again within CONFIG_KPROBES. Trap number argument can now contain upto an 'unsigned int' accommodating all possible platforms.
...
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index 58f69fa..1bc3b18 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c@@ -30,28 +30,6 @@ #ifdef CONFIG_MMU -#ifdef CONFIG_KPROBES -static inline int notify_page_fault(struct pt_regs *regs, unsigned int fsr) -{ - int ret = 0; - - if (!user_mode(regs)) { - /* kprobe_running() needs smp_processor_id() */ - preempt_disable(); - if (kprobe_running() && kprobe_fault_handler(regs, fsr)) - ret = 1; - preempt_enable(); - } - - return ret; -} -#else
You've changed several of the architectures from something like above, where it disables preemption around the call into the below:
+int __kprobes notify_page_fault(struct pt_regs *regs, unsigned int trap)
+{
+ int ret = 0;
+
+ /*
+ * To be potentially processing a kprobe fault and to be allowed
+ * to call kprobe_running(), we have to be non-preemptible.
+ */
+ if (kprobes_built_in() && !preemptible() && !user_mode(regs)) {
+ if (kprobe_running() && kprobe_fault_handler(regs, trap))
+ ret = 1;
+ }
+ return ret;
+}Which skips everything if we're preemptible. Is that an equivalent change? If so can you please explain why in more detail. Also why not have it return bool? cheers