> 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:[email protected]] > Sent: Thursday, May 23, 2013 3:55 PM > To: Seiji Aguchi > Cc: [email protected]; [email protected]; [email protected]; Thomas > Gleixner ([email protected]); '[email protected]' > ([email protected]); Borislav Petkov ([email protected]); > [email protected]; Luck, Tony ([email protected]); dle- > [email protected]; 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 <[email protected]> >
