Messages in this thread | | | Date | Wed, 25 Mar 2009 09:40:54 +0100 | From | Ingo Molnar <> | Subject | Re: [patch 7/9] LTTng instrumentation - kernel |
| |
* Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Wednesday 25 March 2009 05:03:13 Ingo Molnar wrote: > > > > (Rusty Cc:-ed - for the module.c tracepoints below) > > Thanks, tho they look fine and non-intrusive to me.
Thanks - i'll take this as an Acked-by :-)
( Mathieu, mind re-sending a variant that does them via TRACE_EVENT(), against the tracing tree? That makes it useful not just to LTTng but the default mainline kernel as well. Thanks! )
> > I believe that to have a complete picture of module usage, module > > refcount get/put events should be included as well, beyond the basic > > load/free events. > > > > These both have performance impact (a module get/put in a fastpath > > hurts scalability), and are informative in terms of establishing the > > module dependency graph. > > A module_get()/put() should not hurt scalability at all! I went > to great and horrible lengths to ensure that was the case since > the rewrite in 2.4.
i know, it uses percpu refcounts :) Still it can be somewhat non-trivial in a fastpath:
static inline int try_module_get(struct module *module) { int ret = 1;
if (module) { unsigned int cpu = get_cpu(); if (likely(module_is_live(module))) local_inc(__module_ref_addr(module, cpu)); else ret = 0; put_cpu(); } return ret; }
So we want to reduce excessive uses of it.
[ And please forget i mentioned scalability - that bit is fine and you were offended rightfully :) ]
This reminds me. Couldnt we now:
#ifdef CONFIG_SMP char *refptr; #else local_t ref; #endif
... unify these bits to just standardize on a per-cpu refptr all the time, with Tejun's ueber-cool percpu changes in place?
> But a module dependency graph et. al. would be kind of cool.
Yeah. Can be in a separate patch as well - load/unload events are meaningful in isolation as well.
Ingo
| |