On Fri, May 31, 2019 at 02:17:43PM +0530, Anshuman Khandual wrote: > On 05/30/2019 07:09 PM, Matthew Wilcox wrote: > > On Thu, May 30, 2019 at 05:31:15PM +0530, Anshuman Khandual wrote: > >> On 05/30/2019 04:36 PM, Matthew Wilcox wrote: > >>> The two handle preemption differently. Why is x86 wrong and this one > >>> correct? > >> > >> Here it expects context to be already non-preemptible where as the proposed > >> generic function makes it non-preemptible with a preempt_[disable|enable]() > >> pair for the required code section, irrespective of it's present state. Is > >> not this better ? > > > > git log -p arch/x86/mm/fault.c > > > > search for 'kprobes'. > > > > tell me what you think. > > Are you referring to these following commits > > a980c0ef9f6d ("x86/kprobes: Refactor kprobes_fault() like kprobe_exceptions_notify()") > b506a9d08bae ("x86: code clarification patch to Kprobes arch code") > > In particular the later one (b506a9d08bae). It explains how the invoking context > in itself should be non-preemptible for the kprobes processing context irrespective > of whether kprobe_running() or perhaps smp_processor_id() is safe or not. Hence it > does not make much sense to continue when original invoking context is preemptible. > Instead just bail out earlier. This seems to be making more sense than preempt > disable-enable pair. If there are no concerns about this change from other platforms, > I will change the preemption behavior in proposed generic function next time around. Exactly. So, any of the arch maintainers know of a reason they behave differently from x86 in this regard? Or can Anshuman use the x86 implementation for all the architectures supporting kprobes?