lkml.org 
[lkml]   [2008]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [patch 01/15] Kernel Tracepoints
From
Date
On Tue, 2008-07-15 at 09:25 -0400, Mathieu Desnoyers wrote:
> * Peter Zijlstra (peterz@infradead.org) wrote:
> > On Wed, 2008-07-09 at 10:59 -0400, Mathieu Desnoyers wrote:

> > > +#define __DO_TRACE(tp, proto, args) \
> > > + do { \
> > > + int i; \
> > > + void **funcs; \
> > > + preempt_disable(); \
> > > + funcs = (tp)->funcs; \
> > > + smp_read_barrier_depends(); \
> > > + if (funcs) { \
> > > + for (i = 0; funcs[i]; i++) { \
> >
> > can't you get rid of 'i' and write:
> >
> > void **func;
> >
> > preempt_disable();
> > func = (tp)->funcs;
> > smp_read_barrier_depends();
> > for (; func; func++)
> > ((void (*)(proto))func)(args);
> > preempt_enable();
> >
>
> Yes, I though there would be an optimization to do here, I'll use your
> proposal. This code snippet is especially important since it will
> generate instructions near every tracepoint side. Saving a few bytes
> becomes important.
>
> Given that (tp)->funcs references an array of function pointers and that
> it can be NULL, the if (funcs) test must still be there and we must use
>
> #define __DO_TRACE(tp, proto, args) \
> do { \
> void *func; \
> \
> preempt_disable(); \
> if ((tp)->funcs) { \
> func = rcu_dereference((tp)->funcs); \
> for (; func; func++) { \
> ((void(*)(proto))(func))(args); \
> } \
> } \
> preempt_enable(); \
> } while (0)
>
>
> The resulting assembly is a bit more dense than my previous
> implementation, which is good :

My version also has that if ((tp)->funcs), but its hidden in the
for (; func; func++) loop. The only thing your version does is an extra
test of tp->funcs but without read depends barrier - not sure if that is
ok.





\
 
 \ /
  Last update: 2008-07-15 16:01    [W:0.142 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site