Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1508716 > unrolled thread

[PATCH 1/2] perf, x86-mm: Add exit-fault tracing

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] perf, x86-mm: Add exit-fault tracing Alexis Berlemont <alexis.berlemont@gmail.com> - 2016-10-26 02:00 +0200
    Re: [PATCH 1/2] perf, x86-mm: Add exit-fault tracing Peter Zijlstra <peterz@infradead.org> - 2016-10-26 11:00 +0200
      Re: [PATCH 1/2] perf, x86-mm: Add exit-fault tracing Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-26 21:00 +0200
        Re: [PATCH 1/2] perf, x86-mm: Add exit-fault tracing Peter Zijlstra <peterz@infradead.org> - 2016-10-27 16:00 +0200
          [PATCH v2] perf, x86-mm: declare page-faults tracepoints like irq-vectors ones Alexis Berlemont <alexis.berlemont@gmail.com> - 2016-10-28 01:40 +0200
        Re: [PATCH 1/2] perf, x86-mm: Add exit-fault tracing Peter Zijlstra <peterz@infradead.org> - 2016-10-27 16:50 +0200

#1508716 — [PATCH 1/2] perf, x86-mm: Add exit-fault tracing

Signed-off-by: Alexis Berlemont <alexis.berlemont@gmail.com>
---
 arch/x86/include/asm/trace/exceptions.h | 21 +++++++++++++++++++++
 arch/x86/mm/fault.c                     |  1 +
 2 files changed, 22 insertions(+)

diff --git a/arch/x86/include/asm/trace/exceptions.h b/arch/x86/include/asm/trace/exceptions.h
index 2fbc66c..39f78bb 100644
--- a/arch/x86/include/asm/trace/exceptions.h
+++ b/arch/x86/include/asm/trace/exceptions.h
@@ -43,6 +43,27 @@ DEFINE_EVENT_FN(x86_exceptions, name,				\
 DEFINE_PAGE_FAULT_EVENT(page_fault_user);
 DEFINE_PAGE_FAULT_EVENT(page_fault_kernel);
 
+TRACE_EVENT_FN(page_fault_exit,
+
+	TP_PROTO(unsigned long address),
+
+	TP_ARGS(address),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, address)
+	),
+
+	TP_fast_assign(
+		__entry->address = address;
+	),
+
+	TP_printk("address=%lx", __entry->address),
+
+	trace_irq_vector_regfunc,
+
+	trace_irq_vector_unregfunc
+);
+
 #undef TRACE_INCLUDE_PATH
 #define TRACE_INCLUDE_PATH .
 #define TRACE_INCLUDE_FILE exceptions
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 9f72ca3..e31e8ef 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1488,6 +1488,7 @@ trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
 	prev_state = exception_enter();
 	trace_page_fault_entries(address, regs, error_code);
 	__do_page_fault(regs, error_code, address);
+	trace_page_fault_exit(address);
 	exception_exit(prev_state);
 }
 NOKPROBE_SYMBOL(trace_do_page_fault);
-- 
2.10.1

[toc] | [next] | [standalone]

#1508956

On Wed, Oct 26, 2016 at 01:51:59AM +0200, Alexis Berlemont wrote:

-ENOCHANGELOG

> Signed-off-by: Alexis Berlemont <alexis.berlemont@gmail.com>
> ---

> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 9f72ca3..e31e8ef 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -1488,6 +1488,7 @@ trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
>  	prev_state = exception_enter();
>  	trace_page_fault_entries(address, regs, error_code);
>  	__do_page_fault(regs, error_code, address);
> +	trace_page_fault_exit(address);

Aside from my general hatred of tracepoint, it bugs me that its not
symmetric like the irq vector ones. But I'll leave that to x86 people.

>  	exception_exit(prev_state);
>  }
>  NOKPROBE_SYMBOL(trace_do_page_fault);
> -- 
> 2.10.1
> 

[toc] | [prev] | [next] | [standalone]

#1509703

Em Wed, Oct 26, 2016 at 10:51:16AM +0200, Peter Zijlstra escreveu:
> On Wed, Oct 26, 2016 at 01:51:59AM +0200, Alexis Berlemont wrote:
> 
> -ENOCHANGELOG

Yeah, please add one explaining why we want this, how we can use it, for
example, in 'perf trace', to measure how long each page fault took,
examples of it in use, etc.
 
