linux-kernel - RE: [PATCH v12 3/3] trace,x86: code-sharing between non-trace and trace irq handlers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 24 May 2013 13:28:55 +0000
From:	Seiji Aguchi <seiji.aguchi@....com>
To:	Steven Rostedt <rostedt@...dmis.org>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
	"Thomas Gleixner (tglx@...utronix.de)" <tglx@...utronix.de>,
	"'mingo@...e.hu' (mingo@...e.hu)" <mingo@...e.hu>,
	"Borislav Petkov (bp@...en8.de)" <bp@...en8.de>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	"Luck, Tony (tony.luck@...el.com)" <tony.luck@...el.com>,
	"dle-develop@...ts.sourceforge.net" 
	<dle-develop@...ts.sourceforge.net>,
	Tomoki Sekiyama <tomoki.sekiyama@....com>
Subject: RE: [PATCH v12 3/3] trace,x86: code-sharing between non-trace and
 trace irq handlers



> 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@...dmis.org]
> Sent: Thursday, May 23, 2013 3:55 PM
> To: Seiji Aguchi
> Cc: linux-kernel@...r.kernel.org; x86@...nel.org; hpa@...or.com; Thomas Gleixner (tglx@...utronix.de); 'mingo@...e.hu'
> (mingo@...e.hu); Borislav Petkov (bp@...en8.de); linux-edac@...r.kernel.org; Luck, Tony (tony.luck@...el.com); dle-
> develop@...ts.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@....com>
>