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

Keshavamurthy Anil S
2006-04-25T05:46:20Z

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