On Tue, Jan 19, 2010 at 03:59, Li Yi wrote: > On Tue, 2010-01-19 at 02:20 -0500, Mike Frysinger wrote: >> On Tue, Jan 19, 2010 at 02:16, Li Yi wrote: >> > On Mon, 2010-01-18 at 23:49 -0500, Mike Frysinger wrote: >> >> On Mon, Jan 18, 2010 at 23:35, <[email protected]> wrote: >> >> > Modified: trunk/arch/blackfin/include/asm/irqflags.h (8178 => 8179) >> >> > +# define TRACE_IRQS_ON bfin_trace_hardirqs_on >> >> > +# define TRACE_IRQS_OFF bfin_trace_hardirqs_off >> >> > +#else >> >> > +# define TRACE_IRQS_ON >> >> > +# define TRACE_IRQS_OFF >> >> > #endif >> >> >> >> this indirection really isnt needed. have the assembly macro expand >> >> into nothing: >> >> .macro bfin_trace_hardirqs_off >> >> #ifdef TRACE_IRQS_ON >> >> .... >> >> #endif >> >> .endm >> >> >> >> then always call bfin_trace_hardirqs_{on,off} >> >> >> > >> > Agree. >> > >> > Given the fact that the places using TRACE_IRQS_ON(OFF) is not so many, >> > and registers may need to save according to the context, I am thinking >> > to call the C function "trace_hardirqs_on(off)" directly, without using >> > macros. >> >> yeah, and since each location has so much possible register >> requirements, that's probably the way to go > > After a second thought - since calling trace_hardirqs_on(off) itself > will introduce many overhead, does it really make sense to save the time > of save/restore registers?
i think so considering these points are being added to some of the most often executed code (the context save/restore paths) > And using Marco will make it easy and safe to add trace pointers in > assembly code. > > Here is the shortest execution path of trace_hardirqs_off: > > 000454b0 <_trace_hardirqs_off>: > 454b0: fb 05 [--SP] = (R7:7, P5:3); > 454b2: 4a e1 1c 00 P2.H = 0x1c; /* ( 28) > P2=0x1ce7c4 <_tracer_enabled> */ > 454b6: 00 e8 05 00 LINK 0x14; /* (20) */ > 454ba: 0a e1 c8 e7 P2.L = 0xe7c8; /* (-6200) > P2=0x1ce7c8 <_trace_type> */ > 454be: 10 91 R0 = [P2]; > 454c0: 08 48 CC = !BITTST (R0, 0x1); /* bit > 1 */ > 454c2: 0a 1c IF CC JUMP 0x454d6 <_trace_hardirqs_off > +0x26> (BP); > ... > 454d6: 01 e8 00 00 UNLINK; > 454da: bb 05 (R7:7, P5:3) = [SP++]; > 454dc: 10 00 RTS; even though the generated code could be better, you can see that it tries to optimize the path when the tracer is disabled. just like ftrace, you want to minimize the overhead as much as possible when things are turned off. -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
