RE: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured.

RE: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured.

[Luck, Tony]

>Please do not generate any code if the feature cannot ever be 
>used (CONFIG_KPROBES off). With this patch we still have lots of 
>unnecessary code being executed on each page fault.

I can (eventually) wrap this call inside the #ifdef CONFIG_KPROBES.

But I'd like to keep following leads on making the overhead as
low as possible for those people that do have KPROBES configured
(which may be most people if OS distributors ship kernels with
this enabled).

-Tony
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured.

[Andi Kleen]

On Wednesday 31 August 2005 01:05, Luck, Tony wrote:
> >Please do not generate any code if the feature cannot ever be
> >used (CONFIG_KPROBES off). With this patch we still have lots of
> >unnecessary code being executed on each page fault.
>
> I can (eventually) wrap this call inside the #ifdef CONFIG_KPROBES.

At least the original die notifiers were designed as a generic debugger
interface, not a kprobes specific thing. So I don't think it's a good idea.
Given most debuggers don't need the early page fault hook and it's
mostly needed for a special case in kprobes, but it doesn't seem nice to only 
offer a subset of the hooks with specific config options.

Also with the inline the test should be essentially a single test of 
a global variable and jump. Hardly a big performance issue, no? 

-Andi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured.

[Christoph Lameter]

On Wed, 31 Aug 2005, Andi Kleen wrote:

> Also with the inline the test should be essentially a single test of 
> a global variable and jump. Hardly a big performance issue, no? 

There are multiple effects of this code.

- Additional cacheline in use in the page fault handler 
  increasing the cache foot print.

- There are registers in use for checking the global variable.

- The compilers will reserve registers for the code that is never 
  executed which may affect other elements of performance. From the 
  register perspective a function call may be better on ia64.

Certainly not a big effect (if we make sure the compiler knows that 
this test mostly fails and insure that the variable is in 
__mostly_read) but this is a frequently executed code path and the code
is there without purpose if CONFIG_KPROBES is off.

It wont get too bad unless lots of other people have similar ideas about 
fixing their race conditions using similar methods. But we will be setting 
a bad precedent if we allow this.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured., Re: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured.

[David S. Miller, Andi Kleen]

> On Wednesday 31 August 2005 01:05, Luck, Tony wrote:
> > >Please do not generate any code if the feature cannot ever be
> > >used (CONFIG_KPROBES off). With this patch we still have lots of
> > >unnecessary code being executed on each page fault.
> >
> > I can (eventually) wrap this call inside the #ifdef CONFIG_KPROBES.
> 
> At least the original die notifiers were designed as a generic debugger
> interface, not a kprobes specific thing. So I don't think it's a good idea.

Me neither, I think a way too big deal is being made about
about this by the ia64 folks.  Just put the dang hook in
there unconditionally already :-)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] Only process_die notifier in ia64_do_page_fault if KPROBES is configured.

[Andi Kleen]

On Wednesday 31 August 2005 01:54, Christoph Lameter wrote:

> Certainly not a big effect (if we make sure the compiler knows that
> this test mostly fails and insure that the variable is in
> __mostly_read) 

Currently neither, but that could be easily fixed.

> but this is a frequently executed code path and the code 
> is there without purpose if CONFIG_KPROBES is off.

Well if you really worry about it then it might be better to do some dynamic
code patching to make generic and distribution kernels too.

> It wont get too bad unless lots of other people have similar ideas about
> fixing their race conditions using similar methods. But we will be setting
> a bad precedent if we allow this.

Agreed on the general direction, but I didn't see an alternative
for kprobes for this. Well actually the hook could be maybe
right now moved into the part before kernel oops which is much less
frequently executed, but then it'll likely move up again once
kprobes support user space probes.

-Andi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>