> > Signed-off-by: Alexis Berlemont <alexis.berlemont@gmail.com>
> > ---
> 
> > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> > index 9f72ca3..e31e8ef 100644
> > --- a/arch/x86/mm/fault.c
> > +++ b/arch/x86/mm/fault.c
> > @@ -1488,6 +1488,7 @@ trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> >  	prev_state = exception_enter();
> >  	trace_page_fault_entries(address, regs, error_code);
> >  	__do_page_fault(regs, error_code, address);
> > +	trace_page_fault_exit(address);
> 
> Aside from my general hatred of tracepoint, it bugs me that its not
> symmetric like the irq vector ones. But I'll leave that to x86 people.

What is the simmetry problem, you think we should have:

  exceptions:page_fault_kernel_exit
  exceptions:page_fault_user_exit

To be counterparts of:

[root@jouet ~]# perf list exceptions:*

List of pre-defined events (to be used in -e):

  exceptions:page_fault_kernel                       [Tracepoint event]
  exceptions:page_fault_user                         [Tracepoint event]

[root@jouet ~]# 

? 

For perf usage yeah, the good thing would be to have just
exceptions:page_fault_entry and exceptions:page_fault_exit, and then use
perf_event_attr fields to filter what kind was desired, with a
header.misc stating where it took place, just like with other events,
but by now we have two for entry, so two for exit?

- Arnaldo

> >  	exception_exit(prev_state);
> >  }
> >  NOKPROBE_SYMBOL(trace_do_page_fault);
> > -- 
> > 2.10.1
> > 

[toc] | [prev] | [next] | [standalone]

#1510170

On Wed, Oct 26, 2016 at 04:53:39PM -0200, Arnaldo Carvalho de Melo wrote:

> > > +++ b/arch/x86/mm/fault.c
> > > @@ -1488,6 +1488,7 @@ trace_do_page_fault(struct pt_regs *regs, unsigned long error_code)
> > >  	prev_state = exception_enter();
> > >  	trace_page_fault_entries(address, regs, error_code);
> > >  	__do_page_fault(regs, error_code, address);
> > > +	trace_page_fault_exit(address);
> > 
> > Aside from my general hatred of tracepoint, it bugs me that its not
> > symmetric like the irq vector ones. But I'll leave that to x86 people.
> 
> What is the simmetry problem, you think we should have:

Look at arch/x86/include/asm/trace/irq_vectors.h and

$ git grep "trace.*_VECTOR"

The entry and exit tracepoints are fully symmetric and generate from a
single macro.

[toc] | [prev] | [next] | [standalone]

#1510779 — [PATCH v2] perf, x86-mm: declare page-faults tracepoints like irq-vectors ones

So, would you be OK with the following patch ? 

There is a symmetry just like irq_vectors tracepoints:

# perf list | grep -E "exception|irq_vectors"
  exceptions:page_fault_kernel_entry                 [Tracepoint event]
  exceptions:page_fault_kernel_exit                  [Tracepoint event]
  exceptions:page_fault_user_entry                   [Tracepoint event]
  exceptions:page_fault_user_exit                    [Tracepoint event]
  irq_vectors:call_function_entry                    [Tracepoint event]
  irq_vectors:call_function_exit                     [Tracepoint event]
...

Maybe 2 tracepoints (instead of 4) should have been enough; however,
there were already 1 tracepoint per mode before.

Alexis.

Alexis Berlemont (1):
  perf, x86-mm: declare page-faults tracepoints like irq-vectors ones

 arch/x86/include/asm/trace/exceptions.h | 17 ++++++++++++++++-
 arch/x86/mm/fault.c                     | 17 ++++++++++++++---
 2 files changed, 30 insertions(+), 4 deletions(-)

-- 
2.10.1

[toc] | [prev] | [next] | [standalone]

#1510331

On Wed, Oct 26, 2016 at 04:53:39PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 26, 2016 at 10:51:16AM +0200, Peter Zijlstra escreveu:
> > On Wed, Oct 26, 2016 at 01:51:59AM +0200, Alexis Berlemont wrote:
> > 
> > -ENOCHANGELOG
> 
> Yeah, please add one explaining why we want this, how we can use it, for
> example, in 'perf trace', to measure how long each page fault took,
> examples of it in use, etc.

Note that 'to measure page-fault duration' is not a reason, its a what.
This needs a proper reason.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web