[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070510125840.GB22424@Krystal>
Date: Thu, 10 May 2007 08:58:40 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To: Alexey Dobriyan <adobriyan@...il.com>
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
hch@...radead.org, Adrian Bunk <bunk@...sta.de>
Subject: Re: [patch 02/10] Linux Kernel Markers, architecture independent code.
Hi Alexey,
* Alexey Dobriyan (adobriyan@...il.com) wrote:
> On Wed, May 09, 2007 at 09:55:57PM -0400, Mathieu Desnoyers wrote:
> > --- /dev/null
> > +++ linux-2.6-lttng/include/linux/marker.h
> > @@ -0,0 +1,124 @@
>
> > +#ifdef __KERNEL__
>
> Just don't add this file to include/linux/Kbuild and remove __KERNEL__
> ifdef.
>
> > --- linux-2.6-lttng.orig/include/linux/module.h
> > +++ linux-2.6-lttng/include/linux/module.h
> > @@ -356,6 +356,9 @@
> > /* The command line arguments (may be mangled). People like
> > keeping pointers to this stuff */
> > char *args;
> > +
> > + const struct __mark_marker *markers;
> > + unsigned int num_markers;
>
> #ifdef CONFIG_MARKERS, please.
>
Ok, I merged the patch from SystemTAP to help them parse the markers
section, but seeing the violent objections it gets, I guess they will
have to figure out another way to parse it. Let's drop the
allow-userspace-applications-to-use-markerh-to-parse-the-markers-section-in-the-kernel-binary.patch
then.
> > --- linux-2.6-lttng.orig/kernel/module.c
> > +++ linux-2.6-lttng/kernel/module.c
>
> > @@ -1659,6 +1884,9 @@
> > unsigned int unusedcrcindex;
> > unsigned int unusedgplindex;
> > unsigned int unusedgplcrcindex;
> > + unsigned int markersindex;
> > + unsigned int markersdataindex;
> > + unsigned int markersstringsindex;
>
> Bunch of underscores wouldn't hurt.
>
Hrm, I used the exact same variable naming style already present in the
function. Do you suggest changing _every_ variable name to underscores ?
> > +void list_modules(void)
> > +{
> > + /* 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);
> ^
> Buy second CPU, already. ;-)
>
Good catch, will fix. I do already have more than one, don't worry ;)
> > +#endif //CONFIG_MODULE_UNLOAD
> > + trace_mark(list_module, "%s %d %lu",
> > + mod->name, mod->state, refcount);
> > + }
> > + mutex_unlock(&module_mutex);
> > +}
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists