Re: [RFC 4/7] LTTng instrumentation kernel

Mathieu Desnoyers
2007-11-16T02:43:09Z

* Mike Mason (mmlnx@us.ibm.com) wrote:
> snip
>> +void list_modules(void *call_data)
>> +{
>> + /* Enumerate loaded modules */
>> + struct list_head *i;
>> + struct module *mod;
>> + unsigned long refcount = 0;
>> +
>> + mutex_lock(&module_mutex);
>> + list_for_each(i, &modules) {
>> + mod = list_entry(i, struct module, list);
>> +#ifdef CONFIG_MODULE_UNLOAD
>> + refcount = local_read(&mod->ref[0].count);
>> +#endif
>> + __trace_mark(0, list_module, call_data,
>> + "name %s state %d refcount %lu",
>> + mod->name, mod->state, refcount);
>> + }
>> + mutex_unlock(&module_mutex);
>> +}
>> +EXPORT_SYMBOL_GPL(list_modules);
>> +
>> /* Given an address, look for it in the module exception tables. */
>> const struct exception_table_entry *search_module_extables(unsigned long
>> addr)
>> {
>
> What is the purpose of list_modules() in this patch? Seems outside the
> scope of the patches' intent. I assume LTTng uses it for some purpose, but
> it's not required to use the markers added by the patch.
>

Right, I should move it down in my patchset.

> Also, if list_modules() remains, the 0 should be removed from
> "__trace_mark(0, ..."
> Mike Mason

With the immediate values based markers, the 0 means an optimized
markers (non-generic). I use __trace_mark directly to be able to pass
the call_data argument.

Thanks for the review,

Mathieu