[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-edac
Subject:    Re: [PATCH v12 3/3] trace,x86: code-sharing between non-trace and trace irq handlers
From:       Steven Rostedt <rostedt () goodmis ! org>
Date:       2013-05-23 19:54:52
Message-ID: 1369338892.6828.214.camel () gandalf ! local ! home
[Download RAW message or body]

On Fri, 2013-04-05 at 19:21 +0000, Seiji Aguchi wrote:
> [Issue]
> 
> Currently, irq vector handlers for tracing are just
> copied non-trace handlers by simply inserting tracepoints.
> 
> It is difficult to manage the codes.
> 
> [Solution]


This as a separate patch actually makes things more confusing to review.
It should be merged into the previous patch. If you want to break up the
changes, I would first add the entering_irq(), and exiting_irq() as
patch 1, and then do the rest of the changes in patch 2.

-- Steve

> This patch shares common codes between non-trace and trace handlers
> as follows to make them manageable and readable.
> 
> Non-trace irq handler:
> smp_irq_handler()
> {
> 	entering_irq(); /* pre-processing of this handler */
> 	__smp_irq_handler(); /*
>                           * common logic between non-trace and trace handlers
>                           * in a vector.
>                           */
> 	exiting_irq(); /* post-processing of this handler */
> 
> }
> 
> Trace irq_handler:
> smp_trace_irq_handler()
> {
> 	entering_irq(); /* pre-processing of this handler */
> 	trace_irq_entry(); /* tracepoint for irq entry */
> 	__smp_irq_handler(); /*
>                           * common logic between non-trace and trace handlers
>                           * in a vector.
>                           */
> 	trace_irq_exit(); /* tracepoint for irq exit */
> 	exiting_irq(); /* post-processing of this handler */
> 
> }
> 
> If tracepoints can place outside entering_irq()/exiting_irq() as follows, it looks \
> cleaner.
> 
> smp_trace_irq_handler()
> {
> 	trace_irq_entry();
> 	smp_irq_handler();
> 	trace_irq_exit();
> }
> 
> But it doesn't work.
> The problem is with irq_enter/exit() being called. They must be called before \
> trace_irq_enter/exit(),  because of the rcu_irq_enter() must be called before any \
> tracepoints are used, as tracepoints use  rcu to synchronize.
> 
> As a possible alternative, we may be able to call irq_enter() first as follows if \
> irq_enter() can nest.
> 
> smp_trace_irq_hander()
> {
> 	irq_entry();
> 	trace_irq_entry();
> 	smp_irq_handler();
> 	trace_irq_exit();
> 	irq_exit();
> }
> 
> But it doesn't work, either.
> If irq_enter() is nested, it may have a time penalty because it has to check if it \
> was already called or not. The time penalty is not desired in performance sensitive \
> paths even if it is tiny.
> 
> Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>


--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]