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

[Keith Owens <kaos@sgi.com>]

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.

>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?

>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
how long it took us to agree on how to make atomic register and
unregister race safe.  Using the existing notify chain code gives us
race safe register, call and unregister without having to verify that
any hand written replacement code is also race safe.

You would need to demonstrate that any hand crafted code was race safe
and had enough speed improvement to make the coding and debugging effort
worthwhile.