Anshuman Khandual <anshuman.khandual@xxxxxxx> 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.
...
> 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
