Re: [(take 2)patch 7/7] Notify page fault call chain

[Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com>]

On Tue, Apr 25, 2006 at 11:10:59AM +1000, Keith Owens wrote:
> bibo mao (on Mon, 24 Apr 2006 17:19:01 +0800) wrote:
> >I have some questions about page fault call chain.
> >1) Can kprobe_exceptions_notify be divided into two sub-functions, one 
> >is for die call chain, which handles DIE_BREAK/DIE_FAULT trap, the other 
> >is specially for DIE_PAGE_FAULT trap.
> 
> I asked the same question, Anil said that would/could be done in a
> later set of patches, but did not want to change too much code in one
> go.

I don't think it is necessary to split kprobe_exception_notify(). All it is 
having is a simple switch case and based on the swich case it takes appropriate
actions.

> 
> >2) page_fault_notifier is conditionally registered/unregistered in this 
> >patch, notify_page_fault(DIE_PAGE_FAULT...) is unconditionally called in 
> >  ia64_do_page_fault() function. I do not know whether it is possible to 
> >unconditionally register page_fault_notifier() call chain, but 
> >conditionally call notify_page_fault(DIE_PAGE_FAULT...) function.
> 
> Only by putting extra code at the site of notify_page_fault().  That
> would introduce a race against kprobes unregistering a user space
> probe.  The race is probably safe, but why risk it?

Their is no race while we call unregister_page_fault_notifier(), as we unregister
only after the last probe is removed and after synchronized_sched() is done which
guarantees that all the probes have completly finished executing.
> 
> >3) I do know whether there are other conditions like kdb/kgdb which need
> >call page fault call chain when page fault happens. If only kprobe need 
> >handle page fault, then I think kprobe_exceptions_notify can be called 
> >directly in ia64_do_page_fault() function. Of course,  the call chain 
> >method is more convenient and easy to extend for other fault causes(like 
> >kdb/kgdb).
> 
> Only kprobes needs the page fault handler.
> 
> Calling kprobe_exceptions_notify() directly would work but again it
> introduces races.  The register and unregister are atomic, and remember

Calling kprobe_exception_notify() directly can be made to work with
out any races if needed. Not sure if calling directly  will have 
any significant performance improvement over current non-overloaded
call chain notifications.

-Anil