[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:       Seiji Aguchi <seiji.aguchi () hds ! com>
Date:       2013-05-24 13:28:55
Message-ID: A5ED84D3BB3A384992CBB9C77DEDA4D41B00320D () USINDEM103 ! corp ! hds ! com
[Download RAW message or body]



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

Thanks. I will update this patchset as above.

Seiji

> -----Original Message-----
> From: Steven Rostedt [mailto:rostedt@goodmis.org]
> Sent: Thursday, May 23, 2013 3:55 PM
> To: Seiji Aguchi
> Cc: linux-kernel@vger.kernel.org; x86@kernel.org; hpa@zytor.com; Thomas Gleixner \
> (tglx@linutronix.de); 'mingo@elte.hu' (mingo@elte.hu); Borislav Petkov \
> (bp@alien8.de); linux-edac@vger.kernel.org; Luck, Tony (tony.luck@intel.com); dle- \
>                 develop@lists.sourceforge.net; Tomoki Sekiyama
> Subject: Re: [PATCH v12 3/3] trace,x86: code-sharing between non-trace and trace \
> irq handlers 
> 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]